From 05aaeb8a6e01ba2fe7a40258be890104c75fb7fb Mon Sep 17 00:00:00 2001 From: Liam Naddell Date: Tue, 11 Feb 2025 16:42:41 -0500 Subject: [PATCH] Fix modules with same name as builtins causing ICE (#3315) 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 --- gcc/rust/resolve/rust-forever-stack.h | 11 ++++++ gcc/rust/resolve/rust-forever-stack.hxx | 32 ++++++++++++++++ .../resolve/rust-late-name-resolver-2.0.cc | 37 +++++++++++-------- .../resolve/rust-name-resolution-context.cc | 4 ++ gcc/rust/resolve/rust-rib.h | 5 +++ gcc/testsuite/rust/compile/issue-3315-1.rs | 8 ++++ gcc/testsuite/rust/compile/issue-3315-2.rs | 7 ++++ gcc/testsuite/rust/compile/nr2/exclude | 1 + 8 files changed, 90 insertions(+), 15 deletions(-) create mode 100644 gcc/testsuite/rust/compile/issue-3315-1.rs create mode 100644 gcc/testsuite/rust/compile/issue-3315-2.rs diff --git a/gcc/rust/resolve/rust-forever-stack.h b/gcc/rust/resolve/rust-forever-stack.h index 7bfe651edbef..fdece60a2c01 100644 --- a/gcc/rust/resolve/rust-forever-stack.h +++ b/gcc/rust/resolve/rust-forever-stack.h @@ -550,6 +550,7 @@ template 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 ()); @@ -651,6 +652,8 @@ template class ForeverStack * the current map, an empty one otherwise. */ tl::optional get (const Identifier &name); + tl::optional get_prelude (const Identifier &name); + tl::optional get_prelude (const std::string &name); /** * Resolve a path to its definition in the current `ForeverStack` @@ -750,7 +753,15 @@ template class ForeverStack const Node &cursor () const; void update_cursor (Node &new_cursor); + /* The forever stack's actual nodes */ Node root; + /* + * A special prelude node used currently for resolving language builtins + * It has the root node as a parent, and acts as a "special case" for name + * resolution + */ + Node prelude; + std::reference_wrapper cursor_reference; void stream_rib (std::stringstream &stream, const Rib &rib, diff --git a/gcc/rust/resolve/rust-forever-stack.hxx b/gcc/rust/resolve/rust-forever-stack.hxx index 5dd341397ef4..62929c26c625 100644 --- a/gcc/rust/resolve/rust-forever-stack.hxx +++ b/gcc/rust/resolve/rust-forever-stack.hxx @@ -63,6 +63,17 @@ template void ForeverStack::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); + // Prelude doesn't have an access path + rust_assert (!link.path); + 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 @@ -290,6 +301,20 @@ ForeverStack::get (const Identifier &name) return resolved_definition; } +template +tl::optional +ForeverStack::get_prelude (const Identifier &name) +{ + return prelude.rib.get (name.as_string ()); +} + +template +tl::optional +ForeverStack::get_prelude (const std::string &name) +{ + return prelude.rib.get (name); +} + template <> tl::optional inline ForeverStack::get ( const Identifier &name) @@ -539,6 +564,13 @@ ForeverStack::resolve_path ( if (segments.size () == 1) { auto res = get (unwrap_type_segment (segments.back ()).as_string ()); + if (!res) + { + // 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; diff --git a/gcc/rust/resolve/rust-late-name-resolver-2.0.cc b/gcc/rust/resolve/rust-late-name-resolver-2.0.cc index f7a36d8efbd5..109348f2c931 100644 --- a/gcc/rust/resolve/rust-late-name-resolver-2.0.cc +++ b/gcc/rust/resolve/rust-late-name-resolver-2.0.cc @@ -91,16 +91,18 @@ 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, &ty_ctx] (void) -> void { + for (const auto &builtin : builtins) + { + 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); + } + }); // ...here! auto *unit_type = TyTy::TupleType::get_unit_type (); @@ -213,7 +215,6 @@ Late::visit (AST::IdentifierExpr &expr) // TODO: same thing as visit(PathInExpression) here? tl::optional resolved = tl::nullopt; - if (auto value = ctx.values.get (expr.get_ident ())) { resolved = value; @@ -231,10 +232,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 ()), diff --git a/gcc/rust/resolve/rust-name-resolution-context.cc b/gcc/rust/resolve/rust-name-resolution-context.cc index 1a70cd0cf121..cde6e9f41402 100644 --- a/gcc/rust/resolve/rust-name-resolution-context.cc +++ b/gcc/rust/resolve/rust-name-resolution-context.cc @@ -107,6 +107,7 @@ NameResolutionContext::scoped (Rib::Kind rib_kind, NodeId id, std::function lambda, tl::optional path) { + // NOTE: You should be at the root node when calling this function. values.push (rib_kind, id, path); types.push (rib_kind, id, path); macros.push (rib_kind, id, path); @@ -126,6 +127,9 @@ NameResolutionContext::scoped (Rib::Kind rib_kind, Namespace ns, std::function lambda, tl::optional 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: diff --git a/gcc/rust/resolve/rust-rib.h b/gcc/rust/resolve/rust-rib.h index 9a046b2bef0d..dfa6235cd054 100644 --- a/gcc/rust/resolve/rust-rib.h +++ b/gcc/rust/resolve/rust-rib.h @@ -173,6 +173,11 @@ class Rib ForwardTypeParamBan, /* Const generic, as in the following example: fn foo() {} */ 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); diff --git a/gcc/testsuite/rust/compile/issue-3315-1.rs b/gcc/testsuite/rust/compile/issue-3315-1.rs new file mode 100644 index 000000000000..07581dab1170 --- /dev/null +++ b/gcc/testsuite/rust/compile/issue-3315-1.rs @@ -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 +} diff --git a/gcc/testsuite/rust/compile/issue-3315-2.rs b/gcc/testsuite/rust/compile/issue-3315-2.rs new file mode 100644 index 000000000000..71abd6cc1d71 --- /dev/null +++ b/gcc/testsuite/rust/compile/issue-3315-2.rs @@ -0,0 +1,7 @@ +mod i32 { +} + +fn main() -> isize { + let i:i32 = 0 as i32; + i as isize +} diff --git a/gcc/testsuite/rust/compile/nr2/exclude b/gcc/testsuite/rust/compile/nr2/exclude index 26b830eab7e9..8709751384d0 100644 --- a/gcc/testsuite/rust/compile/nr2/exclude +++ b/gcc/testsuite/rust/compile/nr2/exclude @@ -22,6 +22,7 @@ issue-2782.rs issue-2812.rs issue-850.rs issue-855.rs +issue-3315-2.rs iterators1.rs lookup_err1.rs macros/mbe/macro43.rs