-
Notifications
You must be signed in to change notification settings - Fork 14.1k
resolve: Split Scope::Module into two scopes for non-glob and glob bindings
#149681
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
base: main
Are you sure you want to change the base?
Conversation
…d the innermost binding. Previously we could lose the already found binding and break with an error, if some blocking error was found in the shadowed scopes. Also, avoid some impossible state in the return type of `fn resolve_ident_in_scope`.
in the same module to the usual ambiguity infra in `resolve_ident_in_scope_set`
for looking up a name in two scopes insice a module - non-glob and glob bindings.
…ings in the same module to the usual ambiguity infra in `resolve_ident_in_scope_set`
|
r? @davidtwco rustbot has assigned @davidtwco. Use |
|
There may be minor breakage in corner cases due to ambiguity checking being unified between in-scope resolution and in-module resolution, so this needs a crater run. |
This comment has been minimized.
This comment has been minimized.
resolve: Split `Scope::Module` into two scopes for non-glob and glob bindings
| && glob_binding.res() != non_glob_binding.res() | ||
| { | ||
| resolution.non_glob_binding = Some(this.new_ambiguity_binding( | ||
| AmbiguityKind::GlobVsExpanded, |
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.
@yaahc you may be interested, this PR removes one of the "gray areas".
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test for 588eab2 failed: CI. Failed jobs:
|
This is a re-implementation of #144131 with all the issues mentioned there fixed.
As it turned out, the non-trivial part of the split was already done in c91b6ca, so the remaining part implemented in this PR is mostly mechanical.
After addressing the issue of already found bindings being lost due to indeterminacies in outer scopes (7e890bf) and adding one missing
Stage::LateinFinalizethe scope splitting refactoring just worked.(One more ICE was revealed by the refactoring, but not caused by it, fixed up in the last commit.)
This is a part of implementation for the Open API proposal.