Skip to content

Commit c64427f

Browse files
committed
Construct AutoClosureExpr With Deeper Parent Contexts
AutoClosureExprs created by the constraint system used to be constructed with the decl context of the constraint system itself. This meant that autoclosures in expressions nested in closures would initially be parented onto any enclosing functions rather than the deepest closure context. When we ran capture analysis and lookup from inside of the body of these nascent values, we would fail to find declarations brought into scope by those parent closures. This is especially relevant when pre-typechecked code is involved since captures for those declarations will be forced before their bodies have been recontextualized. See issue #34230 for why we need to force things so early. The attached test case demonstrates both bugs: The former a bogus lookup through the parent context that would incorrectly reject this otherwise well-formed code. The latter is a crash in SILGen when the capture computation would fail to note $0. Use the decl context of the solution application target, which is always going to be the deepest user-written closure expression available to us, and therefore the deepest scope that can introduce capturable variables. rdar://79248469
1 parent 1ca4d7c commit c64427f

File tree

4 files changed

+60
-13
lines changed

4 files changed

+60
-13
lines changed

include/swift/Sema/ConstraintSystem.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4433,7 +4433,23 @@ class ConstraintSystem {
44334433

44344434
/// Build implicit autoclosure expression wrapping a given expression.
44354435
/// Given expression represents computed result of the closure.
4436+
///
4437+
/// The \p ClosureDC must be the deepest possible context that
4438+
/// contains this autoclosure expression. For example,
4439+
///
4440+
/// func foo() {
4441+
/// _ = { $0 || $1 || $2 }
4442+
/// }
4443+
///
4444+
/// Even though the decl context of $1 (after solution application) is
4445+
/// `||`'s autoclosure parameter, we cannot know this until solution
4446+
/// application has finished because autoclosure expressions are expanded in
4447+
/// depth-first order then \c ContextualizeClosures comes around to clean up.
4448+
/// All that is required is that the explicit closure be the context since it
4449+
/// is the innermost context that can introduce potential new capturable
4450+
/// declarations.
44364451
Expr *buildAutoClosureExpr(Expr *expr, FunctionType *closureType,
4452+
DeclContext *ClosureDC,
44374453
bool isDefaultWrappedValue = false,
44384454
bool isAsyncLetWrapper = false);
44394455

lib/Sema/CSApply.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,8 +1092,8 @@ namespace {
10921092
FunctionType::ExtInfo closureInfo;
10931093
auto *autoClosureType =
10941094
FunctionType::get(outerParamTypes, fnType->getResult(), closureInfo);
1095-
auto *autoClosure = new (context) AutoClosureExpr(fnCall, autoClosureType,
1096-
discriminator, cs.DC);
1095+
auto *autoClosure = new (context)
1096+
AutoClosureExpr(fnCall, autoClosureType, discriminator, dc);
10971097
autoClosure->setParameterList(outerParams);
10981098
autoClosure->setThunkKind(AutoClosureExpr::Kind::SingleCurryThunk);
10991099
cs.cacheType(autoClosure);
@@ -1126,9 +1126,8 @@ namespace {
11261126

11271127
auto resultTy = selfFnTy->getResult();
11281128
auto discriminator = AutoClosureExpr::InvalidDiscriminator;
1129-
auto closure =
1130-
new (context) AutoClosureExpr(/*set body later*/nullptr, resultTy,
1131-
discriminator, cs.DC);
1129+
auto closure = new (context) AutoClosureExpr(/*set body later*/ nullptr,
1130+
resultTy, discriminator, dc);
11321131
closure->setParameterList(params);
11331132
closure->setType(selfFnTy);
11341133
closure->setThunkKind(AutoClosureExpr::Kind::SingleCurryThunk);
@@ -1693,8 +1692,7 @@ namespace {
16931692
memberLocator);
16941693

16951694
auto outerClosure =
1696-
new (context) AutoClosureExpr(closure, selfFnTy,
1697-
discriminator, cs.DC);
1695+
new (context) AutoClosureExpr(closure, selfFnTy, discriminator, dc);
16981696
outerClosure->setThunkKind(AutoClosureExpr::Kind::DoubleCurryThunk);
16991697

17001698
outerClosure->setParameterList(outerParams);
@@ -6056,14 +6054,15 @@ Expr *ExprRewriter::coerceCallArguments(
60566054
bool isDefaultWrappedValue =
60576055
target->propertyWrapperHasInitialWrappedValue();
60586056
auto *placeholder = injectWrappedValuePlaceholder(
6059-
cs.buildAutoClosureExpr(arg, closureType, isDefaultWrappedValue),
6057+
cs.buildAutoClosureExpr(arg, closureType, dc,
6058+
isDefaultWrappedValue),
60606059
/*isAutoClosure=*/true);
60616060
arg = CallExpr::createImplicit(ctx, placeholder, {}, {});
60626061
arg->setType(closureType->getResult());
60636062
cs.cacheType(arg);
60646063
}
60656064

6066-
convertedArg = cs.buildAutoClosureExpr(arg, closureType);
6065+
convertedArg = cs.buildAutoClosureExpr(arg, closureType, dc);
60676066
} else {
60686067
convertedArg = coerceToType(
60696068
arg, paramType,
@@ -8324,7 +8323,7 @@ static Expr *wrapAsyncLetInitializer(
83248323
auto closureType = FunctionType::get({ }, initializerType, extInfo);
83258324
ASTContext &ctx = dc->getASTContext();
83268325
Expr *autoclosureExpr = cs.buildAutoClosureExpr(
8327-
initializer, closureType, /*isDefaultWrappedValue=*/false,
8326+
initializer, closureType, dc, /*isDefaultWrappedValue=*/false,
83288327
/*isAsyncLetWrapper=*/true);
83298328

83308329
// Call the autoclosure so that the AST types line up. SILGen will ignore the
@@ -8792,7 +8791,8 @@ ExprWalker::rewriteTarget(SolutionApplicationTarget target) {
87928791
// conversion.
87938792
if (FunctionType *autoclosureParamType =
87948793
target.getAsAutoclosureParamType()) {
8795-
resultExpr = cs.buildAutoClosureExpr(resultExpr, autoclosureParamType);
8794+
resultExpr = cs.buildAutoClosureExpr(resultExpr, autoclosureParamType,
8795+
target.getDeclContext());
87968796
}
87978797

87988798
solution.setExprTypes(resultExpr);

lib/Sema/ConstraintSystem.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5098,6 +5098,7 @@ ConstraintSystem::isConversionEphemeral(ConversionRestrictionKind conversion,
50985098

50995099
Expr *ConstraintSystem::buildAutoClosureExpr(Expr *expr,
51005100
FunctionType *closureType,
5101+
DeclContext *ClosureContext,
51015102
bool isDefaultWrappedValue,
51025103
bool isAsyncLetWrapper) {
51035104
auto &Context = DC->getASTContext();
@@ -5116,8 +5117,9 @@ Expr *ConstraintSystem::buildAutoClosureExpr(Expr *expr,
51165117
newClosureType = closureType->withExtInfo(info.withNoEscape(false))
51175118
->castTo<FunctionType>();
51185119

5119-
auto *closure = new (Context) AutoClosureExpr(
5120-
expr, newClosureType, AutoClosureExpr::InvalidDiscriminator, DC);
5120+
auto *closure = new (Context)
5121+
AutoClosureExpr(expr, newClosureType,
5122+
AutoClosureExpr::InvalidDiscriminator, ClosureContext);
51215123

51225124
closure->setParameterList(ParameterList::createEmpty(Context));
51235125

test/expr/capture/local_lazy.swift

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// RUN: %target-swift-frontend -emit-silgen -verify %s > /dev/null
2+
// RUN: %target-swift-frontend -dump-ast %s | %FileCheck %s
3+
4+
struct S {
5+
func foo() -> Int {
6+
// Make sure the decl context for the autoclosure passed to ?? is deep
7+
// enough that it can 'see' the capture of $0 from the outer closure.
8+
// CHECK-LABEL: (lazy_initializer_expr
9+
// CHECK: (closure_expr
10+
// CHECK: location={{.*}}local_lazy.swift:[[@LINE+3]]
11+
// CHECK: (autoclosure_expr implicit
12+
// CHECK: captures=($0<direct>
13+
lazy var nest: (Int) -> Int = { Optional<Int>.none ?? $0 }
14+
return nest(1)
15+
}
16+
}
17+
18+
extension S {
19+
func bar() -> Int {
20+
// CHECK-LABEL: (lazy_initializer_expr
21+
// CHECK: (closure_expr
22+
// CHECK: location={{.*}}local_lazy.swift:[[@LINE+3]]
23+
// CHECK: (autoclosure_expr implicit
24+
// CHECK: captures=($0<direct>
25+
lazy var nest: (Int) -> Int = { Optional<Int>.none ?? $0 }
26+
return nest(1)
27+
}
28+
}
29+

0 commit comments

Comments
 (0)