[Sema] Diagnose likely-unsound script var forward references#88082
Draft
hamishknight wants to merge 1 commit into
Draft
[Sema] Diagnose likely-unsound script var forward references#88082hamishknight wants to merge 1 commit into
hamishknight wants to merge 1 commit into
Conversation
kathygray-pl
reviewed
Apr 7, 2026
| let selfRecurse1 = SelfRecursiveLookup { selfRecurse1.e } | ||
| let selfRecurse1 = SelfRecursiveLookup { selfRecurse1.e } // expected-note {{'selfRecurse1' declared here}} | ||
| // expected-error@-1 {{could not find member 'e'; exceeded the maximum number of nested dynamic member lookups}} | ||
| // expected-warning@-2 {{use of global variable 'selfRecurse1' before its declaration; this will be an error in a future Swift language mode}} |
Contributor
There was a problem hiding this comment.
Nit: I wonder for these if we could detect that the use is at the initialisation of the declaration and special case the message. It's not technically before, it's at the same time...
Contributor
Author
There was a problem hiding this comment.
Ah yes this is because we're re-using the existing diagnostic logic for local cases, e.g:
func foo() {
let x = x // error: use of local variable 'x' before its declaration
}We ought to special-case this yeah, but I think that can be done in a separate PR
Emit an error for a forward reference to a global script-mode variable
that is likely to be unsound. This handles cases where the variable is
stored, and the reference is made at the top level before the
declaration. Such uses would otherwise have likely read uninitialized
memory.
The only way such a reference could have been used in a sound way is
by also initializing it before declaration, e.g:
```
x = 1
print(x)
let x: Int
```
But source compatibility testing has shown this to be incredibly rare,
testing across both the source compat suite and internal Swift
projects at Apple has only turned up a single case so far.
Uses within closures and functions are unfortunately more common so
cannot be outright banned, but we will emit a warning on them.
Note that this checking doesn't help with cross-file cases, ultimately
we still need to properly fix script-mode variables such that they are
consistently treated as either local or global variables. Such a
change will be quite source breaking though, this patch at least
allows us to diagnose the majority of cases in the mean time.
### Why not diagnose in SIL?
SILGen already emits `mark_uninitialized` and `mark_function_escape`
instructions for script-mode variables which allows use-before-init
to be diagnosed by DefiniteInitialization, however this only works
when the use actually follows the declaration. In principe we could
adjust the SILGen here to emit these mark instructions at the start of
`main`, which would let us more precisely diagnose _only_ the unsound
cases. However the fact that we allow these forward references is also
generally problematic in Sema, e.g:
```
print(a)
guard let b = 3 as Int? else {}
let a = b
```
This currently crashes the compiler since the forward reference to `a`
kicks lazy type-checking, which type-checks the statement condition.
This then results in us attempting to type-check the condition
multiple times. Such code would already be invalid if in a local
context such as a function. In principe we could defensively guard
against these cases, but I would rather just eliminate them entirely.
Forum post to follow.
Contributor
Author
|
@swift-ci please test |
Contributor
Author
|
@swift-ci please test source compatibility |
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
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.
Emit an error for a forward reference to a global script-mode variable that is likely to be unsound. This handles cases where the variable is stored, and the reference is made at the top level before the declaration. Such uses would otherwise have likely read uninitialized memory.
The only way such a reference could have been used in a sound way is by also initializing it before declaration, e.g:
But source compatibility testing has shown this to be incredibly rare, testing across both the source compat suite and internal Swift projects at Apple has only turned up a single case so far.
Uses within closures and functions are unfortunately more common so cannot be outright banned, but we will emit a warning on them.
Note that this checking doesn't help with cross-file cases, ultimately we still need to properly fix script-mode variables such that they are consistently treated as either local or global variables. Such a change will be quite source breaking though, this patch at least allows us to diagnose the majority of cases in the mean time.
Why not diagnose in SIL?
SILGen already emits
mark_uninitializedandmark_function_escapeinstructions for script-mode variables which allows use-before-init to be diagnosed by DefiniteInitialization, however this only works when the use actually follows the declaration. In principe we could adjust the SILGen here to emit these mark instructions at the start ofmain, which would let us more precisely diagnose only the unsound cases. However the fact that we allow these forward references is also generally problematic in Sema, e.g:This currently crashes the compiler since the forward reference to
akicks lazy type-checking, which type-checks the statement condition. This then results in us attempting to type-check the condition multiple times. Such code would already be invalid if in a local context such as a function. In principe we could defensively guard against these cases, but I would rather just eliminate them entirely.Forum post: https://forums.swift.org/t/reject-likely-unsound-script-variable-forward-references/85996
Resolves #86438