Skip to content
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

Merged
merged 1 commit into from
Feb 3, 2025
Merged

Conversation

powerboat9
Copy link
Collaborator

@powerboat9 powerboat9 commented Dec 1, 2024

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.

@powerboat9 powerboat9 changed the title Partially fix bug in type resolution of paths Fix bug in type resolution of paths Jan 5, 2025
Copy link
Member

@philberty philberty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this LGTM

@powerboat9
Copy link
Collaborator Author

It looks like this might need something like #3220 to prevent test regressions

@powerboat9
Copy link
Collaborator Author

This now removes a bunch of tests from the exclude file, but causes a regression with issue-2070.rs

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>
@powerboat9
Copy link
Collaborator Author

I've fixed the regression, and removed a few more tests from the exclude list

Copy link
Member

@CohenArthur CohenArthur left a 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!

@CohenArthur CohenArthur added this pull request to the merge queue Feb 3, 2025
Merged via the queue into Rust-GCC:master with commit 668c3bf Feb 3, 2025
12 checks passed
@powerboat9 powerboat9 deleted the fix-seg branch February 3, 2025 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants