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

Modules with same names as built in types causes ICE #3315

Open
matthewjasper opened this issue Dec 21, 2024 · 11 comments
Open

Modules with same names as built in types causes ICE #3315

matthewjasper opened this issue Dec 21, 2024 · 11 comments

Comments

@matthewjasper
Copy link
Contributor

Code

mod i32 {}

Meta

Version: gccrs (Compiler-Explorer-Build-gcc-b5c354d038f800695a8d730c56c4a4f744134adb-binutils-2.42) 14.0.1 20240309 (experimental)

Error output

crab1: internal compiler error: in setup_builtin_types, at rust/resolve/rust-late-name-resolver-2.0.cc:97
0x27eb5ec internal_error(char const*, ...)
	???:0
0xaf6cc9 fancy_abort(char const*, int, char const*)
	???:0
0xdbb522 Rust::Resolver2_0::Late::go(Rust::AST::Crate&)
	???:0
0xc4bef0 Rust::Session::compile_crate(char const*)
	???:0
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
Compiler returned: 1
@liamnaddell
Copy link
Contributor

From reading here: https://doc.rust-lang.org/reference/names/namespaces.html

builtins and modules both belong in the Types namespace, however, that can't really be correct, since rustc allows you to name a module i32. I guess the idea is that builtins are resolved separately from other types? Otherwise, how would you prevent NR conflicts here?

@liamnaddell
Copy link
Contributor

liamnaddell commented Feb 3, 2025

Also, look at this error:

liam@newgame ~/projects/gccrs/gccrs-build $ rustc main.rs
...
error[E0605]: non-primitive cast: `i32` as `i32`
 --> main.rs:4:18
  |
4 |     let a: i32 = 0 as i32;
  |                  ^^^^^^^^ an `as` expression can only be used to convert between primitive types or to coerce to a specific trait object

error: aborting due to 1 previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0605`.
liam@newgame ~/projects/gccrs/gccrs-build $ cat main.rs
struct i32 {}

fn main() {
    let a: i32 = 0 as i32;
}
liam@newgame ~/projects/gccrs/gccrs-build $

What happens here is that i32 gets shadowed to the struct, which causes a compile error, since i32 no longer refers to a primative type.

@liamnaddell
Copy link
Contributor

liamnaddell commented Feb 3, 2025

I guess what we are expected to do is remove builtin types from the resolution context when another Item overshadows it

EDIT: I tried this, but then i32 doesn't exist, and this rustc code (which is supposed to compile) no longer compiles:

mod i32 {}

fn main() {
    let a: i32 = 0 as i32;
}

@liamnaddell
Copy link
Contributor

liamnaddell commented Feb 3, 2025

Modules are clearly handled separately from other types like structs. I think modules are somehow part of their own namespace?

EDIT: I checked rustc, structs, builtins, modules, etc are all in the Types namespace, at least at rustc's HIR level. They end up going into different RIB's though. I'm not really sure how you'd make this work yet though

@powerboat9
Copy link
Collaborator

It looks like builtins are really supposed to go in a language prelude. That way, module definitions can shadow builtin definitions by virtue of being in a different scope.

@liamnaddell
Copy link
Contributor

It looks like builtins are really supposed to go in a language prelude. That way, module definitions can shadow builtin definitions by virtue of being in a different scope.

Would this be part of a distinct RIB?

@powerboat9
Copy link
Collaborator

powerboat9 commented Feb 3, 2025

Yep. Fixing this would probably entail some adjustments to the ForeverStack node hierarchy, and some code for handling descent into prelude nodes (since we'll want multiple preludes).

@liamnaddell
Copy link
Contributor

Yep. Fixing this would probably entail some adjustments to the ForeverStack node hierarchy, and some code for handling descent into prelude nodes (since we'll want multiple preludes).

Wouldn't the prelude end up being handled as an import? I.e. not having a distinct node?

https://github.com/rust-lang/rust/blob/8ad2c9724d983cfb116baab0bb800edd17f31644/compiler/rustc_resolve/src/imports.rs#L75
https://github.com/rust-lang/rust/blob/8ad2c9724d983cfb116baab0bb800edd17f31644/compiler/rustc_resolve/src/lib.rs#L1021

@powerboat9
Copy link
Collaborator

powerboat9 commented Feb 8, 2025

There are multiple preludes, some of which should probably have their own nodes

@liamnaddell
Copy link
Contributor

liamnaddell commented Feb 11, 2025

I'm giving this one a try currently. Solving the issue seems easy enough, since you just replace insert with insert_globbed when builtins are added during late resolution.

Some of the secondary problems are more challenging though.

  1. You aren't able to use builtin types, since self::i32 isn't supposed to exist (theres a rust spec reference for this, I forgot where though). Of course, if you use std::i32, then you MAY reference self::i32
  2. structs and other proper types shadow builtin types for type resolution
  3. modules DON'T shadow BUILTIN types for type resolution (this is despite both modules and structs going in the same namespace ;-;)
  4. modules cannot coexist with other conflicting items in the same namespace (u can't have struct A and mod A)

I'm trying to see currently how many of these I can solve presently

@powerboat9
Copy link
Collaborator

Modules should be inserted into the type namespace, which would solve 4. We might just need a separate rib/node to store builtin types, and some adjustments to path resolution.

liamnaddell added a commit to liamnaddell/gccrs that referenced this issue Feb 11, 2025
liamnaddell added a commit to liamnaddell/gccrs that referenced this issue Feb 12, 2025
gcc/rust/ChangeLog:

	* resolve/rust-forever-stack.h: idk
	* resolve/rust-forever-stack.hxx: idk
	* resolve/rust-late-name-resolver-2.0.cc (Late::visit): idk

gcc/testsuite/ChangeLog:

	* rust/compile/issue-3315-1.rs: idk
	* rust/compile/issue-3315-2.rs: your mom
liamnaddell added a commit to liamnaddell/gccrs that referenced this issue Feb 12, 2025
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.rs: Add test for module with same name
	as builtin
liamnaddell added a commit to liamnaddell/gccrs that referenced this issue Feb 14, 2025
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
liamnaddell added a commit to liamnaddell/gccrs that referenced this issue Feb 14, 2025
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]>
liamnaddell added a commit to liamnaddell/gccrs that referenced this issue Feb 14, 2025
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]>
liamnaddell added a commit to liamnaddell/gccrs that referenced this issue Feb 14, 2025
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]>
liamnaddell added a commit to liamnaddell/gccrs that referenced this issue Feb 15, 2025
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]>
liamnaddell added a commit to liamnaddell/gccrs that referenced this issue Feb 15, 2025
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]>
liamnaddell added a commit to liamnaddell/gccrs that referenced this issue Feb 15, 2025
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]>
liamnaddell added a commit to liamnaddell/gccrs that referenced this issue Feb 15, 2025
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

4 participants