Skip to content

Do path resolution in types NS for all segments except the last one#4527

Open
CohenArthur wants to merge 2 commits intoRust-GCC:masterfrom
CohenArthur:do-path-resolution-in-types-ns
Open

Do path resolution in types NS for all segments except the last one#4527
CohenArthur wants to merge 2 commits intoRust-GCC:masterfrom
CohenArthur:do-path-resolution-in-types-ns

Conversation

@CohenArthur
Copy link
Copy Markdown
Member

Per multiple comments in the rustc frontend this is how path resolution should actually be done. In a path like foo::bar::baz, the segments foo and bar must be resolved in the types NS, regardless of the namespace that we are trying to resolve the entire path in (e.g. even in a path function call like foo::bar::baz() where we expect to find the function in the values namespace). This also links with a further change where imports and modules must be inserted in the types namespace in order to be resolved to properly

@CohenArthur CohenArthur requested review from P-E-P and powerboat9 April 18, 2026 17:46
Comment thread gcc/rust/resolve/rust-forever-stack.h Outdated
Comment on lines -811 to -861
@@ -857,8 +832,6 @@ template <Namespace N> class ForeverStack
void reverse_iter (const Node &start,
std::function<KeepGoing (const Node &)> lambda) const;

Node &cursor ();
const Node &cursor () const;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why move cursor () away from update_cursor ()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was probably in an attempt to make it public before saying fuck it and putting everything as public, considering the ForeverStack is just a helper struct/class that gets used by the NRCtx and doesn't really "need" encapsulation. I'll move it back

namespace Rust {
namespace Resolver2_0 {

template <Namespace N>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should make ForeverStack a non-template class before trying to merge it into NameResolutionContext

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think that it makes more sense for ForeverStacks to still be separated by namespaces and I like the templated solution, but maybe I'm not seeing the issues it has?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Also, in later commits I move more of the path resolution methods like resolve_segments and resolve_final_segment which benefit from being able to be specialized for the Types NS and so on

@CohenArthur CohenArthur force-pushed the do-path-resolution-in-types-ns branch from b7bf014 to 8039cf3 Compare April 19, 2026 12:07
gcc/rust/ChangeLog:

	* resolve/rust-forever-stack.h: Move declarations from ForeverStack to NRCtx, make most
	of the ForeverStack members public as it helps the Ctx a lot.
	* resolve/rust-forever-stack.hxx: Move implementation of resolve_path methods to NRCtx.
	* resolve/rust-name-resolution-context.h: Declare resolve_path methods.
	* resolve/rust-name-resolution-context.hxx: New file with resolve_path impls.
gcc/rust/ChangeLog:

	* resolve/rust-name-resolution-context.hxx: Do segment resolution in types NS for more
	correctness and correct behavior when later resolving paths that use imports and/or
	modules.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants