Skip to content

nr: Add modules to types NS#4404

Open
CohenArthur wants to merge 2 commits into
Rust-GCC:masterfrom
CohenArthur:add-modules-to-types-ns
Open

nr: Add modules to types NS#4404
CohenArthur wants to merge 2 commits into
Rust-GCC:masterfrom
CohenArthur:add-modules-to-types-ns

Conversation

@CohenArthur
Copy link
Copy Markdown
Member

@CohenArthur CohenArthur commented Jan 21, 2026

gcc/rust/ChangeLog:

* util/rust-hir-map.cc (Mappings::insert_module_id): New function. (Mappings::is_module): Likewise.
* util/rust-hir-map.h: Store a set of AST modules, declare functions for adding and retrieving them.
* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::visit): Insert modules in the type namespace and store them in mappings.
* resolve/rust-late-name-resolver-2.0.cc (resolve_type_path_like): Error out when an expected type resolves to a module.

gcc/testsuite/ChangeLog:

* rust/compile/mod_in_types_ns.rs: New test.
* rust/compile/mod_in_types_ns2.rs: New test.

Fixes #4403
Fixes #4563

@P-E-P P-E-P self-requested a review January 22, 2026 11:37
@CohenArthur CohenArthur force-pushed the add-modules-to-types-ns branch 3 times, most recently from c5fe830 to ad025f2 Compare January 30, 2026 12:51
// is that valid?
rust_assert (inserted);
// TODO: Are reinsertions okay?
// rust_assert (inserted);
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.

Problem is, a lot of reinsertions aren't reinsertions so much as "we have multiple nodes sharing the same node id" (though we should really have another pass checking for that). Failing that, resolutions theoretically shouldn't be changing over time, even though the specific use case you're going for makes sense. Could you add a map_usage_unsafe method instead?

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 is also an issue for #4551 - I'd rather not think about it now if I'm honest since I've been working on this for months at this point. I would rather we merge this, merge #4551 and then add an issue for improving map_usage and making it more correct

@powerboat9
Copy link
Copy Markdown
Collaborator

I think I'm going to try to add backtracking to the resolver, as I don't think the type prelude will work quite right otherwise.

@CohenArthur
Copy link
Copy Markdown
Member Author

I think I'm going to try to add backtracking to the resolver, as I don't think the type prelude will work quite right otherwise.

what do you mean by backtracking?

@powerboat9
Copy link
Copy Markdown
Collaborator

I think I'm going to try to add backtracking to the resolver, as I don't think the type prelude will work quite right otherwise.

what do you mean by backtracking?

Backtracking in the sense that resolution doesn't commit to certain segment resolutions until it succeeds, and can retry with a different namespace/starting location (type prelude vs not type prelude) with fewer issues.

@CohenArthur CohenArthur force-pushed the add-modules-to-types-ns branch from ad025f2 to 381b4ff Compare February 11, 2026 14:34
Comment thread gcc/rust/resolve/rust-toplevel-name-resolver-2.0.cc
@CohenArthur
Copy link
Copy Markdown
Member Author

Probably this should get merged after #4551 because #4551 is so big

gcc/rust/ChangeLog:

	* util/rust-hir-map.cc (Mappings::insert_module_id): New function.
	(Mappings::is_module): Likewise.
	* util/rust-hir-map.h: Store a set of AST modules, declare functions for adding and
	retrieving them.
	* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::visit): Insert modules in the type
	namespace and store them in mappings.
	* resolve/rust-late-name-resolver-2.0.cc (resolve_type_path_like): Error out when an
	expected type resolves to a module.
	* resolve/rust-name-resolution-context.cc (NameResolutionContext::map_usage): Allow
	reinsertions.
	* resolve/rust-name-resolution.h: Add empty Definition constructor.
	* resolve/rust-resolve-builtins.cc (find_builtin_node_id): New function.
	* resolve/rust-resolve-builtins.h: Declare it.

gcc/testsuite/ChangeLog:

	* rust/compile/mod_in_types_ns.rs: New test.
	* rust/compile/mod_in_types_ns2.rs: New test.
@CohenArthur CohenArthur force-pushed the add-modules-to-types-ns branch from 6e37b04 to 9f5bfe5 Compare May 29, 2026 13:58
Remove the previous hack that was used for resolver module names in the Types NS. Modules now
get properly inserted, and this hack causes conflicts when a module and value share the same
name.

gcc/rust/ChangeLog:

	* resolve/rust-name-resolution-context.hxx: Remove hacks.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-4563.rs: New test.
@CohenArthur CohenArthur marked this pull request as ready for review May 29, 2026 14:07
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.

Invalid multiple definitions errors for module and function with the same name Modules need to be inserted in the type namespace

3 participants