From 20d5a774ab3f6aabd1d0703ddc1000945689e8bc Mon Sep 17 00:00:00 2001 From: Owen Avery Date: Tue, 21 Jan 2025 17:02:35 -0500 Subject: [PATCH 1/2] Check for type paths nr2.0 can't handle yet Some of our tests only work with name resolution 2.0 because the latter misinterprets type paths. This change should cause the compiler to error out if it would otherwise misinterpret a type path. A fix for type path resolution isn't included in this comment, since doing so would make it harder to track the meaningfulness of test regressions. gcc/rust/ChangeLog: * resolve/rust-late-name-resolver-2.0.cc (Late::visit): Error out if a type path has multiple segments, as we currently ignore every segment except the last. gcc/testsuite/ChangeLog: * rust/compile/nr2/exclude: Add entries. Signed-off-by: Owen Avery --- gcc/rust/resolve/rust-late-name-resolver-2.0.cc | 8 ++++++++ gcc/testsuite/rust/compile/nr2/exclude | 15 +++++++++++++++ 2 files changed, 23 insertions(+) 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 a7408fbe7b6a..7c6948565202 100644 --- a/gcc/rust/resolve/rust-late-name-resolver-2.0.cc +++ b/gcc/rust/resolve/rust-late-name-resolver-2.0.cc @@ -287,6 +287,14 @@ Late::visit (AST::TypePath &type) // maybe we can overload `resolve_path` to only do // typepath-like path resolution? that sounds good + if (type.get_segments ().size () != 1) + { + rust_sorry_at ( + type.get_locus (), + "name resolution 2.0 cannot resolve multi-segment type paths"); + return; + } + auto str = type.get_segments ().back ()->get_ident_segment ().as_string (); auto values = ctx.types.peek ().get_values (); diff --git a/gcc/testsuite/rust/compile/nr2/exclude b/gcc/testsuite/rust/compile/nr2/exclude index d53c14871731..5cc7cf7d64c5 100644 --- a/gcc/testsuite/rust/compile/nr2/exclude +++ b/gcc/testsuite/rust/compile/nr2/exclude @@ -58,6 +58,7 @@ iterators1.rs lookup_err1.rs macros/mbe/macro13.rs macros/mbe/macro15.rs +macros/mbe/macro20.rs macros/mbe/macro23.rs macros/mbe/macro40.rs macros/mbe/macro43.rs @@ -151,4 +152,18 @@ derive_macro6.rs issue-2987.rs issue-3139-1.rs issue-3139-3.rs +issue-1019.rs +issue-1034.rs +issue-2019-1.rs +issue-2019-2.rs +issue-2019-3.rs +issue-2105.rs +issue-2190-1.rs +issue-2190-2.rs +issue-2304.rs +issue-2747.rs +issue-2953-1.rs +issue-3030.rs +traits12.rs +try-trait.rs # please don't delete the trailing newline From 0fbbe18033f5fa74f257b8baf1be1f4453d57064 Mon Sep 17 00:00:00 2001 From: Owen Avery Date: Wed, 22 Jan 2025 18:10:22 -0500 Subject: [PATCH 2/2] stash --- gcc/rust/resolve/rust-forever-stack.h | 85 +--- gcc/rust/resolve/rust-forever-stack.hxx | 371 ++++++------------ .../resolve/rust-name-resolution-context.cc | 4 +- .../resolve/rust-name-resolution-context.h | 1 + 4 files changed, 126 insertions(+), 335 deletions(-) diff --git a/gcc/rust/resolve/rust-forever-stack.h b/gcc/rust/resolve/rust-forever-stack.h index 8fc46ad603f5..44d0cb958d14 100644 --- a/gcc/rust/resolve/rust-forever-stack.h +++ b/gcc/rust/resolve/rust-forever-stack.h @@ -394,7 +394,7 @@ this pass's documentation for more details on this resolution process. **/ /** - * Intended for use by ForeverStack to store Nodes + * Used by ForeverStack to store Nodes * Unlike ForeverStack, does not store a cursor reference * Intended to make path resolution in multiple namespaces simpler **/ @@ -546,21 +546,17 @@ class ForeverStackStore template class ForeverStack { public: - ForeverStack () + ForeverStack (ForeverStackStore &base) // 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)), - cursor_reference (root) - { - rust_assert (root.is_root ()); - rust_assert (root.is_leaf ()); - } + : base (base), cursor_reference (base.get_root ()) + {} /** * Add a new Rib to the stack. If the Rib already exists, nothing is pushed * and the stack's cursor is simply moved to this existing Rib. * - * @param rib The Rib to push + * @param rib_kind The kind of Rib to push * @param id The NodeId of the node for which the Rib was created. For * example, if a Rib is created because a lexical scope is entered, * then `id` is that `BlockExpr`'s NodeId. @@ -679,61 +675,8 @@ template class ForeverStack bool is_module_descendant (NodeId parent, NodeId child) const; private: - /** - * A link between two Nodes in our trie data structure. This class represents - * the edges of the graph - */ - class Link - { - public: - Link (NodeId id, tl::optional path) : id (id), path (path) {} - - bool compare (const Link &other) const { return id < other.id; } - - NodeId id; - tl::optional path; - }; - - /* Link comparison class, which we use in a Node's `children` map */ - class LinkCmp - { - public: - bool operator() (const Link &lhs, const Link &rhs) const - { - return lhs.compare (rhs); - } - }; - - class Node - { - public: - Node (Rib rib, NodeId id) : rib (rib), id (id) {} - Node (Rib rib, NodeId id, Node &parent) - : rib (rib), id (id), parent (parent) - {} - - bool is_root () const; - bool is_leaf () const; - - void insert_child (Link link, Node child); - - Rib rib; // this is the "value" of the node - the data it keeps. - std::map children; // all the other nodes it links to - - NodeId id; // The node id of the Node's scope - - tl::optional parent; // `None` only if the node is a root - }; - - /* Should we keep going upon seeing a Rib? */ - enum class KeepGoing - { - Yes, - No, - }; - - /* Add a new Rib to the stack. This is an internal method */ - void push_inner (Rib rib, Link link); + using Node = ForeverStackStore::Node; + using KeepGoing = ForeverStackStore::KeepGoing; /* Reverse iterate on `Node`s from the cursor, in an outwards fashion */ void reverse_iter (std::function lambda); @@ -748,7 +691,7 @@ template class ForeverStack const Node &cursor () const; void update_cursor (Node &new_cursor); - Node root; + std::reference_wrapper base; std::reference_wrapper cursor_reference; void stream_rib (std::stringstream &stream, const Rib &rib, @@ -774,16 +717,8 @@ template class ForeverStack SegIterator iterator); /* Helper functions for forward resolution (to_canonical_path, to_rib...) */ - struct DfsResult - { - Node &first; - std::string second; - }; - struct ConstDfsResult - { - const Node &first; - std::string second; - }; + using DfsResult = ForeverStackStore::DfsResult; + using ConstDfsResult = ForeverStackStore::ConstDfsResult; // FIXME: Documentation tl::optional dfs (Node &starting_point, NodeId to_find); diff --git a/gcc/rust/resolve/rust-forever-stack.hxx b/gcc/rust/resolve/rust-forever-stack.hxx index 6181c05fc6c5..5f3874c756ea 100644 --- a/gcc/rust/resolve/rust-forever-stack.hxx +++ b/gcc/rust/resolve/rust-forever-stack.hxx @@ -26,60 +26,12 @@ namespace Rust { namespace Resolver2_0 { -template -bool -ForeverStack::Node::is_root () const -{ - return !parent.has_value (); -} - -template -bool -ForeverStack::Node::is_leaf () const -{ - return children.empty (); -} - -template -void -ForeverStack::Node::insert_child (Link link, Node child) -{ - auto res = children.insert ({link, child}); - - // Do we want to error if the child already exists? Probably not, right? - // That's kinda the point, isn't it. So this method always succeeds, right? -} - template void ForeverStack::push (Rib::Kind rib_kind, NodeId id, tl::optional path) { - push_inner (rib_kind, Link (id, path)); -} - -template -void -ForeverStack::push_inner (Rib rib, Link link) -{ - // 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 - // points to it. Otherwise, it points to the newly emplaced value, so we can - // just update our cursor(). - auto emplace = cursor ().children.emplace ( - std::make_pair (link, Node (rib, link.id, cursor ()))); - - auto it = emplace.first; - auto existed = !emplace.second; - - rust_debug ("inserting link: Link(%d [%s]): existed? %s", link.id, - link.path.has_value () ? link.path.value ().as_string ().c_str () - : "", - existed ? "yes" : "no"); - - // We update the cursor - update_cursor (it->second); + update_cursor (cursor ().insert_child (id, std::move (path), rib_kind)); } template @@ -90,107 +42,76 @@ ForeverStack::pop () rust_debug ("popping link"); - for (const auto &kv : cursor ().rib.get_values ()) + for (const auto &kv : cursor ().get_rib (N).get_values ()) rust_debug ("current_rib: k: %s, v: %s", kv.first.c_str (), kv.second.to_string ().c_str ()); - if (cursor ().parent.has_value ()) - for (const auto &kv : cursor ().parent.value ().rib.get_values ()) - rust_debug ("new cursor: k: %s, v: %s", kv.first.c_str (), - kv.second.to_string ().c_str ()); - - update_cursor (cursor ().parent.value ()); -} + if (cursor ().get_parent ().has_value ()) + { + auto &parent = cursor ().get_parent ().value (); + auto &rib_values = parent.get_rib (N).get_values (); + for (const auto &kv : rib_values) + rust_debug ("new cursor: k: %s, v: %s", kv.first.c_str (), + kv.second.to_string ().c_str ()); + } -static tl::expected -insert_inner (Rib &rib, std::string name, Rib::Definition definition) -{ - return rib.insert (name, definition); + update_cursor (cursor ().get_parent ().value ()); } template tl::expected ForeverStack::insert (Identifier name, NodeId node) { - auto &innermost_rib = peek (); - - // So what do we do here - if the Rib has already been pushed in an earlier - // pass, we might end up in a situation where it is okay to re-add new names. - // Do we just ignore that here? Do we keep track of if the Rib is new or not? - // should our cursor have info on the current node like "is it newly pushed"? - return insert_inner (innermost_rib, name.as_string (), - Rib::Definition::NonShadowable (node)); + // Handle Macros and Labels - where we are allowed to shadow + // existing definitions + if (N == Namespace::Labels || N == Namespace::Macros) + return cursor ().insert_shadowable (name, node, N); + else + return cursor ().insert (name, node, N); } template tl::expected ForeverStack::insert_shadowable (Identifier name, NodeId node) { - auto &innermost_rib = peek (); - - return insert_inner (innermost_rib, name.as_string (), - Rib::Definition::Shadowable (node)); + return cursor ().insert_shadowable (name, node, N); } template tl::expected ForeverStack::insert_globbed (Identifier name, NodeId node) { - auto &innermost_rib = peek (); - - return insert_inner (innermost_rib, name.as_string (), - Rib::Definition::Globbed (node)); + return cursor ().insert_globbed (name, node, N); } template tl::expected ForeverStack::insert_at_root (Identifier name, NodeId node) { - auto &root_rib = root.rib; - // inserting in the root of the crate is never a shadowing operation, even for // macros - return insert_inner (root_rib, name.as_string (), - Rib::Definition::NonShadowable (node)); -} - -// Specialization for Macros and Labels - where we are allowed to shadow -// existing definitions -template <> -inline tl::expected -ForeverStack::insert (Identifier name, NodeId node) -{ - return insert_inner (peek (), name.as_string (), - Rib::Definition::Shadowable (node)); -} - -template <> -inline tl::expected -ForeverStack::insert (Identifier name, NodeId node) -{ - return insert_inner (peek (), name.as_string (), - Rib::Definition::Shadowable (node)); + return base.get ().get_root ().insert_shadowable (name, node, N); } template Rib & ForeverStack::peek () { - return cursor ().rib; + return cursor ().get_rib (N); } template const Rib & ForeverStack::peek () const { - return cursor ().rib; + return cursor ().get_rib (N); } template void ForeverStack::reverse_iter (std::function lambda) { - return reverse_iter (cursor (), lambda); + cursor ().reverse_iter (std::move (lambda)); } template @@ -198,7 +119,7 @@ void ForeverStack::reverse_iter ( std::function lambda) const { - return reverse_iter (cursor (), lambda); + cursor ().reverse_iter (std::move (lambda)); } template @@ -206,19 +127,7 @@ void ForeverStack::reverse_iter (Node &start, std::function lambda) { - auto *tmp = &start; - - while (true) - { - auto keep_going = lambda (*tmp); - if (keep_going == KeepGoing::No) - return; - - if (tmp->is_root ()) - return; - - tmp = &tmp->parent.value (); - } + start.reverse_iter (std::move (lambda)); } template @@ -226,19 +135,7 @@ void ForeverStack::reverse_iter ( const Node &start, std::function lambda) const { - auto *tmp = &start; - - while (true) - { - auto keep_going = lambda (*tmp); - if (keep_going == KeepGoing::No) - return; - - if (tmp->is_root ()) - return; - - tmp = &tmp->parent.value (); - } + start.reverse_iter (std::move (lambda)); } template @@ -270,7 +167,7 @@ ForeverStack::get (const Identifier &name) // TODO: Can we improve the API? have `reverse_iter` return an optional? reverse_iter ([&resolved_definition, &name] (Node ¤t) { - auto candidate = current.rib.get (name.as_string ()); + auto candidate = current.get_rib (N).get (name.as_string ()); return candidate.map_or ( [&resolved_definition] (Rib::Definition found) { @@ -298,10 +195,11 @@ tl::optional inline ForeverStack::get ( reverse_iter ([&resolved_definition, &name] (Node ¤t) { // looking up for labels cannot go through function ribs // TODO: What other ribs? - if (current.rib.kind == Rib::Kind::Function) + if (current.get_rib (Namespace::Labels).kind == Rib::Kind::Function) return KeepGoing::No; - auto candidate = current.rib.get (name.as_string ()); + auto candidate + = current.get_rib (Namespace::Labels).get (name.as_string ()); // FIXME: Factor this in a function with the generic `get` return candidate.map_or ( @@ -336,19 +234,7 @@ template typename ForeverStack::Node & ForeverStack::find_closest_module (Node &starting_point) { - auto *closest_module = &starting_point; - - reverse_iter (starting_point, [&closest_module] (Node ¤t) { - if (current.rib.kind == Rib::Kind::Module || current.is_root ()) - { - closest_module = ¤t; - return KeepGoing::No; - } - - return KeepGoing::Yes; - }); - - return *closest_module; + return starting_point.find_closest_module (); } /* If a the given condition is met, emit an error about misused leading path @@ -400,7 +286,7 @@ ForeverStack::find_starting_point ( if (seg.is_crate_path_seg ()) { - starting_point = root; + starting_point = base.get ().get_root (); iterator++; break; } @@ -419,8 +305,8 @@ ForeverStack::find_starting_point ( return tl::nullopt; } - starting_point - = find_closest_module (starting_point.get ().parent.value ()); + starting_point = find_closest_module ( + starting_point.get ().get_parent ().value ()); continue; } @@ -453,23 +339,8 @@ ForeverStack::resolve_segments ( || seg.is_lower_self_seg ())) return tl::nullopt; - tl::optional::Node &> child = tl::nullopt; - - for (auto &kv : current_node->children) - { - auto &link = kv.first; - - if (link.path.map_or ( - [&str] (Identifier path) { - auto &path_str = path.as_string (); - return str == path_str; - }, - false)) - { - child = kv.second; - break; - } - } + auto child = current_node->get_child ( + Identifier (seg.as_string (), seg.get_locus ())); if (!child.has_value ()) { @@ -503,7 +374,7 @@ ForeverStack::resolve_path (const std::vector &segments) return resolve_segments (starting_point.get (), segments, iterator); }) .and_then ([&segments] (Node final_node) { - return final_node.rib.get (segments.back ().as_string ()); + return final_node.get_rib (N).get (segments.back ().as_string ()); }); } @@ -511,7 +382,7 @@ template tl::optional::DfsResult> ForeverStack::dfs (ForeverStack::Node &starting_point, NodeId to_find) { - auto values = starting_point.rib.get_values (); + auto values = starting_point.get_rib (N).get_values (); for (auto &kv : values) { @@ -526,15 +397,24 @@ ForeverStack::dfs (ForeverStack::Node &starting_point, NodeId to_find) return {{starting_point, kv.first}}; } - for (auto &child : starting_point.children) - { - auto candidate = dfs (child.second, to_find); + tl::optional ret; + starting_point.child_iter ( + [this, &ret, to_find] (NodeId id, tl::optional path, + Node &child) { + auto candidate = dfs (child, to_find); if (candidate.has_value ()) - return candidate; - } + { + ret.emplace (std::move (candidate.value ())); + return KeepGoing::No; + } + else + { + return KeepGoing::Yes; + } + }); - return tl::nullopt; + return ret; } template @@ -542,7 +422,7 @@ tl::optional::ConstDfsResult> ForeverStack::dfs (const ForeverStack::Node &starting_point, NodeId to_find) const { - auto values = starting_point.rib.get_values (); + auto values = starting_point.get_rib (N).get_values (); for (auto &kv : values) { @@ -557,15 +437,24 @@ ForeverStack::dfs (const ForeverStack::Node &starting_point, return {{starting_point, kv.first}}; } - for (auto &child : starting_point.children) - { - auto candidate = dfs (child.second, to_find); + tl::optional ret; + starting_point.child_iter ( + [this, &ret, to_find] (NodeId id, tl::optional path, + const Node &child) { + auto candidate = dfs (child, to_find); if (candidate.has_value ()) - return candidate; - } + { + ret.emplace (std::move (candidate.value ())); + return KeepGoing::No; + } + else + { + return KeepGoing::Yes; + } + }); - return tl::nullopt; + return ret; } template @@ -577,51 +466,35 @@ ForeverStack::to_canonical_path (NodeId id) const // back up to the root (parent().parent().parent()...) accumulate link // segments reverse them that's your canonical path - return dfs (root, id).map ([this, id] (ConstDfsResult tuple) { - auto containing_node = tuple.first; - auto name = tuple.second; - - auto segments = std::vector (); - - reverse_iter (containing_node, [&segments] (const Node ¤t) { - if (current.is_root ()) - return KeepGoing::No; - - auto children = current.parent.value ().children; - const Link *outer_link = nullptr; + return dfs (base.get ().get_root (), id) + .map ([this, id] (ConstDfsResult tuple) { + auto containing_node = tuple.first; + auto name = tuple.second; - for (auto &kv : children) - { - auto &link = kv.first; - auto &child = kv.second; + auto segments = std::vector (); - if (link.id == child.id) - { - outer_link = &link; - break; - } - } + reverse_iter (containing_node, [&segments] (const Node ¤t) { + if (current.is_root ()) + return KeepGoing::No; - rust_assert (outer_link); + auto link_path = current.get_parent_path (); + if (link_path.has_value ()) + segments.emplace (segments.begin (), + Resolver::CanonicalPath::new_seg ( + current.get_id (), link_path->as_string ())); - outer_link->path.map ([&segments, outer_link] (Identifier path) { - segments.emplace (segments.begin (), - Resolver::CanonicalPath::new_seg (outer_link->id, - path.as_string ())); + return KeepGoing::Yes; }); - return KeepGoing::Yes; - }); - - auto path = Resolver::CanonicalPath::create_empty (); - for (const auto &segment : segments) - path = path.append (segment); + auto path = Resolver::CanonicalPath::create_empty (); + for (const auto &segment : segments) + path = path.append (segment); - // Finally, append the name - path = path.append (Resolver::CanonicalPath::new_seg (id, name)); + // Finally, append the name + path = path.append (Resolver::CanonicalPath::new_seg (id, name)); - return path; - }); + return path; + }); } template @@ -629,7 +502,7 @@ tl::optional ForeverStack::dfs_rib (ForeverStack::Node &starting_point, NodeId to_find) { return dfs_node (starting_point, to_find).map ([] (Node &x) -> Rib & { - return x.rib; + return x.get_rib (N); }); } @@ -639,7 +512,7 @@ ForeverStack::dfs_rib (const ForeverStack::Node &starting_point, NodeId to_find) const { return dfs_node (starting_point, to_find).map ([] (Node &x) -> Rib & { - return x.rib; + return x.get_rib (N); }); } @@ -648,18 +521,7 @@ tl::optional::Node &> ForeverStack::dfs_node (ForeverStack::Node &starting_point, NodeId to_find) { - if (starting_point.id == to_find) - return starting_point; - - for (auto &child : starting_point.children) - { - auto candidate = dfs_node (child.second, to_find); - - if (candidate.has_value ()) - return candidate; - } - - return tl::nullopt; + return starting_point.dfs_node (to_find); } template @@ -667,32 +529,21 @@ tl::optional::Node &> ForeverStack::dfs_node (const ForeverStack::Node &starting_point, NodeId to_find) const { - if (starting_point.id == to_find) - return starting_point; - - for (auto &child : starting_point.children) - { - auto candidate = dfs_node (child.second, to_find); - - if (candidate.has_value ()) - return candidate; - } - - return tl::nullopt; + return starting_point.dfs_node (to_find); } template tl::optional ForeverStack::to_rib (NodeId rib_id) { - return dfs_rib (root, rib_id); + return dfs_rib (base.get ().get_root (), rib_id); } template tl::optional ForeverStack::to_rib (NodeId rib_id) const { - return dfs_rib (root, rib_id); + return dfs_rib (base.get ().get_root (), rib_id); } template @@ -729,23 +580,24 @@ ForeverStack::stream_node (std::stringstream &stream, unsigned indentation, << next << "is_leaf: " << (node.is_leaf () ? "true" : "false") << ",\n"; - stream_rib (stream, node.rib, next, next_next); + stream_rib (stream, node.get_rib (N), next, next_next); stream << indent << "}\n"; - for (auto &kv : node.children) - { - auto link = kv.first; - auto child = kv.second; - stream << indent << "Link (" << link.id << ", " - << (link.path.has_value () ? link.path.value ().as_string () - : "") - << "):\n"; + node.child_iter ([this, &stream, &indent, + indentation] (NodeId id, + tl::optional path, + const Node &child) { + stream << indent << "Link (" << id << ", " + << (path.has_value () ? path.value ().as_string () : "") + << "):\n"; - stream_node (stream, indentation + 4, child); + stream_node (stream, indentation + 4, child); - stream << '\n'; - } + stream << '\n'; + + return KeepGoing::Yes; + }); } template @@ -754,7 +606,7 @@ ForeverStack::as_debug_string () { std::stringstream stream; - stream_node (stream, 0, root); + stream_node (stream, 0, base.get ().get_root ()); return stream.str (); } @@ -763,7 +615,8 @@ template bool ForeverStack::is_module_descendant (NodeId parent, NodeId child) const { - return dfs_node (dfs_node (root, parent).value (), child).has_value (); + return dfs_node (dfs_node (base.get ().get_root (), parent).value (), child) + .has_value (); } // FIXME: Can we add selftests? diff --git a/gcc/rust/resolve/rust-name-resolution-context.cc b/gcc/rust/resolve/rust-name-resolution-context.cc index 1a70cd0cf121..91252af3932f 100644 --- a/gcc/rust/resolve/rust-name-resolution-context.cc +++ b/gcc/rust/resolve/rust-name-resolution-context.cc @@ -24,7 +24,9 @@ namespace Rust { namespace Resolver2_0 { NameResolutionContext::NameResolutionContext () - : mappings (Analysis::Mappings::get ()) + : node_store (UNKNOWN_NODEID), values (node_store), types (node_store), + macros (node_store), labels (node_store), + mappings (Analysis::Mappings::get ()) {} tl::expected diff --git a/gcc/rust/resolve/rust-name-resolution-context.h b/gcc/rust/resolve/rust-name-resolution-context.h index 44d7da7981de..add4efd9b830 100644 --- a/gcc/rust/resolve/rust-name-resolution-context.h +++ b/gcc/rust/resolve/rust-name-resolution-context.h @@ -203,6 +203,7 @@ class NameResolutionContext std::function lambda, tl::optional path = {}); + ForeverStackStore node_store; ForeverStack values; ForeverStack types; ForeverStack macros;