ASTScope: Take start location into account when modeling local pattern bindings #34038
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
changed the title
slavapestov
changed the title
slavapestov
force-pushed
the
local-pbd-scopes
branch
from
fbf9983 to
1d9d6ed
Compare
slavapestov
force-pushed
the
local-pbd-scopes
branch
from
1d9d6ed to
818b8be
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
slavapestov
force-pushed
the
local-pbd-scopes
branch
from
818b8be to
e4cd209
Compare
|
@swift-ci Please smoke test |
slavapestov
force-pushed
the
local-pbd-scopes
branch
2 times, most recently
from
66bc234 to
da55293
Compare
|
@swift-ci Please smoke test |
slavapestov
force-pushed
the
local-pbd-scopes
branch
from
da55293 to
a78e5a9
Compare
Bindings are not visible from inside their own initializer, eg this is invalid: var (x, y) = (y, x) The PatternEntryDeclScope which starts after the 'var' introduces x and y, and it contains the PatternEntryInitializerScope which starts after the '='. To model this properly in ASTScope, the PatternEntryInitializerScope overrides getLookupParent() to skip the PatternEntryDeclScope that contains it. I believe this is simpler than having PatternEntryDeclScope begin at the source location following the initializer, because it is hard to compute this source location in a way that works with invalid code, for example the 'var' might be nested inside of a BraceStmt with the closing '}' missing.
slavapestov
force-pushed
the
local-pbd-scopes
branch
from
a78e5a9 to
40cbc22
Compare
… bindings This gives us the desired behavior, where local bindings are in scope after their definition. Note that BraceStmt still introduces all bindings at the beginning, but now we change BraceStmt to only introduce local functions and types, allowing us to disable parse-time lookup.
slavapestov
force-pushed
the
local-pbd-scopes
branch
from
40cbc22 to
5461508
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
davidungar
reviewed
Sep 24, 2020
| /// If true, nest scopes so a variable is out of scope before its declaration | ||
| /// Does not handle capture rules for local functions properly. | ||
| /// If false don't push uses down into subscopes after decls. | ||
| static const bool handleUseBeforeDef = false; |
There was a problem hiding this comment.
This is the flag I've been talking about!
davidungar
reviewed
Sep 24, 2020
Comment on lines
+758
to
+760
| ASTScopeAssert(isLocalBinding || insertionPoint == parentScope, | ||
| "Bindings at the top-level or members of types should " | ||
| "not change the insertion point"); |
davidungar
reviewed
Sep 24, 2020
Comment on lines
+1042
to
+1044
| // In local context, the PatternEntryDeclScope becomes the insertion point, so | ||
| // that all any bindings introduecd by the pattern are in scope for subsequent | ||
| // lookups. |
| // CHECK-EXPANDED-NEXT: |-TypeAliasDeclScope {{.*}}, [15:1 - 15:15] | ||
| // CHECK-EXPANDED-NEXT: `-PatternEntryDeclScope {{.*}}, [13:5 - 21:28] entry 0 'c' | ||
| // CHECK-EXPANDED-NEXT: |-PatternEntryInitializerScope {{.*}}, [13:9 - 13:9] entry 0 'c' | ||
| // CHECK-EXPANDED-NEXT: |-TypeAliasDeclScope {{.*}}, [15:1 - 15:15] |
There was a problem hiding this comment.
Note: It seems like the TypeAliasDeclScope got overindented along the way.
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.
Today,
BraceStmtScopeintroduces all of its local bindings without regard to source order. Instead, let's model pattern bindings properly, by having thePatternEntryDeclScopeintroduce its bindings. Note that whilePatternEntryInitializerScopeis a child of thePatternEntryDeclScope, it's lookup parent skips over thePatternEntryDeclScopesince bindings are not visible from their own initializer.For example,
The names
xandyare in scope immediately following(0, 0), and the namesuandvare in scope immediately followingfoo(x, y).Builds on top of #34039.