From 0482bd36522c46c43b1fde39e4c005f2a6195a09 Mon Sep 17 00:00:00 2001 From: Sima Nerush <2002ssn@gmail.com> Date: Fri, 8 Dec 2023 23:49:13 -0800 Subject: [PATCH 1/3] [Sema] Refactor code and improve documentation pertaining to SE-0408. --- include/swift/AST/Decl.h | 3 +++ include/swift/Sema/ConstraintSystem.h | 16 +++++++++++++--- include/swift/Sema/SyntacticElementTarget.h | 14 ++++++-------- lib/AST/Decl.cpp | 5 +++-- lib/Sema/CSGen.cpp | 10 ++++++---- lib/Sema/CSSimplify.cpp | 6 +++--- 6 files changed, 34 insertions(+), 20 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 3ab74f01c1588..e3af3be40d3ec 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -5857,6 +5857,9 @@ enum class PropertyWrapperSynthesizedPropertyKind { class VarDecl : public AbstractStorageDecl { friend class NamingPatternRequest; NamedPattern *NamingPattern = nullptr; + /// When the variable is declared in context of a for-in loop over the elements of + /// a parameter pack, this is the opened element environment of the pack expansion + /// to use as the variable's context generic environment. GenericEnvironment *OpenedElementEnvironment = nullptr; public: diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 0edfe099469c1..09c25f787f322 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -3765,10 +3765,20 @@ class ConstraintSystem { RememberChoice_t rememberChoice, ConstraintLocatorBuilder locator, ConstraintFix *compatFix = nullptr); - - /// Add a materialize constraint for a pack expansion. + + /// Given a tuple with a single unlabeled element that represents a pack + /// expansion (either directly via \c PackExpansionType or through a type + /// variable constrained to a pack expansion), materialize the pack expansion + /// and return its pattern type as a result. The result is a type variable + /// because element of the tuple is not required to be resolved at the time of + /// the call and operation is delayed until the element is sufficiently + /// resolved (see \c simplifyMaterializePackExpansionConstraint) + /// + /// \param tupleType A tuple with a single unlabeled element that represents a + /// pack expansion. + /// \param locator The locator. TypeVariableType * - addMaterializePackExpansionConstraint(Type patternType, + addMaterializePackExpansionConstraint(Type tupleType, ConstraintLocatorBuilder locator); /// Add a disjunction constraint. diff --git a/include/swift/Sema/SyntacticElementTarget.h b/include/swift/Sema/SyntacticElementTarget.h index eafc13f824c9f..4a87c152396cc 100644 --- a/include/swift/Sema/SyntacticElementTarget.h +++ b/include/swift/Sema/SyntacticElementTarget.h @@ -29,8 +29,8 @@ namespace swift { namespace constraints { -/// Describes information specific to a sequence -/// in a for-each loop. +/// Describes information about a for-in loop over a sequence that needs to be +/// tracked in the constraint system. struct SequenceIterationInfo { /// The type of the sequence. Type sequenceType; @@ -48,18 +48,16 @@ struct SequenceIterationInfo { Expr *nextCall; }; -/// Describes information specific to a pack expansion expression -/// in a for-each loop. +/// Describes information about a for-in loop over a pack that needs to be +/// tracked in the constraint system. struct PackIterationInfo { /// The type of the pattern that matches the elements. Type patternType; }; -/// Describes information about a for-each loop that needs to be tracked +/// Describes information about a for-in loop that needs to be tracked /// within the constraint system. -struct ForEachStmtInfo : TaggedUnion { - using TaggedUnion::TaggedUnion; -}; +using ForEachStmtInfo = TaggedUnion; /// Describes the target to which a constraint system's solution can be /// applied. diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 875e69eceebb2..bed235e1eba49 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -7149,8 +7149,9 @@ VarDecl::VarDecl(DeclKind kind, bool isStatic, VarDecl::Introducer introducer, } Type VarDecl::getTypeInContext() const { - // If we are performing pack iteration, use the generic environment of the - // pack expansion expression to get the right context of a local variable. + // If the variable is declared in context of a for-in loop over the elements + // of a parameter pack, its interface type must be mapped into context using + // the opened element environment of the pack expansion. if (auto *env = getOpenedElementEnvironment()) return GenericEnvironment::mapTypeIntoContext(env, getInterfaceType()); diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 6c901d12aa55f..0039621003657 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -4447,8 +4447,8 @@ static bool generateInitPatternConstraints(ConstraintSystem &cs, return false; } -/// Generate constraints for a for-in statement preamble, expecting a -/// `PackExpansionExpr`. +/// Generate constraints for a for-in statement preamble where the expression +/// is a `PackExpansionExpr`. static llvm::Optional generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc, PackExpansionExpr *expansion, Type patternType) { @@ -4524,6 +4524,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc, auto *makeIteratorCall = CallExpr::createImplicitEmpty(ctx, makeIteratorRef); + Pattern *pattern = NamedPattern::createImplicit(ctx, makeIteratorVar); auto *PB = PatternBindingDecl::createImplicit( ctx, StaticSpellingKind::None, pattern, makeIteratorCall, dc); @@ -4540,6 +4541,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc, return llvm::None; sequenceIterationInfo.makeIteratorVar = PB; + // Type of sequence expression has to conform to Sequence protocol. // // Note that the following emulates having `$generator` separately @@ -4606,7 +4608,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc, cs.setTargetFor(sequenceIterationInfo.nextCall, nextTarget); } - // Generate constraints for the pattern + // Generate constraints for the pattern. Type initType = cs.generateConstraints(typeCheckedPattern, elementLocator, shouldBindPatternVarsOneWay, nullptr, 0); @@ -4677,7 +4679,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs, if (isa(forEachExpr)) { auto *expansion = cast(forEachExpr); - // Generate constraints for the pattern + // Generate constraints for the pattern. Type patternType = cs.generateConstraints( pattern, elementLocator, target.shouldBindPatternVarsOneWay(), nullptr, 0); diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index a25d11622a494..2ef80336852ed 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -15640,11 +15640,11 @@ void ConstraintSystem::addExplicitConversionConstraint( } TypeVariableType *ConstraintSystem::addMaterializePackExpansionConstraint( - Type patternType, ConstraintLocatorBuilder locator) { - assert(isSingleUnlabeledPackExpansionTuple(patternType)); + Type tupleType, ConstraintLocatorBuilder locator) { + assert(isSingleUnlabeledPackExpansionTuple(tupleType)); TypeVariableType *packVar = createTypeVariable(getConstraintLocator(locator), TVO_CanBindToPack); - addConstraint(ConstraintKind::MaterializePackExpansion, patternType, packVar, + addConstraint(ConstraintKind::MaterializePackExpansion, tupleType, packVar, getConstraintLocator(locator, {ConstraintLocator::Member})); return packVar; } From 92d81f340ebd146334d0f7093168dde066e8f1e3 Mon Sep 17 00:00:00 2001 From: Sima Nerush <2002ssn@gmail.com> Date: Fri, 8 Dec 2023 23:53:31 -0800 Subject: [PATCH 2/3] [SILGen] Improve documentation of SE-0408, put back the documentation that was misplaced. --- lib/SILGen/SILGenStmt.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/SILGen/SILGenStmt.cpp b/lib/SILGen/SILGenStmt.cpp index f986b45dbad7a..fcb84854c474e 100644 --- a/lib/SILGen/SILGenStmt.cpp +++ b/lib/SILGen/SILGenStmt.cpp @@ -1246,8 +1246,6 @@ void StmtEmitter::visitRepeatWhileStmt(RepeatWhileStmt *S) { } void StmtEmitter::visitForEachStmt(ForEachStmt *S) { - // Emit the 'iterator' variable that we'll be using for iteration. - LexicalScope OuterForScope(SGF, CleanupLocation(S)); if (auto *expansion = dyn_cast(S->getTypeCheckedSequence())) { @@ -1256,8 +1254,6 @@ void StmtEmitter::visitForEachStmt(ForEachStmt *S) { ->getCanonicalType()); JumpDest loopDest = createJumpDest(S->getBody()); - - // Set the destinations for 'break' and 'continue'. JumpDest endDest = createJumpDest(S->getBody()); SGF.emitDynamicPackLoop( @@ -1271,6 +1267,7 @@ void StmtEmitter::visitForEachStmt(ForEachStmt *S) { SGF.emitExprInto(expansion->getPatternExpr(), letValueInit.get()); + // Set the destinations for 'break' and 'continue'. SGF.BreakContinueDestStack.push_back({S, endDest, loopDest}); visit(S->getBody()); SGF.BreakContinueDestStack.pop_back(); @@ -1284,6 +1281,8 @@ void StmtEmitter::visitForEachStmt(ForEachStmt *S) { return; } + // Emit the 'iterator' variable that we'll be using for iteration. + LexicalScope OuterForScope(SGF, CleanupLocation(S)); SGF.emitPatternBinding(S->getIteratorVar(), /*index=*/0, /*debuginfo*/ true); From 40167fbd3f3b2212346ea06ca63796c03ccd08a3 Mon Sep 17 00:00:00 2001 From: Sima Nerush <2002ssn@gmail.com> Date: Fri, 8 Dec 2023 23:58:03 -0800 Subject: [PATCH 3/3] [ASTVerifier] Use `hasError()` to avoid propagating the error type. --- lib/AST/ASTVerifier.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/AST/ASTVerifier.cpp b/lib/AST/ASTVerifier.cpp index 94b30a1ce0115..64d78e40412e3 100644 --- a/lib/AST/ASTVerifier.cpp +++ b/lib/AST/ASTVerifier.cpp @@ -2645,7 +2645,7 @@ class Verifier : public ASTWalker { } // Catch cases where there's a missing generic environment. - if (var->getTypeInContext()->is()) { + if (var->getTypeInContext()->hasError()) { Out << "VarDecl is missing a Generic Environment: "; var->getInterfaceType().print(Out); Out << "\n";