-
Notifications
You must be signed in to change notification settings - Fork 166
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
Fix bug in type resolution of paths #3277
Conversation
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 LGTM
It looks like this might need something like #3220 to prevent test regressions |
This now removes a bunch of tests from the exclude file, but causes a regression with |
gcc/rust/ChangeLog: * resolve/rust-early-name-resolver-2.0.cc (Early::resolve_glob_import): Use NameResolutionContext::resolve_path instead of ForeverStack::resolve_path. (Early::visit): Likewise. (Early::visit_attributes): Likewise. * resolve/rust-early-name-resolver-2.0.h (Early::resolve_path_in_all_ns): Likewise. * resolve/rust-late-name-resolver-2.0.cc (Late::visit): Likewise, insert segment resolutions not handled by NameResolutionContext::resolve_path, and avoid throwing an error when path resolution could be finished by the typechecker. * resolve/rust-name-resolution-context.h (NameResolutionContext::resolve_path): Add. * typecheck/rust-hir-type-check-path.cc (TypeCheckExpr::resolve_root_path): Use segment node ids instead of the path node id to look up segment resolutions when using the 2.0 resolver, as is done with the 1.0 resolver. * typecheck/rust-hir-type-check-type.cc (TypeCheckType::resolve_root_path): Likewise. * resolve/rust-forever-stack.h (ForeverStack::resolve_path): Add callback parameter for inserting segment resolutions. (ForeverStack::find_starting_point): Likewise. (ForeverStack::resolve_segments): Likewise. * resolve/rust-forever-stack.hxx (ForeverStack::find_starting_point): Likewise. (ForeverStack::resolve_segments): Likewise. (ForeverStack::resolve_path): Likewise and avoid resolving inside TraitOrImpl ribs. gcc/testsuite/ChangeLog: * rust/compile/nr2/exclude: Remove entries. Signed-off-by: Owen Avery <powerboat9.gamer@gmail.com>
I've fixed the regression, and removed a few more tests from the exclude list |
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.
LGTM, thanks a lot Owen!
When name resolution 2.0 is enabled these sections of the typechecker should be performing resolution lookups using segment node ids, rather than path node ids, as is done when name resolution 1.0 is enabled. Since name resolution 2.0 doesn't currently insert resolutions for individual segments, lookups based on path node id are left as a fallback.