From a6755f17541e3a4f52cdf7024f481c7ada4202be Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 25 Oct 2021 13:04:45 -0700 Subject: [PATCH 01/14] [CSFix] Diagnose multiple solutions with missing closure param as un-ambiguous If there are multiple overloads and all of them require closure to have a parameter, let's diagnose that as such instead of ambiguity. --- include/swift/Sema/CSFix.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h index f776d83d062ba..9988ab05250b3 100644 --- a/include/swift/Sema/CSFix.h +++ b/include/swift/Sema/CSFix.h @@ -2308,6 +2308,10 @@ class SpecifyClosureParameterType final : public ConstraintFix { bool diagnose(const Solution &solution, bool asNote = false) const override; + bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override { + return diagnose(*commonFixes.front().first); + } + static SpecifyClosureParameterType *create(ConstraintSystem &cs, ConstraintLocator *locator); From b337360d63ee318fc3428afdf68ebd730930a847 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 25 Oct 2021 13:06:58 -0700 Subject: [PATCH 02/14] [Sema] DebuggerTestingTransform: avoid re-using AST nodes in generated code While building a closure to inject `checkExpect` code, clone member references associated with assignment. Re-using AST nodes is generally invalid. This is visible with multi-statement closure inference enabled, because the body is type-checked together with enclosing context and both elements end up sharing `DeclRefExpr` and `MemberRefExpr`s. --- lib/Sema/DebuggerTestingTransform.cpp | 43 +++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/lib/Sema/DebuggerTestingTransform.cpp b/lib/Sema/DebuggerTestingTransform.cpp index c1de09699583c..0cebffb8c741f 100644 --- a/lib/Sema/DebuggerTestingTransform.cpp +++ b/lib/Sema/DebuggerTestingTransform.cpp @@ -208,6 +208,49 @@ class DebuggerTestingTransform : public ASTWalker { auto *PODeclRef = new (Ctx) UnresolvedDeclRefExpr(StringForPrintObjectName, DeclRefKind::Ordinary, DeclNameLoc()); + + std::function cloneDeclRef = + [&](DeclRefExpr *DRE) { + auto *ref = new (Ctx) DeclRefExpr( + DRE->getDeclRef(), + /*Loc=*/DeclNameLoc(), + /*Implicit=*/true, DRE->getAccessSemantics(), DRE->getType()); + + if (auto hopTarget = DRE->isImplicitlyAsync()) + ref->setImplicitlyAsync(*hopTarget); + + ref->setImplicitlyThrows(DRE->isImplicitlyThrows()); + + return ref; + }; + + std::function cloneMemberRef = + [&](MemberRefExpr *M) { + auto *base = M->getBase(); + + if (auto *DRE = dyn_cast(base)) + base = cloneDeclRef(DRE); + else if (auto *M = dyn_cast(base)) + base = cloneMemberRef(M); + + auto *ref = new (Ctx) + MemberRefExpr(base, /*dotLoc=*/SourceLoc(), M->getDecl(), + /*loc=*/DeclNameLoc(), + /*Implicit=*/true, M->getAccessSemantics()); + ref->setType(M->getType()); + + return ref; + }; + + // Let's make a copy of either decl or member ref without source + // information. It's invalid to have decl reference expressions + // reused, each reference should get a fresh expression. + if (auto *DRE = dyn_cast(DstRef)) { + DstRef = cloneDeclRef(DRE); + } else { + DstRef = cloneMemberRef(cast(DstRef)); + } + auto *POArgList = ArgumentList::forImplicitUnlabeled(Ctx, {DstRef}); auto *POCall = CallExpr::createImplicit(Ctx, PODeclRef, POArgList); POCall->setThrows(false); From 46ff410a23f688a6fe2c8983fc1a5b90c15432d5 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 26 Oct 2021 11:38:01 -0700 Subject: [PATCH 03/14] [ConstraintSystem] Warn about discarded expressions found in multi-statement closures --- include/swift/Sema/Constraint.h | 15 +++++-- include/swift/Sema/ConstraintSystem.h | 4 +- lib/Sema/CSClosure.cpp | 60 ++++++++++++++++----------- lib/Sema/CSSimplify.cpp | 1 + lib/Sema/Constraint.cpp | 19 +++++---- 5 files changed, 62 insertions(+), 37 deletions(-) diff --git a/include/swift/Sema/Constraint.h b/include/swift/Sema/Constraint.h index c2aee59383749..083bf90f0a244 100644 --- a/include/swift/Sema/Constraint.h +++ b/include/swift/Sema/Constraint.h @@ -442,6 +442,8 @@ class Constraint final : public llvm::ilist_node, ASTNode Element; /// Contextual information associated with the element (if any). ContextualTypeInfo Context; + /// Identifies whether result of this node is unused. + bool IsDiscarded; } ClosureElement; }; @@ -495,7 +497,7 @@ class Constraint final : public llvm::ilist_node, SmallPtrSetImpl &typeVars); /// Construct a closure body element constraint. - Constraint(ASTNode node, ContextualTypeInfo context, + Constraint(ASTNode node, ContextualTypeInfo context, bool isDiscarded, ConstraintLocator *locator, SmallPtrSetImpl &typeVars); @@ -585,12 +587,14 @@ class Constraint final : public llvm::ilist_node, static Constraint *createClosureBodyElement(ConstraintSystem &cs, ASTNode node, - ConstraintLocator *locator); + ConstraintLocator *locator, + bool isDiscarded = false); static Constraint *createClosureBodyElement(ConstraintSystem &cs, ASTNode node, ContextualTypeInfo context, - ConstraintLocator *locator); + ConstraintLocator *locator, + bool isDiscarded = false); /// Determine the kind of constraint. ConstraintKind getKind() const { return Kind; } @@ -857,6 +861,11 @@ class Constraint final : public llvm::ilist_node, return ClosureElement.Context; } + bool isDiscardedElement() const { + assert(Kind == ConstraintKind::ClosureBodyElement); + return ClosureElement.IsDiscarded; + } + /// For an applicable function constraint, retrieve the trailing closure /// matching rule. Optional getTrailingClosureMatching() const; diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 8f22254a479af..c149899082ea8 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -4854,8 +4854,8 @@ class ConstraintSystem { /// Simplify a closure body element constraint by generating required /// constraints to represent the given element in constraint system. SolutionKind simplifyClosureBodyElementConstraint( - ASTNode element, ContextualTypeInfo context, TypeMatchOptions flags, - ConstraintLocatorBuilder locator); + ASTNode element, ContextualTypeInfo context, bool isDiscarded, + TypeMatchOptions flags, ConstraintLocatorBuilder locator); public: // FIXME: Public for use by static functions. /// Simplify a conversion constraint with a fix applied to it. diff --git a/lib/Sema/CSClosure.cpp b/lib/Sema/CSClosure.cpp index 0fc7e3fb4f8b6..63facbf5596c6 100644 --- a/lib/Sema/CSClosure.cpp +++ b/lib/Sema/CSClosure.cpp @@ -162,8 +162,8 @@ static bool isViableElement(ASTNode element) { return true; } -using ElementInfo = - std::tuple; +using ElementInfo = std::tuple; static void createConjunction(ConstraintSystem &cs, ArrayRef elements, @@ -190,7 +190,8 @@ static void createConjunction(ConstraintSystem &cs, for (const auto &entry : elements) { ASTNode element = std::get<0>(entry); ContextualTypeInfo context = std::get<1>(entry); - ConstraintLocator *elementLoc = std::get<2>(entry); + bool isDiscarded = std::get<2>(entry); + ConstraintLocator *elementLoc = std::get<3>(entry); if (!isViableElement(element)) continue; @@ -201,8 +202,8 @@ static void createConjunction(ConstraintSystem &cs, if (isIsolated) element.walk(paramCollector); - constraints.push_back( - Constraint::createClosureBodyElement(cs, element, context, elementLoc)); + constraints.push_back(Constraint::createClosureBodyElement( + cs, element, context, elementLoc, isDiscarded)); } // It's possible that there are no viable elements in the body, @@ -220,8 +221,9 @@ static void createConjunction(ConstraintSystem &cs, } ElementInfo makeElement(ASTNode node, ConstraintLocator *locator, - ContextualTypeInfo context = ContextualTypeInfo()) { - return std::make_tuple(node, context, locator); + ContextualTypeInfo context = ContextualTypeInfo(), + bool isDiscarded = false) { + return std::make_tuple(node, context, isDiscarded, locator); } static ProtocolDecl *getSequenceProtocol(ASTContext &ctx, SourceLoc loc, @@ -751,6 +753,8 @@ class ClosureConstraintGenerator void visitBraceStmt(BraceStmt *braceStmt) { if (isSupportedMultiStatementClosure()) { + auto &ctx = cs.getASTContext(); + if (isChildOf(StmtKind::Case)) { auto *caseStmt = cast( locator->castLastElementTo() @@ -765,10 +769,15 @@ class ClosureConstraintGenerator SmallVector elements; for (auto element : braceStmt->getElements()) { + bool isDiscarded = + element.is() && + (!ctx.LangOpts.Playground && !ctx.LangOpts.DebuggerSupport); + elements.push_back(makeElement( element, cs.getConstraintLocator( - locator, LocatorPathElt::ClosureBodyElement(element)))); + locator, LocatorPathElt::ClosureBodyElement(element)), + /*contextualInfo=*/{}, isDiscarded)); } createConjunction(cs, elements, locator); @@ -925,28 +934,22 @@ bool isConditionOfStmt(ConstraintLocatorBuilder locator) { ConstraintSystem::SolutionKind ConstraintSystem::simplifyClosureBodyElementConstraint( - ASTNode element, ContextualTypeInfo context, TypeMatchOptions flags, - ConstraintLocatorBuilder locator) { + ASTNode element, ContextualTypeInfo context, bool isDiscarded, + TypeMatchOptions flags, ConstraintLocatorBuilder locator) { auto *closure = castToExpr(locator.getAnchor()); ClosureConstraintGenerator generator(*this, closure, getConstraintLocator(locator)); if (auto *expr = element.dyn_cast()) { - if (context.purpose != CTP_Unused) { - SolutionApplicationTarget target(expr, closure, context.purpose, - context.getType(), - /*isDiscarded=*/false); - - if (generateConstraints(target, FreeTypeVariableBinding::Disallow)) - return SolutionKind::Error; - - setSolutionApplicationTarget(expr, target); - return SolutionKind::Solved; - } + SolutionApplicationTarget target(expr, closure, context.purpose, + context.getType(), isDiscarded); - if (!generateConstraints(expr, closure, /*isInputExpression=*/false)) + if (generateConstraints(target, FreeTypeVariableBinding::Disallow)) return SolutionKind::Error; + + setSolutionApplicationTarget(expr, target); + return SolutionKind::Solved; } else if (auto *stmt = element.dyn_cast()) { generator.visit(stmt); } else if (auto *cond = element.dyn_cast()) { @@ -1241,13 +1244,20 @@ class ClosureConstraintApplication } ASTNode visitBraceStmt(BraceStmt *braceStmt) { + auto &cs = solution.getConstraintSystem(); + for (auto &node : braceStmt->getElements()) { if (auto expr = node.dyn_cast()) { // Rewrite the expression. - if (auto rewrittenExpr = rewriteExpr(expr)) - node = rewrittenExpr; - else + auto target = *cs.getSolutionApplicationTarget(expr); + if (auto rewrittenTarget = rewriteTarget(target)) { + node = rewrittenTarget->getAsExpr(); + + if (target.isDiscardedExpr()) + TypeChecker::checkIgnoredExpr(castToExpr(node)); + } else { hadError = true; + } } else if (auto stmt = node.dyn_cast()) { node = visit(stmt); } else { diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index be34c40987539..3498659d3b5a6 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -12709,6 +12709,7 @@ ConstraintSystem::simplifyConstraint(const Constraint &constraint) { case ConstraintKind::ClosureBodyElement: return simplifyClosureBodyElementConstraint( constraint.getClosureElement(), constraint.getElementContext(), + constraint.isDiscardedElement(), /*flags=*/None, constraint.getLocator()); case ConstraintKind::BindTupleOfFunctionParams: diff --git a/lib/Sema/Constraint.cpp b/lib/Sema/Constraint.cpp index 4f32c23c19370..9bb7de867d0f8 100644 --- a/lib/Sema/Constraint.cpp +++ b/lib/Sema/Constraint.cpp @@ -258,13 +258,14 @@ Constraint::Constraint(ConstraintKind kind, ConstraintFix *fix, Type first, } Constraint::Constraint(ASTNode node, ContextualTypeInfo context, - ConstraintLocator *locator, + bool isDiscarded, ConstraintLocator *locator, SmallPtrSetImpl &typeVars) : Kind(ConstraintKind::ClosureBodyElement), TheFix(nullptr), HasRestriction(false), IsActive(false), IsDisabled(false), IsDisabledForPerformance(false), RememberChoice(false), IsFavored(false), IsIsolated(false), - NumTypeVariables(typeVars.size()), ClosureElement{node, context}, + NumTypeVariables(typeVars.size()), ClosureElement{node, context, + isDiscarded}, Locator(locator) { std::copy(typeVars.begin(), typeVars.end(), getTypeVariablesBuffer().begin()); } @@ -343,7 +344,8 @@ Constraint *Constraint::clone(ConstraintSystem &cs) const { getLocator()); case ConstraintKind::ClosureBodyElement: - return createClosureBodyElement(cs, getClosureElement(), getLocator()); + return createClosureBodyElement(cs, getClosureElement(), getLocator(), + isDiscardedElement()); } llvm_unreachable("Unhandled ConstraintKind in switch."); @@ -1012,18 +1014,21 @@ Constraint *Constraint::createApplicableFunction( Constraint *Constraint::createClosureBodyElement(ConstraintSystem &cs, ASTNode node, - ConstraintLocator *locator) { - return createClosureBodyElement(cs, node, ContextualTypeInfo(), locator); + ConstraintLocator *locator, + bool isDiscarded) { + return createClosureBodyElement(cs, node, ContextualTypeInfo(), locator, + isDiscarded); } Constraint *Constraint::createClosureBodyElement(ConstraintSystem &cs, ASTNode node, ContextualTypeInfo context, - ConstraintLocator *locator) { + ConstraintLocator *locator, + bool isDiscarded) { SmallPtrSet typeVars; unsigned size = totalSizeToAlloc(typeVars.size()); void *mem = cs.getAllocator().Allocate(size, alignof(Constraint)); - return new (mem) Constraint(node, context, locator, typeVars); + return new (mem) Constraint(node, context, isDiscarded, locator, typeVars); } Optional From 0e6e058e7ca34f341819948501889cdf7553b270 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 27 Oct 2021 16:29:48 -0700 Subject: [PATCH 04/14] [TypeChecker] Fix constraint solver to respect `LeaveClosureBodyUnchecked` flag --- include/swift/Sema/ConstraintSystem.h | 25 +++++++++++---------- lib/Sema/BuilderTransform.cpp | 4 +++- lib/Sema/CSApply.cpp | 3 +-- lib/Sema/CSClosure.cpp | 4 ++-- lib/Sema/CSDiagnostics.cpp | 6 ++++-- lib/Sema/CSGen.cpp | 4 ++-- lib/Sema/CSSimplify.cpp | 4 ++-- lib/Sema/ConstraintSystem.cpp | 12 +++++++++++ lib/Sema/PreCheckExpr.cpp | 27 +++++++++++++++++------ lib/Sema/TypeCheckCodeCompletion.cpp | 16 ++++++++++---- lib/Sema/TypeCheckConstraints.cpp | 31 ++++++++++++--------------- 11 files changed, 84 insertions(+), 52 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index c149899082ea8..4519575a4d112 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5014,7 +5014,8 @@ class ConstraintSystem { /// \param replaceInvalidRefsWithErrors Indicates whether it's allowed /// to replace any discovered invalid member references with `ErrorExpr`. static bool preCheckExpression(Expr *&expr, DeclContext *dc, - bool replaceInvalidRefsWithErrors); + bool replaceInvalidRefsWithErrors, + bool leaveClosureBodiesUnchecked); /// Solve the system of constraints generated from provided target. /// @@ -5245,6 +5246,16 @@ class ConstraintSystem { /// imported from C/ObjectiveC. bool isArgumentOfImportedDecl(ConstraintLocatorBuilder locator); + /// Check whether given closure should participate in inference e.g. + /// if it's a single-expression closure - it always does, but + /// multi-statement closures require special flags. + bool participatesInInference(ClosureExpr *closure) const; + + /// Visit each subexpression that will be part of the constraint system + /// of the given expression, including those in closure bodies that will be + /// part of the constraint system. + void forEachExpr(Expr *expr, llvm::function_ref callback); + SWIFT_DEBUG_DUMP; SWIFT_DEBUG_DUMPER(dump(Expr *)); @@ -6069,18 +6080,6 @@ BraceStmt *applyResultBuilderTransform( constraints::SolutionApplicationTarget)> rewriteTarget); -/// Determine whether the given closure expression should be type-checked -/// within the context of its enclosing expression. Otherwise, it will be -/// separately type-checked once its enclosing expression has determined all -/// of the parameter and result types without looking at the body. -bool shouldTypeCheckInEnclosingExpression(ClosureExpr *expr); - -/// Visit each subexpression that will be part of the constraint system -/// of the given expression, including those in closure bodies that will be -/// part of the constraint system. -void forEachExprInConstraintSystem( - Expr *expr, llvm::function_ref callback); - /// Whether the given parameter requires an argument. bool parameterRequiresArgument( ArrayRef params, diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index 87c65e0927f52..787b0a574bb94 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -1932,7 +1932,9 @@ class PreCheckResultBuilderApplication : public ASTWalker { DiagnosticTransaction transaction(diagEngine); HasError |= ConstraintSystem::preCheckExpression( - E, DC, /*replaceInvalidRefsWithErrors=*/true); + E, DC, /*replaceInvalidRefsWithErrors=*/true, + /*leaveClosureBodiesUnchecked=*/true); + HasError |= transaction.hasErrors(); if (!HasError) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index f601c938cf946..e249cf7b52ac9 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -4056,8 +4056,7 @@ namespace { if (expr->isLiteralInit()) { auto *literalInit = expr->getSubExpr(); if (auto *call = dyn_cast(literalInit)) { - forEachExprInConstraintSystem(call->getFn(), - [&](Expr *subExpr) -> Expr * { + cs.forEachExpr(call->getFn(), [&](Expr *subExpr) -> Expr * { auto *TE = dyn_cast(subExpr); if (!TE) return subExpr; diff --git a/lib/Sema/CSClosure.cpp b/lib/Sema/CSClosure.cpp index 63facbf5596c6..cd1dd95568477 100644 --- a/lib/Sema/CSClosure.cpp +++ b/lib/Sema/CSClosure.cpp @@ -854,7 +854,7 @@ class ClosureConstraintGenerator bool isSupportedMultiStatementClosure() const { return !closure->hasSingleExpressionBody() && - shouldTypeCheckInEnclosingExpression(closure); + cs.participatesInInference(closure); } #define UNSUPPORTED_STMT(STMT) void visit##STMT##Stmt(STMT##Stmt *) { \ @@ -885,7 +885,7 @@ class ClosureConstraintGenerator bool ConstraintSystem::generateConstraints(ClosureExpr *closure) { auto &ctx = closure->getASTContext(); - if (shouldTypeCheckInEnclosingExpression(closure)) { + if (participatesInInference(closure)) { ClosureConstraintGenerator generator(*this, closure, getConstraintLocator(closure)); generator.visit(closure->getBody()); diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 0b13a8408531a..25236fdcd6ce7 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -466,7 +466,8 @@ bool MissingConformanceFailure::diagnoseAsError() { } bool hasFix = false; - forEachExprInConstraintSystem(caseExpr, [&](Expr *expr) -> Expr * { + auto &cs = getConstraintSystem(); + cs.forEachExpr(caseExpr, [&](Expr *expr) -> Expr * { hasFix |= anchors.count(expr); return hasFix ? nullptr : expr; }); @@ -3594,7 +3595,8 @@ bool MissingMemberFailure::diagnoseAsError() { bool hasUnresolvedPattern = false; if (auto *E = getAsExpr(anchor)) { - forEachExprInConstraintSystem(const_cast(E), [&](Expr *expr) { + auto &cs = getConstraintSystem(); + cs.forEachExpr(const_cast(E), [&](Expr *expr) { hasUnresolvedPattern |= isa(expr); return hasUnresolvedPattern ? nullptr : expr; }); diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index db3e6df029f8c..f91d6159a1450 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -2105,7 +2105,7 @@ namespace { // If this is a multi-statement closure, let's mark result // as potential hole right away. return Type(CS.createTypeVariable( - resultLocator, shouldTypeCheckInEnclosingExpression(closure) + resultLocator, CS.participatesInInference(closure) ? 0 : TVO_CanBindToHole)); }(); @@ -2627,7 +2627,7 @@ namespace { // genreation only if closure is going to participate // in the type-check. This allows us to delay validation of // multi-statement closures until body is opened. - if (shouldTypeCheckInEnclosingExpression(closure) && + if (CS.participatesInInference(closure) && collectVarRefs.hasErrorExprs) { return Type(); } diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 3498659d3b5a6..4725f3cdb6164 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -2040,7 +2040,7 @@ static bool fixMissingArguments(ConstraintSystem &cs, ASTNode anchor, // Something like `foo { x in }` or `foo { $0 }` if (auto *closure = getAsExpr(anchor)) { - forEachExprInConstraintSystem(closure, [&](Expr *expr) -> Expr * { + cs.forEachExpr(closure, [&](Expr *expr) -> Expr * { if (auto *UDE = dyn_cast(expr)) { if (!isParam(UDE->getBase())) return expr; @@ -11662,7 +11662,7 @@ bool ConstraintSystem::recordFix(ConstraintFix *fix, unsigned impact) { bool found = false; if (auto *expr = getAsExpr(anchor)) { - forEachExprInConstraintSystem(expr, [&](Expr *subExpr) -> Expr * { + forEachExpr(expr, [&](Expr *subExpr) -> Expr * { found |= anchors.count(subExpr); return subExpr; }); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 241a51a6d1005..8dcab13e39af1 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -5874,6 +5874,18 @@ bool ConstraintSystem::isReadOnlyKeyPathComponent( return false; } +bool ConstraintSystem::participatesInInference(ClosureExpr *closure) const { + if (closure->hasSingleExpressionBody()) + return true; + + if (Options.contains(ConstraintSystemFlags::LeaveClosureBodyUnchecked)) + return false; + + auto &ctx = closure->getASTContext(); + return !closure->hasEmptyBody() && + ctx.TypeCheckerOpts.EnableMultiStatementClosureInference; +} + TypeVarBindingProducer::TypeVarBindingProducer(BindingSet &bindings) : BindingProducer(bindings.getConstraintSystem(), bindings.getTypeVariable()->getImpl().getLocator()), diff --git a/lib/Sema/PreCheckExpr.cpp b/lib/Sema/PreCheckExpr.cpp index 0175cfb12cb33..c982749d8b2fa 100644 --- a/lib/Sema/PreCheckExpr.cpp +++ b/lib/Sema/PreCheckExpr.cpp @@ -893,6 +893,8 @@ namespace { /// implicit `ErrorExpr` in place of invalid references. bool UseErrorExprs; + bool LeaveClosureBodiesUnchecked; + /// A stack of expressions being walked, used to determine where to /// insert RebindSelfInConstructorExpr nodes. llvm::SmallVector ExprStack; @@ -1080,9 +1082,11 @@ namespace { public: PreCheckExpression(DeclContext *dc, Expr *parent, - bool replaceInvalidRefsWithErrors) - : Ctx(dc->getASTContext()), DC(dc), ParentExpr(parent), - UseErrorExprs(replaceInvalidRefsWithErrors) {} + bool replaceInvalidRefsWithErrors, + bool leaveClosureBodiesUnchecked) + : Ctx(dc->getASTContext()), DC(dc), + ParentExpr(parent), UseErrorExprs(replaceInvalidRefsWithErrors), + LeaveClosureBodiesUnchecked(leaveClosureBodiesUnchecked) {} ASTContext &getASTContext() const { return Ctx; } @@ -1428,8 +1432,13 @@ namespace { /// true when we want the body to be considered part of this larger expression. bool PreCheckExpression::walkToClosureExprPre(ClosureExpr *closure) { // If we won't be checking the body of the closure, don't walk into it here. - if (!shouldTypeCheckInEnclosingExpression(closure)) - return false; + if (!closure->hasSingleExpressionBody()) { + if (LeaveClosureBodiesUnchecked) + return false; + + if (!Ctx.TypeCheckerOpts.EnableMultiStatementClosureInference) + return false; + } // Update the current DeclContext to be the closure we're about to // recurse into. @@ -2082,11 +2091,15 @@ Expr *PreCheckExpression::simplifyTypeConstructionWithLiteralArg(Expr *E) { /// Pre-check the expression, validating any types that occur in the /// expression and folding sequence expressions. bool ConstraintSystem::preCheckExpression(Expr *&expr, DeclContext *dc, - bool replaceInvalidRefsWithErrors) { + bool replaceInvalidRefsWithErrors, + bool leaveClosureBodiesUnchecked) { auto &ctx = dc->getASTContext(); FrontendStatsTracer StatsTracer(ctx.Stats, "precheck-expr", expr); - PreCheckExpression preCheck(dc, expr, replaceInvalidRefsWithErrors); + PreCheckExpression preCheck(dc, expr, + replaceInvalidRefsWithErrors, + leaveClosureBodiesUnchecked); + // Perform the pre-check. if (auto result = expr->walk(preCheck)) { expr = result; diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp index 9e06aba71278b..6ac5c821240c6 100644 --- a/lib/Sema/TypeCheckCodeCompletion.cpp +++ b/lib/Sema/TypeCheckCodeCompletion.cpp @@ -206,7 +206,9 @@ class SanitizeExpr : public ASTWalker { // If this is a closure, only walk into its children if they // are type-checked in the context of the enclosing expression. if (auto closure = dyn_cast(expr)) { - if (!shouldTypeCheckInEnclosingExpression(closure)) + // TODO: This has to be deleted once `EnableMultiStatementClosureInference` + // is enabled by default. + if (!closure->hasSingleExpressionBody()) return { false, expr }; for (auto &Param : *closure->getParameters()) { Param->setSpecifier(swift::ParamSpecifier::Default); @@ -305,8 +307,11 @@ getTypeOfExpressionWithoutApplying(Expr *&expr, DeclContext *dc, PrettyStackTraceExpr stackTrace(Context, "type-checking", expr); referencedDecl = nullptr; + ConstraintSystemOptions options; + options |= ConstraintSystemFlags::SuppressDiagnostics; + // Construct a constraint system from this expression. - ConstraintSystem cs(dc, ConstraintSystemFlags::SuppressDiagnostics); + ConstraintSystem cs(dc, options); // Attempt to solve the constraint system. const Type originalType = expr->getType(); @@ -788,7 +793,8 @@ bool TypeChecker::typeCheckForCodeCompletion( // expression and folding sequence expressions. auto failedPreCheck = ConstraintSystem::preCheckExpression( expr, DC, - /*replaceInvalidRefsWithErrors=*/true); + /*replaceInvalidRefsWithErrors=*/true, + /*leaveClosureBodiesUnchecked=*/true); target.setExpr(expr); @@ -892,7 +898,9 @@ static Optional getTypeOfCompletionContextExpr( Expr *&parsedExpr, ConcreteDeclRef &referencedDecl) { if (constraints::ConstraintSystem::preCheckExpression( - parsedExpr, DC, /*replaceInvalidRefsWithErrors=*/true)) + parsedExpr, DC, + /*replaceInvalidRefsWithErrors=*/true, + /*leaveClosureBodiesUnchecked=*/true)) return None; switch (kind) { diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 3378abd1b4f92..d25cd0c473a4a 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -344,7 +344,8 @@ TypeChecker::typeCheckExpression( // First, pre-check the expression, validating any types that occur in the // expression and folding sequence expressions. if (ConstraintSystem::preCheckExpression( - expr, dc, /*replaceInvalidRefsWithErrors=*/true)) { + expr, dc, /*replaceInvalidRefsWithErrors=*/true, + options.contains(TypeCheckExprFlags::LeaveClosureBodyUnchecked))) { target.setExpr(expr); return None; } @@ -588,14 +589,17 @@ bool TypeChecker::typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt) { // Precheck the sequence. Expr *sequence = stmt->getSequence(); if (ConstraintSystem::preCheckExpression( - sequence, dc, /*replaceInvalidRefsWithErrors=*/true)) + sequence, dc, /*replaceInvalidRefsWithErrors=*/true, + /*leaveClosureBodiesUnchecked=*/false)) return failed(); stmt->setSequence(sequence); // Precheck the filtering condition. if (Expr *whereExpr = stmt->getWhere()) { if (ConstraintSystem::preCheckExpression( - whereExpr, dc, /*replaceInvalidRefsWithErrors=*/true)) + whereExpr, dc, + /*replaceInvalidRefsWithErrors=*/true, + /*leaveClosureBodiesUnchecked=*/false)) return failed(); stmt->setWhere(whereExpr); @@ -2052,26 +2056,19 @@ HasDynamicCallableAttributeRequest::evaluate(Evaluator &evaluator, }); } -bool swift::shouldTypeCheckInEnclosingExpression(ClosureExpr *expr) { - if (expr->hasSingleExpressionBody()) - return true; - - auto &ctx = expr->getASTContext(); - return !expr->hasEmptyBody() && - ctx.TypeCheckerOpts.EnableMultiStatementClosureInference; -} - -void swift::forEachExprInConstraintSystem( +void ConstraintSystem::forEachExpr( Expr *expr, llvm::function_ref callback) { struct ChildWalker : ASTWalker { + ConstraintSystem &CS; llvm::function_ref callback; - ChildWalker(llvm::function_ref callback) - : callback(callback) {} + ChildWalker(ConstraintSystem &CS, + llvm::function_ref callback) + : CS(CS), callback(callback) {} std::pair walkToExprPre(Expr *E) override { if (auto closure = dyn_cast(E)) { - if (!shouldTypeCheckInEnclosingExpression(closure)) + if (!CS.participatesInInference(closure)) return { false, callback(E) }; } return { true, callback(E) }; @@ -2084,5 +2081,5 @@ void swift::forEachExprInConstraintSystem( bool walkToTypeReprPre(TypeRepr *T) override { return false; } }; - expr->walk(ChildWalker(callback)); + expr->walk(ChildWalker(*this, callback)); } From d0fc5806231075cb6649b7fbfea64ccfa4720af0 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 27 Oct 2021 16:52:03 -0700 Subject: [PATCH 05/14] [IDE] Skip complex closures while checking pattern bindings This preserves previous behavior where multi-statement closures where always type-checked without context. --- include/swift/Sema/IDETypeChecking.h | 3 ++- lib/IDE/ExprContextAnalysis.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/swift/Sema/IDETypeChecking.h b/include/swift/Sema/IDETypeChecking.h index a1447932de11e..28118f05b4b61 100644 --- a/include/swift/Sema/IDETypeChecking.h +++ b/include/swift/Sema/IDETypeChecking.h @@ -47,7 +47,8 @@ namespace swift { struct PrintOptions; /// Typecheck binding initializer at \p bindingIndex. - void typeCheckPatternBinding(PatternBindingDecl *PBD, unsigned bindingIndex); + void typeCheckPatternBinding(PatternBindingDecl *PBD, unsigned bindingIndex, + bool leaveClosureBodiesUnchecked); /// Check if T1 is convertible to T2. /// diff --git a/lib/IDE/ExprContextAnalysis.cpp b/lib/IDE/ExprContextAnalysis.cpp index 653510d04cd24..f6fe5e3dff3cc 100644 --- a/lib/IDE/ExprContextAnalysis.cpp +++ b/lib/IDE/ExprContextAnalysis.cpp @@ -98,7 +98,8 @@ void swift::ide::typeCheckContextAt(DeclContext *DC, SourceLoc Loc) { [](VarDecl *VD) { (void)VD->getInterfaceType(); }); if (PBD->getInit(i)) { if (!PBD->isInitializerChecked(i)) - typeCheckPatternBinding(PBD, i); + typeCheckPatternBinding(PBD, i, + /*LeaveClosureBodyUnchecked=*/true); } } } else if (auto *defaultArg = dyn_cast(DC)) { From 77ab650f59115886c924371865ef0bac7e13da97 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 28 Oct 2021 11:14:44 -0700 Subject: [PATCH 06/14] [TypeChecker/Constness] Diagnostics: Walk into multi-statement closures when inference is enabled When multi-statement closure inference is enabled it's body is type-checked together with enclosing context, so they could be walked directly just like single-expressions ones. --- lib/Sema/ConstantnessSemaDiagnostics.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/Sema/ConstantnessSemaDiagnostics.cpp b/lib/Sema/ConstantnessSemaDiagnostics.cpp index 88b9a74be5470..ce786b1b517d8 100644 --- a/lib/Sema/ConstantnessSemaDiagnostics.cpp +++ b/lib/Sema/ConstantnessSemaDiagnostics.cpp @@ -359,10 +359,13 @@ void swift::diagnoseConstantArgumentRequirement( } std::pair walkToClosureExprPre(ClosureExpr *closure) { - if (closure->hasSingleExpressionBody()) { - // Single expression closure bodies are not visited directly - // by the ASTVisitor, so we must descend into the body manually - // and set the DeclContext to that of the closure. + auto &ctx = DC->getASTContext(); + + if (closure->hasSingleExpressionBody() || + ctx.TypeCheckerOpts.EnableMultiStatementClosureInference) { + // Closure bodies are not visited directly by the ASTVisitor, + // so we must descend into the body manuall and set the + // DeclContext to that of the closure. DC = closure; return {true, closure}; } From 248316536b6cee993c0f212d42c94d2459ffad0f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 28 Oct 2021 15:53:22 -0700 Subject: [PATCH 07/14] [CSClosure] Warn about `defer` being the last element in the closure body --- lib/Sema/CSClosure.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/Sema/CSClosure.cpp b/lib/Sema/CSClosure.cpp index cd1dd95568477..8154d445147fb 100644 --- a/lib/Sema/CSClosure.cpp +++ b/lib/Sema/CSClosure.cpp @@ -1246,6 +1246,18 @@ class ClosureConstraintApplication ASTNode visitBraceStmt(BraceStmt *braceStmt) { auto &cs = solution.getConstraintSystem(); + // Diagnose defer statement being last one in block. + if (!braceStmt->empty()) { + if (auto stmt = braceStmt->getLastElement().dyn_cast()) { + if (auto deferStmt = dyn_cast(stmt)) { + auto &diags = closure->getASTContext().Diags; + diags + .diagnose(deferStmt->getStartLoc(), diag::defer_stmt_at_block_end) + .fixItReplace(deferStmt->getStartLoc(), "do"); + } + } + } + for (auto &node : braceStmt->getElements()) { if (auto expr = node.dyn_cast()) { // Rewrite the expression. From 9bd603bfc9777cee2f6709deefbc783b3975d098 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 28 Oct 2021 17:08:52 -0700 Subject: [PATCH 08/14] [Diagnostics] Apply "unhandled throw" diagnostic for `for-in` loop in closures Extract diagnostic into a method and use it while type-checking `for-in` in top-level code and in closures. --- lib/Sema/CSClosure.cpp | 6 ++++++ lib/Sema/MiscDiagnostics.cpp | 30 ++++++++++++++++++++++++++++++ lib/Sema/MiscDiagnostics.h | 7 +++++++ lib/Sema/TypeCheckConstraints.cpp | 22 ++++------------------ 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/lib/Sema/CSClosure.cpp b/lib/Sema/CSClosure.cpp index 8154d445147fb..2a64de40b55c9 100644 --- a/lib/Sema/CSClosure.cpp +++ b/lib/Sema/CSClosure.cpp @@ -16,6 +16,7 @@ // //===----------------------------------------------------------------------===// +#include "MiscDiagnostics.h" #include "TypeChecker.h" #include "swift/Sema/ConstraintSystem.h" @@ -1163,12 +1164,17 @@ class ClosureConstraintApplication auto forEachTarget = rewriteTarget(*cs.getSolutionApplicationTarget(forEachStmt)); + if (!forEachTarget) hadError = true; auto body = visit(forEachStmt->getBody()).get(); forEachStmt->setBody(cast(body)); + // Check to see if the sequence expr is throwing (in async context), + // if so require the stmt to have a `try`. + hadError |= diagnoseUnhandledThrowsInAsyncContext(closure, forEachStmt); + return forEachStmt; } diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index ce3784cd8eda7..df71f0735dc17 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -5140,3 +5140,33 @@ Optional TypeChecker::omitNeedlessWords(VarDecl *var) { return None; } + +bool swift::diagnoseUnhandledThrowsInAsyncContext(DeclContext *dc, + ForEachStmt *forEach) { + if (!forEach->getAwaitLoc().isValid()) + return false; + + auto &ctx = dc->getASTContext(); + + auto sequenceProto = TypeChecker::getProtocol( + ctx, forEach->getForLoc(), KnownProtocolKind::AsyncSequence); + + if (!sequenceProto) + return false; + + // fetch the sequence out of the statement + // else wise the value is potentially unresolved + auto Ty = forEach->getSequence()->getType(); + auto module = dc->getParentModule(); + auto conformanceRef = module->lookupConformance(Ty, sequenceProto); + + if (conformanceRef.hasEffect(EffectKind::Throws) && + forEach->getTryLoc().isInvalid()) { + ctx.Diags + .diagnose(forEach->getAwaitLoc(), diag::throwing_call_unhandled, "call") + .fixItInsert(forEach->getAwaitLoc(), "try"); + return true; + } + + return false; +} diff --git a/lib/Sema/MiscDiagnostics.h b/lib/Sema/MiscDiagnostics.h index 10e847af16496..a5a5794990974 100644 --- a/lib/Sema/MiscDiagnostics.h +++ b/lib/Sema/MiscDiagnostics.h @@ -32,6 +32,7 @@ namespace swift { class Stmt; class TopLevelCodeDecl; class ValueDecl; + class ForEachStmt; /// Emit diagnostics for syntactic restrictions on a given expression. void performSyntacticExprDiagnostics( @@ -115,6 +116,12 @@ void fixItEncloseTrailingClosure(ASTContext &ctx, /// we emit a diagnostic suggesting the async call. void checkFunctionAsyncUsage(AbstractFunctionDecl *decl); void checkPatternBindingDeclAsyncUsage(PatternBindingDecl *decl); + +/// Detect and diagnose a missing `try` in `for-in` loop sequence +/// expression in async context (denoted with `await` keyword). +bool diagnoseUnhandledThrowsInAsyncContext(DeclContext *dc, + ForEachStmt *forEach); + } // namespace swift #endif // SWIFT_SEMA_MISC_DIAGNOSTICS_H diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index d25cd0c473a4a..d66afc7652089 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -610,24 +610,10 @@ bool TypeChecker::typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt) { if (!typeCheckExpression(target)) return failed(); - // check to see if the sequence expr is throwing (and async), if so require - // the stmt to have a try loc - if (stmt->getAwaitLoc().isValid()) { - // fetch the sequence out of the statement - // else wise the value is potentially unresolved - auto Ty = stmt->getSequence()->getType(); - auto module = dc->getParentModule(); - auto conformanceRef = module->lookupConformance(Ty, sequenceProto); - - if (conformanceRef.hasEffect(EffectKind::Throws) && - stmt->getTryLoc().isInvalid()) { - auto &diags = dc->getASTContext().Diags; - diags.diagnose(stmt->getAwaitLoc(), diag::throwing_call_unhandled, "call") - .fixItInsert(stmt->getAwaitLoc(), "try"); - - return failed(); - } - } + // Check to see if the sequence expr is throwing (in async context), + // if so require the stmt to have a `try`. + if (diagnoseUnhandledThrowsInAsyncContext(dc, stmt)) + return failed(); return false; } From dcf80e62d04c931cc3c49ef1207d3c0fa396fdd8 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 29 Oct 2021 10:09:48 -0700 Subject: [PATCH 09/14] [PreCheck] Avoid patterns that appear in closures when multi-statement inference is enabled Scope down previous check to avoid walking into patterns that appear in multi-statement closures if the inference is enabled. --- lib/Sema/PreCheckExpr.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/Sema/PreCheckExpr.cpp b/lib/Sema/PreCheckExpr.cpp index c982749d8b2fa..7d14b83f5caed 100644 --- a/lib/Sema/PreCheckExpr.cpp +++ b/lib/Sema/PreCheckExpr.cpp @@ -1419,11 +1419,12 @@ namespace { std::pair walkToPatternPre(Pattern *pattern) override { // With multi-statement closure inference enabled, constraint generation - // is responsible for pattern verification and type-checking, so there - // is no need to walk into patterns in that mode. - bool shouldWalkIntoPatterns = - !Ctx.TypeCheckerOpts.EnableMultiStatementClosureInference; - return {shouldWalkIntoPatterns, pattern}; + // is responsible for pattern verification and type-checking in the body + // of the closure, so there is no need to walk into patterns. + bool walkIntoPatterns = + !(isa(DC) && + Ctx.TypeCheckerOpts.EnableMultiStatementClosureInference); + return {walkIntoPatterns, pattern}; } }; } // end anonymous namespace From f258a77dfbcc1273b2c89446a07970aa0b89a59e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 29 Oct 2021 10:13:19 -0700 Subject: [PATCH 10/14] [Sema/CodeCompletion] Leave complex closure bodies unchecked Preserve pre SE-0326 for code completion, so it could be ported gradually. --- lib/Sema/TypeCheckCodeCompletion.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp index 6ac5c821240c6..5ec1a944b34c8 100644 --- a/lib/Sema/TypeCheckCodeCompletion.cpp +++ b/lib/Sema/TypeCheckCodeCompletion.cpp @@ -309,6 +309,7 @@ getTypeOfExpressionWithoutApplying(Expr *&expr, DeclContext *dc, ConstraintSystemOptions options; options |= ConstraintSystemFlags::SuppressDiagnostics; + options |= ConstraintSystemFlags::LeaveClosureBodyUnchecked; // Construct a constraint system from this expression. ConstraintSystem cs(dc, options); @@ -403,6 +404,7 @@ getTypeOfCompletionOperatorImpl(DeclContext *DC, Expr *expr, ConstraintSystemOptions options; options |= ConstraintSystemFlags::SuppressDiagnostics; options |= ConstraintSystemFlags::ReusePrecheckedType; + options |= ConstraintSystemFlags::LeaveClosureBodyUnchecked; // Construct a constraint system from this expression. ConstraintSystem CS(DC, options); @@ -810,6 +812,8 @@ bool TypeChecker::typeCheckForCodeCompletion( options |= ConstraintSystemFlags::AllowFixes; options |= ConstraintSystemFlags::SuppressDiagnostics; options |= ConstraintSystemFlags::ForCodeCompletion; + options |= ConstraintSystemFlags::LeaveClosureBodyUnchecked; + ConstraintSystem cs(DC, options); @@ -980,7 +984,9 @@ bool swift::typeCheckExpression(DeclContext *DC, Expr *&parsedExpr) { parsedExpr = parsedExpr->walk(SanitizeExpr(ctx, /*shouldReusePrecheckedType=*/false)); DiagnosticSuppression suppression(ctx.Diags); - auto resultTy = TypeChecker::typeCheckExpression(parsedExpr, DC); + auto resultTy = TypeChecker::typeCheckExpression( + parsedExpr, DC, + /*contextualInfo=*/{}, TypeCheckExprFlags::LeaveClosureBodyUnchecked); return !resultTy; } From 4cd404d0207866dd219013d0445027ef32c101b6 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 29 Oct 2021 18:10:36 -0700 Subject: [PATCH 11/14] [MiscDiagnostics] Introduce a base class for diagnostic walkers Put some common logic related to local declaration to the base class and refactor other walkers to use it instead of `ASTWalker`. --- lib/Sema/MiscDiagnostics.cpp | 58 +++++++++++++++++------------- lib/Sema/MiscDiagnostics.h | 14 ++++++++ lib/Sema/TypeCheckAvailability.cpp | 3 +- 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index df71f0735dc17..dc3ef7e334c36 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -24,6 +24,7 @@ #include "swift/AST/NameLookupRequests.h" #include "swift/AST/Pattern.h" #include "swift/AST/SourceFile.h" +#include "swift/AST/Stmt.h" #include "swift/AST/TypeCheckRequests.h" #include "swift/Basic/Defer.h" #include "swift/Basic/SourceManager.h" @@ -50,6 +51,37 @@ static Expr *isImplicitPromotionToOptional(Expr *E) { return nullptr; } +bool BaseDiagnosticWalker::walkToDeclPre(Decl *D) { + return isa(D->getDeclContext()) + ? shouldWalkIntoDeclInClosureContext(D) + : false; +} + +bool BaseDiagnosticWalker::shouldWalkIntoDeclInClosureContext(Decl *D) { + auto *closure = dyn_cast(D->getDeclContext()); + assert(closure); + + if (closure->isSeparatelyTypeChecked()) + return false; + + auto &opts = D->getASTContext().TypeCheckerOpts; + + // If multi-statement inference is enabled, let's not walk + // into declarations contained in a multi-statement closure + // because they'd be handled via `typeCheckDecl` that runs + // syntactic diagnostics. + if (opts.EnableMultiStatementClosureInference && + !closure->hasSingleExpressionBody()) { + // Since pattern bindings get their types through solution application, + // `typeCheckDecl` doesn't touch initializers (because they are already + // fully type-checked), so pattern bindings have to be allowed to be + // walked to diagnose syntactic issues. + return isa(D); + } + + return true; +} + /// Diagnose syntactic restrictions of expressions. /// /// - Module values may only occur as part of qualification. @@ -72,7 +104,7 @@ static Expr *isImplicitPromotionToOptional(Expr *E) { /// static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC, bool isExprStmt) { - class DiagnoseWalker : public ASTWalker { + class DiagnoseWalker : public BaseDiagnosticWalker { SmallPtrSet AlreadyDiagnosedMetatypes; SmallPtrSet AlreadyDiagnosedBitCasts; @@ -89,18 +121,8 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC, return { false, P }; } - bool walkToDeclPre(Decl *D) override { - if (auto *closure = dyn_cast(D->getDeclContext())) - return !closure->isSeparatelyTypeChecked(); - return false; - } - bool walkToTypeReprPre(TypeRepr *T) override { return true; } - bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override { - return false; - } - bool shouldWalkCaptureInitializerExpressions() override { return true; } bool shouldWalkIntoTapExpression() override { return false; } @@ -1449,7 +1471,7 @@ static void diagRecursivePropertyAccess(const Expr *E, const DeclContext *DC) { /// confusion, so we force an explicit self. static void diagnoseImplicitSelfUseInClosure(const Expr *E, const DeclContext *DC) { - class DiagnoseWalker : public ASTWalker { + class DiagnoseWalker : public BaseDiagnosticWalker { ASTContext &Ctx; SmallVector Closures; public: @@ -1530,18 +1552,6 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E, return true; } - - // Don't walk into nested decls. - bool walkToDeclPre(Decl *D) override { - if (auto *closure = dyn_cast(D->getDeclContext())) - return !closure->isSeparatelyTypeChecked(); - return false; - } - - bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override { - return false; - } - bool shouldWalkCaptureInitializerExpressions() override { return true; } bool shouldWalkIntoTapExpression() override { return false; } diff --git a/lib/Sema/MiscDiagnostics.h b/lib/Sema/MiscDiagnostics.h index a5a5794990974..2a34e62a2f47a 100644 --- a/lib/Sema/MiscDiagnostics.h +++ b/lib/Sema/MiscDiagnostics.h @@ -13,6 +13,7 @@ #ifndef SWIFT_SEMA_MISC_DIAGNOSTICS_H #define SWIFT_SEMA_MISC_DIAGNOSTICS_H +#include "swift/AST/ASTWalker.h" #include "swift/AST/AttrKind.h" #include "swift/AST/Pattern.h" #include "swift/AST/Expr.h" @@ -26,7 +27,9 @@ namespace swift { class AbstractFunctionDecl; class ApplyExpr; class CallExpr; + class ClosureExpr; class DeclContext; + class Decl; class Expr; class InFlightDiagnostic; class Stmt; @@ -122,6 +125,17 @@ void checkPatternBindingDeclAsyncUsage(PatternBindingDecl *decl); bool diagnoseUnhandledThrowsInAsyncContext(DeclContext *dc, ForEachStmt *forEach); +class BaseDiagnosticWalker : public ASTWalker { + bool walkToDeclPre(Decl *D) override; + + bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override { + return false; + } + +private: + static bool shouldWalkIntoDeclInClosureContext(Decl *D); +}; + } // namespace swift #endif // SWIFT_SEMA_MISC_DIAGNOSTICS_H diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index a7d4967898a67..3fd5727681c19 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -3297,7 +3297,7 @@ void swift::diagnoseExprAvailability(const Expr *E, DeclContext *DC) { namespace { -class StmtAvailabilityWalker : public ASTWalker { +class StmtAvailabilityWalker : public BaseDiagnosticWalker { DeclContext *DC; bool WalkRecursively; @@ -3333,7 +3333,6 @@ class StmtAvailabilityWalker : public ASTWalker { return std::make_pair(true, P); } }; - } void swift::diagnoseStmtAvailability(const Stmt *S, DeclContext *DC, From d7984f4453d1abb347aa736dee228bd69b50ed67 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 29 Oct 2021 19:58:39 -0700 Subject: [PATCH 12/14] [ConstraintSystem] Attempt conjunction before closure result or generic parameter holes Closure result type or generic parameter associated with such a location could bw inferred from a body of a multi-statement closure (when inference is enabled), so we need to give closure a chance to run before attemtping a hole for such positions in diagnostic mode. --- include/swift/Sema/CSBindings.h | 8 ++++++++ lib/Sema/CSBindings.cpp | 8 ++++++++ lib/Sema/CSStep.cpp | 13 +++++++++++++ 3 files changed, 29 insertions(+) diff --git a/include/swift/Sema/CSBindings.h b/include/swift/Sema/CSBindings.h index 62511ea90bfe2..1ec0520d303a0 100644 --- a/include/swift/Sema/CSBindings.h +++ b/include/swift/Sema/CSBindings.h @@ -338,6 +338,14 @@ class BindingSet { TypeVariableType *getTypeVariable() const { return Info.TypeVar; } + /// Check whether this binding set belongs to a type variable + /// that represents a result type of a closure. + bool forClosureResult() const; + + /// Check whether this binding set belongs to a type variable + /// that represents a generic parameter. + bool forGenericParameter() const; + bool canBeNil() const; /// If this type variable doesn't have any viable bindings, or diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 8d9c4e856f88e..ca318a2eb4f91 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -25,6 +25,14 @@ using namespace swift; using namespace constraints; using namespace inference; +bool BindingSet::forClosureResult() const { + return Info.TypeVar->getImpl().isClosureResultType(); +} + +bool BindingSet::forGenericParameter() const { + return bool(Info.TypeVar->getImpl().getGenericParameter()); +} + bool BindingSet::canBeNil() const { auto &ctx = CS.getASTContext(); return Literals.count( diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index 5ba760413c4a7..d534fbec95144 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -345,6 +345,19 @@ StepResult ComponentStep::take(bool prevFailed) { auto *disjunction = CS.selectDisjunction(); auto bestBindings = CS.determineBestBindings(); + if (CS.shouldAttemptFixes()) { + if ((bestBindings && + (bestBindings->forClosureResult() || + bestBindings->forGenericParameter()) && + bestBindings->isHole()) && + !disjunction) { + if (auto *conjunction = CS.selectConjunction()) { + return suspend( + std::make_unique(CS, conjunction, Solutions)); + } + } + } + if (bestBindings && (!disjunction || bestBindings->favoredOverDisjunction(disjunction))) { // Produce a type variable step. From aef83067470a94cc8d5f5059b53ae42fdfa1fe09 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 1 Nov 2021 13:20:17 -0700 Subject: [PATCH 13/14] [TypeChecker] Teach declaration type-checking about `LeaveClosureBodyUnchecked` flag Propagate `LeaveClosureBodyUnchecked` flag from `typeCheckASTNodeAtLoc` down to declaration checker to skip checking closures associated with pattern binding entry initializers. This is no-op until multi-statement inference becomes enabled by default. --- lib/Sema/TypeCheckConstraints.cpp | 16 +++++++++------- lib/Sema/TypeCheckDeclPrimary.cpp | 19 +++++++++++++++---- lib/Sema/TypeCheckStmt.cpp | 2 +- lib/Sema/TypeChecker.cpp | 13 +++++++++---- lib/Sema/TypeChecker.h | 8 +++++--- 5 files changed, 39 insertions(+), 19 deletions(-) diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index d66afc7652089..60fef699d7b4e 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -431,9 +431,11 @@ Type TypeChecker::typeCheckParameterDefault(Expr *&defaultValue, : CTP_DefaultParameter}); } -bool TypeChecker::typeCheckBinding( - Pattern *&pattern, Expr *&initializer, DeclContext *DC, - Type patternType, PatternBindingDecl *PBD, unsigned patternNumber) { +bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer, + DeclContext *DC, Type patternType, + PatternBindingDecl *PBD, + unsigned patternNumber, + TypeCheckExprOptions options) { SolutionApplicationTarget target = PBD ? SolutionApplicationTarget::forInitialization( initializer, DC, patternType, PBD, patternNumber, @@ -442,7 +444,6 @@ bool TypeChecker::typeCheckBinding( initializer, DC, patternType, pattern, /*bindPatternVarsOneWay=*/false); - auto options = TypeCheckExprOptions(); if (DC->getASTContext().LangOpts.CheckAPIAvailabilityOnly && PBD && !DC->getAsDecl()) { // Skip checking the initializer for non-public decls when @@ -491,7 +492,8 @@ bool TypeChecker::typeCheckBinding( bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD, unsigned patternNumber, - Type patternType) { + Type patternType, + TypeCheckExprOptions options) { Pattern *pattern = PBD->getPattern(patternNumber); Expr *init = PBD->getInit(patternNumber); @@ -520,8 +522,8 @@ bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD, } } - bool hadError = TypeChecker::typeCheckBinding( - pattern, init, DC, patternType, PBD, patternNumber); + bool hadError = TypeChecker::typeCheckBinding(pattern, init, DC, patternType, + PBD, patternNumber, options); if (!init) { PBD->setInvalid(); return true; diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index e1b44b6f15cc3..d035442ac04d9 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -1670,7 +1670,12 @@ class DeclChecker : public DeclVisitor { ASTContext &Ctx; SourceFile *SF; - explicit DeclChecker(ASTContext &ctx, SourceFile *SF) : Ctx(ctx), SF(SF) {} + bool LeaveClosureBodiesUnchecked; + + explicit DeclChecker(ASTContext &ctx, SourceFile *SF, + bool LeaveClosureBodiesUnchecked = false) + : Ctx(ctx), SF(SF), + LeaveClosureBodiesUnchecked(LeaveClosureBodiesUnchecked) {} ASTContext &getASTContext() const { return Ctx; } void addDelayedFunction(AbstractFunctionDecl *AFD) { @@ -2044,7 +2049,13 @@ class DeclChecker : public DeclVisitor { continue; if (!PBD->isInitializerChecked(i)) { - TypeChecker::typeCheckPatternBinding(PBD, i); + TypeCheckExprOptions options; + + if (LeaveClosureBodiesUnchecked) + options |= TypeCheckExprFlags::LeaveClosureBodyUnchecked; + + TypeChecker::typeCheckPatternBinding(PBD, i, /*patternType=*/Type(), + options); } if (!PBD->isInvalid()) { @@ -3164,9 +3175,9 @@ class DeclChecker : public DeclVisitor { }; } // end anonymous namespace -void TypeChecker::typeCheckDecl(Decl *D) { +void TypeChecker::typeCheckDecl(Decl *D, bool LeaveClosureBodiesUnchecked) { auto *SF = D->getDeclContext()->getParentSourceFile(); - DeclChecker(D->getASTContext(), SF).visit(D); + DeclChecker(D->getASTContext(), SF, LeaveClosureBodiesUnchecked).visit(D); } void TypeChecker::checkParameterList(ParameterList *params, diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp index ba9de18634b42..b507a72715e00 100644 --- a/lib/Sema/TypeCheckStmt.cpp +++ b/lib/Sema/TypeCheckStmt.cpp @@ -1514,7 +1514,7 @@ void StmtChecker::typeCheckASTNode(ASTNode &node) { // Type check the declaration. if (auto *D = node.dyn_cast()) { - TypeChecker::typeCheckDecl(D); + TypeChecker::typeCheckDecl(D, LeaveBraceStmtBodyUnchecked); return; } diff --git a/lib/Sema/TypeChecker.cpp b/lib/Sema/TypeChecker.cpp index d5db7d4407578..c223c3cfbe043 100644 --- a/lib/Sema/TypeChecker.cpp +++ b/lib/Sema/TypeChecker.cpp @@ -445,15 +445,20 @@ swift::handleSILGenericParams(GenericParamList *genericParams, } void swift::typeCheckPatternBinding(PatternBindingDecl *PBD, - unsigned bindingIndex) { + unsigned bindingIndex, + bool leaveClosureBodiesUnchecked) { assert(!PBD->isInitializerChecked(bindingIndex) && PBD->getInit(bindingIndex)); auto &Ctx = PBD->getASTContext(); DiagnosticSuppression suppression(Ctx.Diags); - (void)evaluateOrDefault( - Ctx.evaluator, PatternBindingEntryRequest{PBD, bindingIndex}, nullptr); - TypeChecker::typeCheckPatternBinding(PBD, bindingIndex); + + TypeCheckExprOptions options; + if (leaveClosureBodiesUnchecked) + options |= TypeCheckExprFlags::LeaveClosureBodyUnchecked; + + TypeChecker::typeCheckPatternBinding(PBD, bindingIndex, + /*patternType=*/Type(), options); } bool swift::typeCheckASTNodeAtLoc(DeclContext *DC, SourceLoc TargetLoc) { diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index b3503b0ce48ef..057a2f859f032 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -400,7 +400,7 @@ Type typeCheckParameterDefault(Expr *&defaultValue, DeclContext *DC, void typeCheckTopLevelCodeDecl(TopLevelCodeDecl *TLCD); -void typeCheckDecl(Decl *D); +void typeCheckDecl(Decl *D, bool LeaveClosureBodiesUnchecked = false); void addImplicitDynamicAttribute(Decl *D); void checkDeclAttributes(Decl *D); @@ -665,9 +665,11 @@ void coerceParameterListToType(ParameterList *P, AnyFunctionType *FN); bool typeCheckBinding(Pattern *&P, Expr *&Init, DeclContext *DC, Type patternType, PatternBindingDecl *PBD = nullptr, - unsigned patternNumber = 0); + unsigned patternNumber = 0, + TypeCheckExprOptions options = {}); bool typeCheckPatternBinding(PatternBindingDecl *PBD, unsigned patternNumber, - Type patternType = Type()); + Type patternType = Type(), + TypeCheckExprOptions options = {}); /// Type-check a for-each loop's pattern binding and sequence together. /// From 277b0fcca5a8fdf61bcf048e1af25db876629e08 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 3 Nov 2021 00:13:54 -0700 Subject: [PATCH 14/14] [BuilderTransform] Don't skip multi-statement closures during pre-check Each of the elements in the result builder has to be fully pre-checked now that multi-statement inference has been enabled. --- lib/Sema/BuilderTransform.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index 787b0a574bb94..5b6260ed84d08 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -1933,7 +1933,7 @@ class PreCheckResultBuilderApplication : public ASTWalker { HasError |= ConstraintSystem::preCheckExpression( E, DC, /*replaceInvalidRefsWithErrors=*/true, - /*leaveClosureBodiesUnchecked=*/true); + /*leaveClosureBodiesUnchecked=*/false); HasError |= transaction.hasErrors();