From a865b40aef15505945770779158d17b066a7a84e 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 | 12 ++++ gcc/rust/resolve/rust-forever-stack.hxx | 72 ++++++++++++++++--- .../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, 123 insertions(+), 23 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..1215b726035d 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` @@ -715,6 +718,7 @@ template class ForeverStack {} bool is_root () const; + bool is_prelude () const; bool is_leaf () const; void insert_child (Link link, Node child); @@ -750,7 +754,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 65dd02aa84d8..8923f15bbaf1 100644 --- a/gcc/rust/resolve/rust-forever-stack.hxx +++ b/gcc/rust/resolve/rust-forever-stack.hxx @@ -34,6 +34,13 @@ ForeverStack::Node::is_root () const return !parent.has_value (); } +template +bool +ForeverStack::Node::is_prelude () const +{ + return rib.kind == Rib::Kind::Prelude; +} + template bool ForeverStack::Node::is_leaf () const @@ -63,6 +70,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 +308,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) @@ -389,7 +421,7 @@ ForeverStack::find_starting_point ( break; auto &seg = unwrap_type_segment (outer_seg); - auto is_self_or_crate + bool is_self_or_crate = seg.is_crate_path_seg () || seg.is_lower_self_seg (); // if we're after the first path segment and meet `self` or `crate`, it's @@ -452,7 +484,7 @@ ForeverStack::resolve_segments ( typename std::vector::const_iterator iterator, std::function insert_segment_resolution) { - auto *current_node = &starting_point; + Node *current_node = &starting_point; for (; !is_last (iterator, segments); iterator++) { auto &outer_seg = *iterator; @@ -468,7 +500,7 @@ ForeverStack::resolve_segments ( } auto &seg = unwrap_type_segment (outer_seg); - auto str = seg.as_string (); + std::string str = seg.as_string (); rust_debug ("[ARTHUR]: resolving segment part: %s", str.c_str ()); // check that we don't encounter *any* leading keywords afterwards @@ -487,6 +519,7 @@ ForeverStack::resolve_segments ( * * This ensures termination */ + bool searched_prelude = false; while (true) { // may set the value of child @@ -522,9 +555,16 @@ ForeverStack::resolve_segments ( } } + if (current_node->is_root () && !searched_prelude) + { + searched_prelude = true; + current_node = &prelude; + continue; + } + if (!is_start (iterator, segments) || current_node->rib.kind == Rib::Kind::Module - || current_node->is_root ()) + || current_node->is_prelude ()) { return tl::nullopt; } @@ -564,7 +604,14 @@ ForeverStack::resolve_path ( return Rib::Definition::NonShadowable (seg_id); } - auto res = get (unwrap_type_segment (segments.back ()).as_string ()); + tl::optional 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; @@ -579,16 +626,25 @@ ForeverStack::resolve_path ( return resolve_segments (starting_point.get (), segments, iterator, insert_segment_resolution); }) - .and_then ([&segments, &insert_segment_resolution] ( + .and_then ([this, &segments, &insert_segment_resolution] ( Node final_node) -> tl::optional { // leave resolution within impl blocks to type checker if (final_node.rib.kind == Rib::Kind::TraitOrImpl) return tl::nullopt; + + std::string seg_name + = unwrap_type_segment (segments.back ()).as_string (); + // assuming this can't be a lang item segment - auto res = final_node.rib.get ( - unwrap_type_segment (segments.back ()).as_string ()); + tl::optional res = final_node.rib.get (seg_name); + + // Ok we didn't find it in the rib, Lets try the prelude... + if (!res) + res = get_prelude (seg_name); + 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 02da99de52f8..56ec0dde03b6 100644 --- a/gcc/testsuite/rust/compile/nr2/exclude +++ b/gcc/testsuite/rust/compile/nr2/exclude @@ -18,6 +18,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