Skip to content

[CS] Don’t fail constraint generation for ErrorExpr or if type fails to resolve #60062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions include/swift/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,13 @@ enum class FixKind : uint8_t {
/// Ignore result builder body if it has `return` statements.
IgnoreResultBuilderWithReturnStmts,

/// Ignore `ErrorExpr` or `ErrorType` during pre-check.
IgnoreInvalidASTNode,

/// Ignore a named pattern whose type we couldn't infer. This issue should
/// already have been diagnosed elsewhere.
IgnoreInvalidNamedPattern,

/// Resolve type of `nil` by providing a contextual type.
SpecifyContextualTypeForNil,

Expand Down Expand Up @@ -707,6 +714,26 @@ class ContextualMismatch : public ConstraintFix {

bool diagnose(const Solution &solution, bool asNote = false) const override;

bool coalesceAndDiagnose(const Solution &solution,
ArrayRef<ConstraintFix *> secondaryFixes,
bool asNote = false) const override {
// If the from type or to type is a placeholer type that corresponds to an
// ErrorExpr, the issue has already been diagnosed. There's no need to
// produce another diagnostic for the contextual mismatch complainting that
// a type is not convertible to a placeholder type.
if (auto fromPlaceholder = getFromType()->getAs<PlaceholderType>()) {
if (fromPlaceholder->getOriginator().is<ErrorExpr *>()) {
return true;
}
}
if (auto toPlaceholder = getToType()->getAs<PlaceholderType>()) {
if (toPlaceholder->getOriginator().is<ErrorExpr *>()) {
return true;
}
}
return ConstraintFix::coalesceAndDiagnose(solution, secondaryFixes, asNote);
}

bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override;

static ContextualMismatch *create(ConstraintSystem &cs, Type lhs, Type rhs,
Expand Down Expand Up @@ -2693,6 +2720,57 @@ class IgnoreResultBuilderWithReturnStmts final
}
};

class IgnoreInvalidASTNode final : public ConstraintFix {
IgnoreInvalidASTNode(ConstraintSystem &cs, ConstraintLocator *locator)
: IgnoreInvalidASTNode(cs, FixKind::IgnoreInvalidASTNode, locator) {}

protected:
IgnoreInvalidASTNode(ConstraintSystem &cs, FixKind kind,
ConstraintLocator *locator)
: ConstraintFix(cs, kind, locator) {}

public:
std::string getName() const override { return "ignore invalid AST node"; }

bool diagnose(const Solution &solution, bool asNote = false) const override;

bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
return diagnose(*commonFixes.front().first);
}

static IgnoreInvalidASTNode *create(ConstraintSystem &cs,
ConstraintLocator *locator);

static bool classof(ConstraintFix *fix) {
return fix->getKind() == FixKind::IgnoreInvalidASTNode;
}
};

class IgnoreInvalidNamedPattern final : public ConstraintFix {
IgnoreInvalidNamedPattern(ConstraintSystem &cs, NamedPattern *pattern,
ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::IgnoreInvalidNamedPattern, locator) {}

public:
std::string getName() const override {
return "specify type for pattern match";
}

bool diagnose(const Solution &solution, bool asNote = false) const override;

bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
return diagnose(*commonFixes.front().first);
}

static IgnoreInvalidNamedPattern *create(ConstraintSystem &cs,
NamedPattern *pattern,
ConstraintLocator *locator);

static bool classof(ConstraintFix *fix) {
return fix->getKind() == FixKind::IgnoreInvalidNamedPattern;
}
};

