Skip to content

Commit

Permalink
Fix modules with same name as builtins causing ICE (#3315)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
liamnaddell committed Feb 14, 2025
1 parent 1eb4620 commit 484b8d3
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 15 deletions.
7 changes: 7 additions & 0 deletions gcc/rust/resolve/rust-forever-stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,7 @@ template <Namespace N> class ForeverStack
// FIXME: Is that valid? Do we use the root? If yes, we should give the
// crate's node id to ForeverStack's constructor
: root (Node (Rib (Rib::Kind::Normal), UNKNOWN_NODEID)),
prelude (Node (Rib (Rib::Kind::Prelude), UNKNOWN_NODEID, root)),
cursor_reference (root)
{
rust_assert (root.is_root ());
Expand Down Expand Up @@ -651,6 +652,8 @@ template <Namespace N> class ForeverStack
* the current map, an empty one otherwise.
*/
tl::optional<Rib::Definition> get (const Identifier &name);
tl::optional<Rib::Definition> get_prelude (const Identifier &name);
tl::optional<Rib::Definition> get_prelude (const std::string &name);

/**
* Resolve a path to its definition in the current `ForeverStack`
Expand Down Expand Up @@ -750,7 +753,11 @@ template <Namespace N> class ForeverStack
const Node &cursor () const;
void update_cursor (Node &new_cursor);

/* The forever stack's actual nodes */
Node root;
/* TODO: Document */
Node prelude;

std::reference_wrapper<Node> cursor_reference;

void stream_rib (std::stringstream &stream, const Rib &rib,
Expand Down
31 changes: 31 additions & 0 deletions gcc/rust/resolve/rust-forever-stack.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ template <Namespace N>
void
ForeverStack<N>::push_inner (Rib rib, Link link)
{
if (rib.kind == Rib::Kind::Prelude)
{
// If you push_inner into the prelude from outside the root, you will pop
// back into the
// root, which could screw up a traversal.
rust_assert (&cursor_reference.get () == &root);
update_cursor (this->prelude);
return;
}
// If the link does not exist, we create it and emplace a new `Node` with the
// current node as its parent. `unordered_map::emplace` returns a pair with
// the iterator and a boolean. If the value already exists, the iterator
Expand Down Expand Up @@ -290,6 +299,20 @@ ForeverStack<N>::get (const Identifier &name)
return resolved_definition;
}

template <Namespace N>
tl::optional<Rib::Definition>
ForeverStack<N>::get_prelude (const Identifier &name)
{
return prelude.rib.get (name.as_string ());
}

template <Namespace N>
tl::optional<Rib::Definition>
ForeverStack<N>::get_prelude (const std::string &name)
{
return prelude.rib.get (name);
}

template <>
tl::optional<Rib::Definition> inline ForeverStack<Namespace::Labels>::get (
const Identifier &name)
Expand Down Expand Up @@ -512,6 +535,14 @@ ForeverStack<N>::resolve_path (
if (segments.size () == 1)
{
auto res = get (unwrap_type_segment (segments.back ()).as_string ());
if (!res)
{
// idk maybe try in the prelude then :/
// NOTE: one day we may need to support multisegment resolution in the
// prelude.
res
= get_prelude (unwrap_type_segment (segments.back ()).as_string ());
}
if (res && !res->is_ambiguous ())
insert_segment_resolution (segments.back (), res->get_node_id ());
return res;
Expand Down
41 changes: 26 additions & 15 deletions gcc/rust/resolve/rust-late-name-resolver-2.0.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,22 @@ Late::setup_builtin_types ()
// insert it in the type context...
};

for (const auto &builtin : builtins)
{
// we should be able to use `insert_at_root` or `insert` here, since we're
// at the root :) hopefully!
auto ok = ctx.types.insert (builtin.name, builtin.node_id);
rust_assert (ok);

ctx.mappings.insert_node_to_hir (builtin.node_id, builtin.hir_id);
ty_ctx.insert_builtin (builtin.hir_id, builtin.node_id, builtin.type);
}
// There's a special Rib for putting prelude items, since prelude items need
// to satisfy certain special rules.
ctx.scoped (Rib::Kind::Prelude, 0, [this, &builtins, &ty_ctx] (void) -> void {
for (const auto &builtin : builtins)
{
// we should be able to use `insert_at_root` or `insert` here, since
// 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);
rust_assert (ok);

ctx.mappings.insert_node_to_hir (builtin.node_id, builtin.hir_id);
ty_ctx.insert_builtin (builtin.hir_id, builtin.node_id, builtin.type);
}
});

// ...here!
auto *unit_type = TyTy::TupleType::get_unit_type ();
Expand Down Expand Up @@ -213,7 +219,6 @@ Late::visit (AST::IdentifierExpr &expr)
// TODO: same thing as visit(PathInExpression) here?

tl::optional<Rib::Definition> resolved = tl::nullopt;

if (auto value = ctx.values.get (expr.get_ident ()))
{
resolved = value;
Expand All @@ -231,10 +236,16 @@ Late::visit (AST::IdentifierExpr &expr)
}
else
{
rust_error_at (expr.get_locus (),
"could not resolve identifier expression: %qs",
expr.get_ident ().as_string ().c_str ());
return;
if (auto typ = ctx.types.get_prelude (expr.get_ident ()))
{
resolved = typ;
}
else
{
rust_error_at (expr.get_locus (),
"could not resolve identifier expression: %qs",
expr.get_ident ().as_string ().c_str ());
}
}

ctx.map_usage (Usage (expr.get_node_id ()),
Expand Down
8 changes: 8 additions & 0 deletions gcc/rust/resolve/rust-name-resolution-context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
if (rib_kind == Rib::Kind::Prelude)
rust_assert (!path);

values.push (rib_kind, id, path);
types.push (rib_kind, id, path);
macros.push (rib_kind, id, path);
Expand All @@ -126,6 +131,9 @@ NameResolutionContext::scoped (Rib::Kind rib_kind, Namespace ns,
std::function<void (void)> lambda,
tl::optional<Identifier> path)
{
// This could work... I'm not sure why you would want to do this though.
rust_assert (rib_kind != Rib::Kind::Prelude);

switch (ns)
{
case Namespace::Values:
Expand Down
5 changes: 5 additions & 0 deletions gcc/rust/resolve/rust-rib.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,11 @@ class Rib
ForwardTypeParamBan,
/* Const generic, as in the following example: fn foo<T, const X: T>() {} */
ConstParamType,
/* Prelude rib, used for both the language prelude (i32,usize,etc) and the
* (future) {std,core}::prelude::* import. A regular rib with the
* restriction that you cannot `use` items from the Prelude
*/
Prelude,
} kind;

Rib (Kind kind);
Expand Down
8 changes: 8 additions & 0 deletions gcc/testsuite/rust/compile/issue-3315-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//You should be able to create a module of the same name as a builtin type

mod i32 {
}

fn main() -> isize {
0
}
7 changes: 7 additions & 0 deletions gcc/testsuite/rust/compile/issue-3315-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
mod i32 {
}

fn main() -> isize {
let i:i32 = 0 as i32;
i as isize
}
1 change: 1 addition & 0 deletions gcc/testsuite/rust/compile/nr2/exclude
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ issue-2782.rs
issue-2812.rs
issue-850.rs
issue-855.rs
issue-3315-2.rs
iterators1.rs
lookup_err1.rs
macros/mbe/macro20.rs
Expand Down

0 comments on commit 484b8d3

Please sign in to comment.