From 98f79026d96de6918199795c4886e345decdb221 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 | 7 ++++ gcc/rust/resolve/rust-forever-stack.hxx | 31 ++++++++++++++ .../resolve/rust-late-name-resolver-2.0.cc | 41 ++++++++++++------- .../resolve/rust-name-resolution-context.cc | 8 ++++ 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, 93 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..70520fbd196d 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,11 @@ template 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 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 a628c61aa088..3afb92141719 100644 --- a/gcc/rust/resolve/rust-forever-stack.hxx +++ b/gcc/rust/resolve/rust-forever-stack.hxx @@ -63,6 +63,15 @@ 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); + 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 +299,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) @@ -512,6 +535,14 @@ ForeverStack::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; 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 59cac8d02b7b..6caaa6562d19 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,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, &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 (); @@ -213,7 +219,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 +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 ()), diff --git a/gcc/rust/resolve/rust-name-resolution-context.cc b/gcc/rust/resolve/rust-name-resolution-context.cc index 1a70cd0cf121..98842103656e 100644 --- a/gcc/rust/resolve/rust-name-resolution-context.cc +++ b/gcc/rust/resolve/rust-name-resolution-context.cc @@ -107,6 +107,11 @@ NameResolutionContext::scoped (Rib::Kind rib_kind, NodeId id, std::function lambda, tl::optional 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); @@ -126,6 +131,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 6f6280d3b661..ed6ba1de8911 100644 --- a/gcc/testsuite/rust/compile/nr2/exclude +++ b/gcc/testsuite/rust/compile/nr2/exclude @@ -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