-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Prepare to disable parser lookup #34158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prepare to disable parser lookup #34158
Conversation
@swift-ci Please test |
@swift-ci Please test source compatibility |
10e85a8
to
5457b1a
Compare
@swift-ci Please smoke test |
@swift-ci Please test source compatibility |
@swift-ci Please smoke test |
…PatternEntryDeclScope
…rnEntryDeclScope Today, the reported source range of a GuardStmtScope is just that of the statement itself, ending after the 'else' block. Similarly, a PatternEntryDeclScope ends immediately after the initializer expression. Since these scopes introduce names until the end of the parent BraceStmt (meaning they change the insertion point), we were relying on the addition of child scopes to extend the source range. While we still need the hack for extending source ranges to deal with string interpolation, at least in the other cases we can get rid of it. Note that this fixes a bug where a GuardStmtScope would end before an inactive '#if' block if the inactive '#if' block was the last element of a BraceStmt.
5457b1a
to
e5c813a
Compare
@swift-ci Please smoke test |
Another spot where we have to check for redeclaration manually now. As before, we can use the existing mechanism though.
This is for re-enabling it once it is turned off by default.
I created a second copy of each test where the output changes after disabling parser lookup. The primary copy now explicitly calls the frontend with -disable-parser-lookup and expects the new diagnostics; the *_parser_lookup.swift version calls the frontend with -enable-parser-lookup and has the old expectations. This allows us to turn parser lookup on and off by default without disturbing tests. Once parser lookup is completely removed we can remove the *_parser_lookup.swift variants.
e5c813a
to
bd36100
Compare
@swift-ci Please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a quick look at it, even post-merge. Looks good!
insertionPoint = | ||
ifUniqueConstructExpandAndInsert<PatternEntryDeclScope>( | ||
insertionPoint, patternBinding, i, | ||
isLocalBinding, endLocForBinding) | ||
.getPtrOr(insertionPoint); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad we spoke about this offline. Makes perfect sense.
This PR includes the final set of fixes before all remaining test failures are diagnostics changes. It also adds an
-enable-parser-lookup
flag to complement the existing-disable-parser-lookup
; it becomes useful once the default is to disable parser lookup.To deal with the test output differences, I created a second copy of each test where the output changes after disabling parser lookup. The primary copy now explicitly calls the frontend with
-disable-parser-lookup
and expects the new diagnostics; the*_parser_lookup.swift
version calls the frontend with-enable-parser-lookup
and has the old expectations.This allows us to turn parser lookup on and off by default without disturbing tests. Once parser lookup is completely removed we can remove the
*_parser_lookup.swift
variants.Note: Parser lookup remains enabled by default with this PR, except for the handful of tests where it is now explicitly disabled.
This PR builds upon #34135.