-
Notifications
You must be signed in to change notification settings - Fork 13k
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
resolve: Scope visiting doesn't need an Ident
#80782
Conversation
) | ||
| Res::SelfTy(..)), | ||
) | Res::SelfTy(..) | ||
), |
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.
self.r.hygienic_lexical_parent(search_module, &mut span_data.ctxt), | ||
break | ||
); | ||
ident.span = span_data.span(); |
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.
This will go away in #80765 which reuses visit_scopes
for this logic.
}; | ||
ident.span = span_data.span(); |
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.
This code is not yet refactored to use visit_scopes
, but it should also go away eventually.
@bors r+ |
📌 Commit 3ff866e has been approved by |
⌛ Testing commit 3ff866e with merge 10b96fdd6c1f290c822c5b49339678e12336368e... |
💥 Test timed out |
@bors retry |
☀️ Test successful - checks-actions |
Resolution scope visitor (
fn visit_scopes
) currently takes anIdent
parameter, but it doesn't need a full identifier, or even its span, it only needs theSyntaxContext
part.The
SyntaxContext
part is necessary because scope visitor has to jump to macro definition sites, so it has to be directed by macro expansion information somehow.I think it's clearer to pass only the necessary part.
Yes, usually visiting happens as a part of an identifier resolution, but in cases like collecting traits in scope (#80765) or collecting typo suggestions that's not the case.
r? @matthewjasper