diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 29499bfa802bc..c82babdf03a80 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -6193,6 +6193,39 @@ class ConjunctionElementProducer : public BindingProducer { } }; +/// Find any references to not yet resolved outer VarDecls (including closure +/// parameters) used in the body of a conjunction element (e.g closures, taps, +/// if/switch expressions). This is required because isolated conjunctions, just +/// like single-expression closures, have to be connected to type variables they +/// are going to use, otherwise they'll get placed in a separate solver +/// component and would never produce a solution. +class VarRefCollector : public ASTWalker { + ConstraintSystem &CS; + llvm::SmallSetVector TypeVars; + +public: + VarRefCollector(ConstraintSystem &cs) : CS(cs) {} + + /// Infer the referenced type variables from a given decl. + void inferTypeVars(Decl *D); + + MacroWalking getMacroWalkingBehavior() const override { + return MacroWalking::Arguments; + } + + PreWalkResult walkToExprPre(Expr *expr) override; + + PreWalkAction walkToDeclPre(Decl *D) override { + // We only need to walk into PatternBindingDecls, other kinds of decls + // cannot reference outer vars. + return Action::VisitChildrenIf(isa(D)); + } + + ArrayRef getTypeVars() const { + return TypeVars.getArrayRef(); + } +}; + /// Determine whether given type is a known one /// for a key path `{Writable, ReferenceWritable}KeyPath`. bool isKnownKeyPathType(Type type); diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index f0a55b2978f00..dc7c5a6c052f8 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -846,63 +846,42 @@ namespace { }; } // end anonymous namespace -namespace { - -// Collect any variable references whose types involve type variables, -// because there will be a dependency on those type variables once we have -// generated constraints for the closure/tap body. This includes references -// to other closure params such as in `{ x in { x }}` where the inner -// closure is dependent on the outer closure's param type, as well as -// cases like `for i in x where bar({ i })` where there's a dependency on -// the type variable for the pattern `i`. -struct VarRefCollector : public ASTWalker { - ConstraintSystem &cs; - llvm::SmallPtrSet varRefs; - - VarRefCollector(ConstraintSystem &cs) : cs(cs) {} - - bool shouldWalkCaptureInitializerExpressions() override { return true; } +void VarRefCollector::inferTypeVars(Decl *D) { + // We're only interested in VarDecls. + if (!isa_and_nonnull(D)) + return; - MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Arguments; - } + auto ty = CS.getTypeIfAvailable(D); + if (!ty) + return; - PreWalkResult walkToExprPre(Expr *expr) override { - // Retrieve type variables from references to var decls. - if (auto *declRef = dyn_cast(expr)) { - if (auto *varDecl = dyn_cast(declRef->getDecl())) { - if (auto varType = cs.getTypeIfAvailable(varDecl)) { - varType->getTypeVariables(varRefs); - } - } - } + SmallPtrSet typeVars; + ty->getTypeVariables(typeVars); + TypeVars.insert(typeVars.begin(), typeVars.end()); +} - // FIXME: We can see UnresolvedDeclRefExprs here because we have - // not yet run preCheckExpression() on the entire closure body - // yet. - // - // We could consider pre-checking more eagerly. - if (auto *declRef = dyn_cast(expr)) { - auto name = declRef->getName(); - auto loc = declRef->getLoc(); - if (name.isSimpleName() && loc.isValid()) { - auto *varDecl = - dyn_cast_or_null(ASTScope::lookupSingleLocalDecl( - cs.DC->getParentSourceFile(), name.getFullName(), loc)); - if (varDecl) - if (auto varType = cs.getTypeIfAvailable(varDecl)) - varType->getTypeVariables(varRefs); - } +ASTWalker::PreWalkResult +VarRefCollector::walkToExprPre(Expr *expr) { + if (auto *DRE = dyn_cast(expr)) + inferTypeVars(DRE->getDecl()); + + // FIXME: We can see UnresolvedDeclRefExprs here because we don't walk into + // patterns when running preCheckExpression, since we don't resolve patterns + // until CSGen. We ought to consider moving pattern resolution into + // pre-checking, which would allow us to pre-check patterns normally. + if (auto *declRef = dyn_cast(expr)) { + auto name = declRef->getName(); + auto loc = declRef->getLoc(); + if (name.isSimpleName() && loc.isValid()) { + auto *SF = CS.DC->getParentSourceFile(); + auto *D = ASTScope::lookupSingleLocalDecl(SF, name.getFullName(), loc); + inferTypeVars(D); } - - return Action::Continue(expr); - } - - PreWalkAction walkToDeclPre(Decl *D) override { - return Action::VisitChildrenIf(isa(D)); } -}; + return Action::Continue(expr); +} +namespace { class ConstraintGenerator : public ExprVisitor { ConstraintSystem &CS; DeclContext *CurDC; @@ -1309,8 +1288,8 @@ struct VarRefCollector : public ASTWalker { body->walk(refCollector); - referencedVars.append(refCollector.varRefs.begin(), - refCollector.varRefs.end()); + auto vars = refCollector.getTypeVars(); + referencedVars.append(vars.begin(), vars.end()); } } @@ -2971,9 +2950,7 @@ struct VarRefCollector : public ASTWalker { if (!inferredType || inferredType->hasError()) return Type(); - SmallVector referencedVars{ - refCollector.varRefs.begin(), refCollector.varRefs.end()}; - + auto referencedVars = refCollector.getTypeVars(); CS.addUnsolvedConstraint( Constraint::create(CS, ConstraintKind::FallbackType, closureType, inferredType, locator, referencedVars)); diff --git a/lib/Sema/CSSyntacticElement.cpp b/lib/Sema/CSSyntacticElement.cpp index 26465899c3d9c..4f5135e17e26d 100644 --- a/lib/Sema/CSSyntacticElement.cpp +++ b/lib/Sema/CSSyntacticElement.cpp @@ -239,50 +239,6 @@ class TypeVariableRefFinder : public ASTWalker { } }; -/// Find any references to not yet resolved outer VarDecls (including closure -/// parameters) used in the body of the inner closure. This is required because -/// isolated conjunctions, just like single-expression closures, have -/// to be connected to type variables they are going to use, otherwise -/// they'll get placed in a separate solver component and would never -/// produce a solution. -class UnresolvedVarCollector : public ASTWalker { - ConstraintSystem &CS; - - llvm::SmallSetVector Vars; - -public: - UnresolvedVarCollector(ConstraintSystem &cs) : CS(cs) {} - - MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Arguments; - } - - PreWalkResult walkToExprPre(Expr *expr) override { - if (auto *DRE = dyn_cast(expr)) { - auto *decl = DRE->getDecl(); - if (isa(decl)) { - if (auto type = CS.getTypeIfAvailable(decl)) { - if (auto *typeVar = type->getAs()) { - Vars.insert(typeVar); - } else if (type->hasTypeVariable()) { - // Parameter or result type could be only partially - // resolved e.g. `{ (x: X) -> Void in ... }` where - // `X` is a generic type. - SmallPtrSet typeVars; - type->getTypeVariables(typeVars); - Vars.insert(typeVars.begin(), typeVars.end()); - } - } - } - } - return Action::Continue(expr); - } - - ArrayRef getVariables() const { - return Vars.getArrayRef(); - } -}; - // MARK: Constraint generation /// Check whether it makes sense to convert this element into a constraint. @@ -366,7 +322,7 @@ static void createConjunction(ConstraintSystem &cs, isIsolated = true; } - UnresolvedVarCollector paramCollector(cs); + VarRefCollector paramCollector(cs); for (const auto &entry : elements) { ASTNode element = std::get<0>(entry); @@ -394,7 +350,7 @@ static void createConjunction(ConstraintSystem &cs, if (constraints.empty()) return; - for (auto *externalVar : paramCollector.getVariables()) + for (auto *externalVar : paramCollector.getTypeVars()) referencedVars.push_back(externalVar); cs.addUnsolvedConstraint(Constraint::createConjunction( diff --git a/test/Constraints/rdar112264204.swift b/test/Constraints/rdar112264204.swift new file mode 100644 index 0000000000000..68768df0774f2 --- /dev/null +++ b/test/Constraints/rdar112264204.swift @@ -0,0 +1,15 @@ +// RUN: %target-typecheck-verify-swift + +// rdar://112264204: Make sure we can type-check this. +func foo(_ fn: (Int) -> Void) {} + +func bar(_ x: Int) { + foo { [x] y in + switch y { + case x: + () + default: + () + } + } +}