Fix modules with same name as builtins causing ICE (#3315) #3437
Conversation
liamnaddell
force-pushed
the
modname
branch
5 times, most recently
from
44e6e4e to
98f7902
Compare
| // we're at the root :) hopefully! Globbed declaration allows | ||
| // user-defined items in the `types` namespace | ||
| // to shadow builtins. Probably unwise, but allowed by rustc! | ||
| auto ok = ctx.types.insert_globbed (builtin.name, builtin.node_id); |
There was a problem hiding this comment.
Since it's in a separate rib, insert, insert_shadowable, or insert_globbed would all work fine here
| @@ -107,6 +107,11 @@ NameResolutionContext::scoped (Rib::Kind rib_kind, NodeId id, | |||
| std::function<void (void)> lambda, | |||
| tl::optional<Identifier> path) | |||
| { | |||
| // Prelude doesn't have an access path | |||
| // TODO: Assert current cursor is at the root | |||
There was a problem hiding this comment.
This should already hit an assertion, so you could remove the comment or add another assertion
| // Prelude doesn't have an access path | ||
| // TODO: Assert current cursor is at the root | ||
| if (rib_kind == Rib::Kind::Prelude) | ||
| rust_assert (!path); |
There was a problem hiding this comment.
This could be moved to push_inner or copied there, but an assertion like this should probably be in push_inner
It's alright, it looks good |
liamnaddell
force-pushed
the
modname
branch
2 times, most recently
from
c9a02b2 to
20e456a
Compare
|
@powerboat9 Thank you for the reviews, and sorry about the TODO: comments -- I forgot about those. I think I fixed all of the issues you pointed out. |
liamnaddell
force-pushed
the
modname
branch
2 times, most recently
from
a54f21a to
933eba4
Compare
gcc/rust/ChangeLog: * resolve/rust-forever-stack.h: Add a dedicated prelude node for the Language prelude * resolve/rust-forever-stack.hxx: Add support code for the prelude node * resolve/rust-late-name-resolver-2.0.cc (Late::visit): Move language prelude builtins to the prelude context * resolve/rust-name-resolution-context.cc: Add code for handling the prelude corner case * resolve/rust-rib.h: Add a special Prelude rib type gcc/testsuite/ChangeLog: * rust/compile/issue-3315-1.rs: Add test for module with same name as builtin * rust/compile/issue-3315-2.rs: Test with utilization of i32 type * rust/compile/nr2/exclude: issue-3315-2.rs Does not work with NR2.0 Signed-off-by: Liam Naddell <[email protected]>
There was a problem hiding this comment.
Looks like a regression for:
Running /home/runner/work/gccrs/gccrs/gcc/testsuite/rust/borrowck/borrowck.exp ...
Running /home/runner/work/gccrs/gccrs/gcc/testsuite/rust/compile/compile.exp ...
Running /home/runner/work/gccrs/gccrs/gcc/testsuite/rust/compile/macros/builtin/builtin_macro.exp ...
Running /home/runner/work/gccrs/gccrs/gcc/testsuite/rust/compile/macros/mbe/mbe_macro.exp ...
Running /home/runner/work/gccrs/gccrs/gcc/testsuite/rust/compile/macros/proc/proc_macro.exp ...
Running /home/runner/work/gccrs/gccrs/gcc/testsuite/rust/compile/nr2/compile.exp ...
FAIL: /home/runner/work/gccrs/gccrs/gcc/testsuite/rust/compile/macros/mbe/macro54.rs: nr2 failure: FAIL: rust/compile/macros/mbe/macro54.rs -frust-name-resolution-2.0 (test for excess errors)
For some reason after rebasing these fail now. I'll take a look at these |
|
I think the strategy I chose to implement this doesn't work, since the test involves resolving something like u32::VALUE. I'll try to re-implement |
@powerboat9 I can update to move the language prelude to rust-forever-stack.hxx if you still want the change
Thank you for making Rust GCC better!
If your PR fixes an issue, you can add "Fixes #issue_number" into this
PR description and the git commit message. This way the issue will be
automatically closed when your PR is merged. If your change addresses
an issue but does not fully fix it please mark it as "Addresses #issue_number"
in the git commit message.
Here is a checklist to help you with your PR.
make check-rustpasses locallyclang-formatgcc/testsuite/rust/Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
*Please write a comment explaining your change. This is the message
that will be part of the merge commit.