Fix various problems with top-level 'guard' statements and captures #28596
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
slavapestov
force-pushed
the
fun-with-guard-let
branch
from
e4aacb6 to
f5854bd
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
|
@swift-ci Please smoke test |
|
swiftlang/llvm-project#506 |
…Decls A TopLevelCodeDecl is a local context and any declarations inside of one must be treated as captures. Furthermore, all the TopLevelCodeDecl children of a source file are peers, and can see each other's bindings, so don't perform an exact match on the DeclContext. Part of <rdar://problem/23051362> / <https://bugs.swift.org/browse/SR-3528>.
A method of a nominal type is not allowed to capture anything. If we did compute a capture list, clear it out to preserve downstream invariants. This can happen because not all invalid captures involving nominal types in local context can be diagnosed in Sema. In particular, we allow a method of a local type to reference a local function from an outer scope, as long as that local function does not have any (transitive) captures. Since Sema cannot validate the latter condition, it punts and allows any reference to a FuncDecl in local context. When emitting the captures in this situation, SILGen will diagnose the problem using the existing 'forward reference' logic. The diagnostic is sub-optimal -- it should talk about a reference from inside a local type, and not a 'forward reference'. But still better than crashing.
This commit adds a new ValueDecl::isLocalCapture() predicate and uses it in the right places. The predicate is true if the declaration is in local context, *or* if its at the top level of the main source file and follows a 'guard' statement. Fixes <rdar://problem/23051362> / <https://bugs.swift.org/browse/SR-3528>.
slavapestov
force-pushed
the
fun-with-guard-let
branch
from
f5854bd to
8214e18
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Methods of local types are normally not allowed to capture local bindings from the outer scope, however we did not diagnose this case if the local binding was itself part of a TopLevelCodeDecl, causing a crash on invalid; an example would be a
varnested inside a top-leveldostatement. One case that used to work is where the local type referenced a local function that itself had no captures; this continues to work.Bindings established by a
guardstatement are visible outside of the TopLevelCodeDecl in which the GuardStmt and it's associated VarDecls appear, but the functions referencing them were not properly considered to be in local context, so we did not always expect them to have a capture list. These cases now work correctly.Finally, clean up some the assertions that check for the presence of a capture list. We compute capture lists eagerly in Sema, so by the time we get to SILGen we must have all the capture lists that we need. However this was complicated by references to top-level functions defined in other files and similar, where no capture list was computed or necessary. By centralizing the check inside
getLoweredLocalCaptures()we can just skip all of the work entirely if we know there won't be any local captures.Fixed rdar://problem/23051362 / https://bugs.swift.org/browse/SR-3528, and some related issues I discovered while working on those.