Construct AutoClosureExpr With Deeper Parent Contexts #38244
Conversation
|
Definitely not happy with this. But it's not the worst solution. |
|
@swift-ci smoke test |
There was a problem hiding this comment.
Looks reasonable to me, maybe @hborla wants to take a look.
| func foo() -> Int { | ||
| // Make sure the decl context for the autoclosure passed to ?? is deep | ||
| // enough that it can 'see' the capture of $0 from the outer closure. | ||
| lazy var nest: (Int) -> Int = { Optional<Int>.none ?? $0 } |
There was a problem hiding this comment.
Can you FileCheck the SIL function definitions to ensure they have capture arguments too, just in case?
There was a problem hiding this comment.
Followed the other tests and checked the AST dump. The SILGen test is now being used to make sure we don't crash.
|
“ Definitely not happy with this.” Why not? |
|
It relies on the structure of the AST - in a reasonable way but it’s a dependency nonetheless. I’d really like for capture computation to be more request-like but there’s structural barriers to that. In particular, it’d be nice if typechecked ASTs were always correctly constructed so we could ditch recontextualization… |
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 swiftlang#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
|
@swift-ci smoke test and merge |
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