class SpecifyContextualTypeForNil final : public ConstraintFix {
SpecifyContextualTypeForNil(ConstraintSystem &cs,
ConstraintLocator *locator)
Expand Down
3 changes: 3 additions & 0 deletions include/swift/Sema/ConstraintLocatorPathElts.def
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ CUSTOM_LOCATOR_PATH_ELT(SyntacticElement)
/// The element of the pattern binding declaration.
CUSTOM_LOCATOR_PATH_ELT(PatternBindingElement)

/// The variable declared by a named pattern.
SIMPLE_LOCATOR_PATH_ELT(NamedPatternDecl)

#undef LOCATOR_PATH_ELT
#undef CUSTOM_LOCATOR_PATH_ELT
#undef SIMPLE_LOCATOR_PATH_ELT
Expand Down
7 changes: 7 additions & 0 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,13 @@ T *getAsStmt(ASTNode node) {
return nullptr;
}

template <typename T = Pattern>
T *getAsPattern(ASTNode node) {
if (auto *P = node.dyn_cast<Pattern *>())
return dyn_cast_or_null<T>(P);
return nullptr;
}

SourceLoc getLoc(ASTNode node);
SourceRange getSourceRange(ASTNode node);

Expand Down
24 changes: 24 additions & 0 deletions lib/Sema/CSBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ bool BindingSet::isDelayed() const {
if (locator->directlyAt<NilLiteralExpr>())
return true;

// When inferring the type of a variable in a pattern, delay its resolution
// so that we resolve type variables inside the expression as placeholders
// instead of marking the type of the variable itself as a placeholder. This
// allows us to produce more specific errors because the type variable in
// the expression that introduced the placeholder might be diagnosable using
// fixForHole.
if (locator->isLastElement<LocatorPathElt::NamedPatternDecl>()) {
return true;
}

// It's possible that type of member couldn't be determined,
// and if so it would be beneficial to bind member to a hole
// early to propagate that information down to arguments,
Expand Down Expand Up @@ -1979,6 +1989,20 @@ TypeVariableBinding::fixForHole(ConstraintSystem &cs) const {
return std::make_pair(fix, /*impact=*/(unsigned)10);
}

if (auto pattern = getAsPattern<NamedPattern>(dstLocator->getAnchor())) {
if (dstLocator->getPath().size() == 1 &&
dstLocator->isLastElement<LocatorPathElt::NamedPatternDecl>()) {
// Not being able to infer the type of a variable in a pattern binding
// decl is more dramatic than anything that could happen inside the
// expression because we want to preferrably point the diagnostic to a
// part of the expression that caused us to be unable to infer the
// variable's type.
ConstraintFix *fix =
IgnoreInvalidNamedPattern::create(cs, pattern, dstLocator);
return std::make_pair(fix, /*impact=*/(unsigned)100);
}
}

return None;
}

Expand Down
24 changes: 24 additions & 0 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1945,6 +1945,16 @@ IgnoreInvalidResultBuilderBody::create(ConstraintSystem &cs,
return new (cs.getAllocator()) IgnoreInvalidResultBuilderBody(cs, locator);
}

bool IgnoreInvalidASTNode::diagnose(const Solution &solution,
bool asNote) const {
return true; // Already diagnosed by the producer of ErrorExpr or ErrorType.
}

IgnoreInvalidASTNode *IgnoreInvalidASTNode::create(ConstraintSystem &cs,
ConstraintLocator *locator) {
return new (cs.getAllocator()) IgnoreInvalidASTNode(cs, locator);
}

bool SpecifyContextualTypeForNil::diagnose(const Solution &solution,
bool asNote) const {
MissingContextualTypeForNil failure(solution, getLocator());
Expand Down Expand Up @@ -1994,6 +2004,20 @@ IgnoreResultBuilderWithReturnStmts::create(ConstraintSystem &cs, Type builderTy,
IgnoreResultBuilderWithReturnStmts(cs, builderTy, locator);
}

bool IgnoreInvalidNamedPattern::diagnose(const Solution &solution,
bool asNote) const {
// Not being able to infer the type of a pattern should already have been
// diagnosed on the pattern's initializer or as a structural issue of the AST.
return true;
}

IgnoreInvalidNamedPattern *
IgnoreInvalidNamedPattern::create(ConstraintSystem &cs, NamedPattern *pattern,
ConstraintLocator *locator) {
return new (cs.getAllocator())
IgnoreInvalidNamedPattern(cs, pattern, locator);
}

bool SpecifyBaseTypeForOptionalUnresolvedMember::diagnose(
const Solution &solution, bool asNote) const {
MemberMissingExplicitBaseTypeFailure failure(solution, MemberName,
Expand Down
39 changes: 23 additions & 16 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,11 @@ namespace {
// that member through the base returns a value convertible to the type
// of this expression.
auto baseTy = CS.getType(base);
if (isa<ErrorExpr>(base)) {
return CS.createTypeVariable(
CS.getConstraintLocator(expr, ConstraintLocator::Member),
TVO_CanBindToHole);
}
auto tv = CS.createTypeVariable(
CS.getConstraintLocator(expr, ConstraintLocator::Member),
TVO_CanBindToLValue | TVO_CanBindToNoEscape);
Expand Down Expand Up @@ -1068,19 +1073,11 @@ namespace {
ConstraintSystem &getConstraintSystem() const { return CS; }

virtual Type visitErrorExpr(ErrorExpr *E) {
if (!CS.isForCodeCompletion())
return nullptr;

// For code completion, treat error expressions that don't contain
// the completion location itself as holes. If an ErrorExpr contains the
// code completion location, a fallback typecheck is called on the
// ErrorExpr's OriginalExpr (valid sub-expression) if it had one,
// independent of the wider expression containing the ErrorExpr, so
// there's no point attempting to produce a solution for it.
if (CS.containsCodeCompletionLoc(E))
return nullptr;
CS.recordFix(
IgnoreInvalidASTNode::create(CS, CS.getConstraintLocator(E)));

return PlaceholderType::get(CS.getASTContext(), E);
return CS.createTypeVariable(CS.getConstraintLocator(E),
TVO_CanBindToHole);
}

virtual Type visitCodeCompletionExpr(CodeCompletionExpr *E) {
Expand Down Expand Up @@ -1374,7 +1371,11 @@ namespace {
const auto result = TypeResolution::resolveContextualType(
repr, CS.DC, resCtx, genericOpener, placeholderHandler);
if (result->hasError()) {
return Type();
CS.recordFix(
IgnoreInvalidASTNode::create(CS, CS.getConstraintLocator(locator)));

return CS.createTypeVariable(CS.getConstraintLocator(repr),
TVO_CanBindToHole);
}
// Diagnose top-level usages of placeholder types.
if (isa<PlaceholderTypeRepr>(repr->getWithoutParens())) {
Expand Down Expand Up @@ -1600,7 +1601,11 @@ namespace {

Type visitUnresolvedSpecializeExpr(UnresolvedSpecializeExpr *expr) {
auto baseTy = CS.getType(expr->getSubExpr());


if (baseTy->isTypeVariableOrMember()) {
return baseTy;
}

// We currently only support explicit specialization of generic types.
// FIXME: We could support explicit function specialization.
auto &de = CS.getASTContext().Diags;
Expand Down Expand Up @@ -2322,8 +2327,10 @@ namespace {
}

if (!varType) {
varType = CS.createTypeVariable(CS.getConstraintLocator(locator),
TVO_CanBindToNoEscape);
varType = CS.createTypeVariable(
CS.getConstraintLocator(pattern,
LocatorPathElt::NamedPatternDecl()),
TVO_CanBindToNoEscape | TVO_CanBindToHole);

// If this is either a `weak` declaration or capture e.g.
// `weak var ...` or `[weak self]`. Let's wrap type variable
Expand Down
41 changes: 29 additions & 12 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11211,18 +11211,6 @@ ConstraintSystem::simplifyApplicableFnConstraint(
// following: $T1 -> $T2.
auto func1 = type1->castTo<FunctionType>();

// If a type variable representing "function type" is a hole
// or it could be bound to some concrete type with a help of
// a fix, let's propagate holes to the "input" type. Doing so
// provides more information to upcoming argument and result matching.
if (shouldAttemptFixes()) {
if (auto *typeVar = type2->getAs<TypeVariableType>()) {
auto *locator = typeVar->getImpl().getLocator();
if (typeVar->isPlaceholder() || hasFixFor(locator))
recordAnyTypeVarAsPotentialHole(func1);
}
}

// Before stripping lvalue-ness and optional types, save the original second
// type for handling `func callAsFunction` and `@dynamicCallable`
// applications. This supports the following cases:
Expand All @@ -11237,6 +11225,29 @@ ConstraintSystem::simplifyApplicableFnConstraint(
type2 = getFixedTypeRecursive(type2, flags, /*wantRValue=*/true);
auto desugar2 = type2->getDesugaredType();

// If a type variable representing "function type" is a hole
// or it could be bound to some concrete type with a help of
// a fix, let's propagate holes to the "input" type. Doing so
// provides more information to upcoming argument and result matching.
if (shouldAttemptFixes()) {
if (auto *typeVar = type2->getAs<TypeVariableType>()) {
auto *locator = typeVar->getImpl().getLocator();
if (hasFixFor(locator)) {
recordAnyTypeVarAsPotentialHole(func1);
}
}
Type underlyingType = desugar2;
while (auto *MT = underlyingType->getAs<AnyMetatypeType>()) {
underlyingType = MT->getInstanceType();
}
underlyingType =
getFixedTypeRecursive(underlyingType, flags, /*wantRValue=*/true);
if (underlyingType->isPlaceholder()) {
recordAnyTypeVarAsPotentialHole(func1);
return SolutionKind::Solved;
}
}

TypeMatchOptions subflags = getDefaultDecompositionOptions(flags);

SmallVector<LocatorPathElt, 2> parts;
Expand Down Expand Up @@ -12927,6 +12938,12 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
case FixKind::RenameConflictingPatternVariables: {
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
}
case FixKind::IgnoreInvalidASTNode: {
return recordFix(fix, 10) ? SolutionKind::Error : SolutionKind::Solved;
}
case FixKind::IgnoreInvalidNamedPattern: {
return recordFix(fix, 100) ? SolutionKind::Error : SolutionKind::Solved;
}

case FixKind::ExplicitlyConstructRawRepresentable: {
// Let's increase impact of this fix for binary operators because
Expand Down
12 changes: 12 additions & 0 deletions lib/Sema/ConstraintLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const {
case ConstraintLocator::PackType:
case ConstraintLocator::PackElement:
case ConstraintLocator::PatternBindingElement:
case ConstraintLocator::NamedPatternDecl:
return 0;

case ConstraintLocator::FunctionArgument:
Expand Down Expand Up @@ -441,6 +442,11 @@ void LocatorPathElt::dump(raw_ostream &out) const {
<< llvm::utostr(patternBindingElt.getIndex());
break;
}

case ConstraintLocator::NamedPatternDecl: {
out << "named pattern decl";
break;
}
}
}

Expand Down Expand Up @@ -592,6 +598,12 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) const {
out << '@';
expr->getLoc().print(out, *sm);
}
} else if (auto *pattern = anchor.dyn_cast<Pattern *>()) {
out << Pattern::getKindName(pattern->getKind()) << "Pattern";
if (sm) {
out << '@';
pattern->getLoc().print(out, *sm);
}
}

for (auto elt : getPath()) {
Expand Down
7 changes: 7 additions & 0 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5256,6 +5256,13 @@ void constraints::simplifyLocator(ASTNode &anchor,
continue;
}

case ConstraintLocator::NamedPatternDecl: {
auto pattern = cast<NamedPattern>(anchor.get<Pattern *>());
anchor = pattern->getDecl();
path = path.slice(1);
break;
}

case ConstraintLocator::ImplicitConversion:
break;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
// expected-error@-1 {{cannot find 'CGRect' in scope}}

let (r, s) = square.divided(atDistance: 50, from: .minXEdge)
// expected-error@-1 {{type of expression is ambiguous without more context}}
// expected-error@-1 {{cannot infer contextual base in reference to member 'minXEdge'}}
#endif

#if canImport(MixedWithHeader)
Expand Down
Loading