From: Arthur Cohen <[email protected]>

This now uses find_leaf_definition to better resolve macro definitions
to their actual definitions instead of a possible import, and likewise for
invocations. This also improves the robustness and error checking for
resolving definitions.

gcc/rust/ChangeLog:

        * resolve/rust-early-name-resolver-2.0.cc (Early::insert_once): 
Rename...
        (Early::try_insert_once): ...to this, and improve logic.
        (Early::go): Use new API.
        (Early::visit): Likewise.
        (Early::finalize_simple_import): Likewise.
        (Early::finalize_rebind_import): Likewise.
        * resolve/rust-early-name-resolver-2.0.h: Declare the new API.
---
This change was merged into the gccrs repository and is posted here for
upstream visibility and potential drive-by review, as requested by GCC
release managers.
Each commit email contains a link to its details on github from where you can
find the Pull-Request and associated discussions.


Commit on github: 
https://github.com/Rust-GCC/gccrs/commit/114ee6442a0eb0b40f5db39cc0afc522ab4f0ca2

The commit has NOT been mentioned in any issue.

The commit has been mentioned in the following pull-request(s):
 - https://github.com/Rust-GCC/gccrs/pull/4551

 .../resolve/rust-early-name-resolver-2.0.cc   | 25 +++++++++++++------
 .../resolve/rust-early-name-resolver-2.0.h    |  2 +-
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/gcc/rust/resolve/rust-early-name-resolver-2.0.cc 
b/gcc/rust/resolve/rust-early-name-resolver-2.0.cc
index 2109ab263..8ba452d66 100644
--- a/gcc/rust/resolve/rust-early-name-resolver-2.0.cc
+++ b/gcc/rust/resolve/rust-early-name-resolver-2.0.cc
@@ -37,10 +37,17 @@ Early::Early (NameResolutionContext &ctx)
 {}
 
 void
-Early::insert_once (AST::MacroInvocation &invocation, NodeId resolved)
+Early::try_insert_once (AST::MacroInvocation &invocation, NodeId resolved)
 {
-  // TODO: Should we use `ctx.mark_resolved()`?
-  auto definition = ctx.mappings.lookup_macro_def (resolved);
+  auto leaf_macro = ctx.find_leaf_definition (resolved);
+
+  // Sometimes the import itself isn't resolved yet this turn of the 
fixed-point
+  if (!leaf_macro)
+    return;
+
+  // TODO: Should we use `ctx.map_usage()`?
+
+  auto definition = ctx.mappings.lookup_macro_def (leaf_macro->id);
 
   if (!ctx.mappings.lookup_macro_invocation (invocation))
     ctx.mappings.insert_macro_invocation (invocation, definition.value ());
@@ -49,7 +56,6 @@ Early::insert_once (AST::MacroInvocation &invocation, NodeId 
resolved)
 void
 Early::insert_once (AST::MacroRulesDefinition &def)
 {
-  // TODO: Should we use `ctx.mark_resolved()`?
   if (!ctx.mappings.lookup_macro_def (def.get_node_id ()))
     ctx.mappings.insert_macro_def (&def);
 }
@@ -64,6 +70,7 @@ Early::go (AST::Crate &crate)
   // us
 
   dirty = toplevel.is_dirty ();
+
   // We now proceed with resolving macros, which can be nested in almost any
   // items
   textual_scope.push ();
@@ -345,7 +352,7 @@ Early::visit (AST::MacroInvocation &invoc)
       return;
     }
 
-  insert_once (invoc, definition->get_node_id ());
+  try_insert_once (invoc, definition->get_node_id ());
 
   // now do we need to keep mappings or something? or insert "uses" into our
   // ForeverStack? can we do that? are mappings simpler?
@@ -447,7 +454,7 @@ Early::finalize_simple_import (const Early::ImportPair 
&mapping)
 
   for (auto &&definition : data.definitions ())
     {
-      dirty = true;
+      // dirty = true;
 
       ctx.map_usage (Usage (import_id),
                     Definition (definition.first.get_node_id ()));
@@ -455,6 +462,8 @@ Early::finalize_simple_import (const Early::ImportPair 
&mapping)
       toplevel
        .insert_or_error_out (identifier,
                              import.get_locus (), import_id, definition.second 
/* TODO: This isn't clear - it would be better if it was called .ns or 
something */);
+
+      dirty = dirty || toplevel.is_dirty ();
     }
 }
 
@@ -526,7 +535,7 @@ Early::finalize_rebind_import (const Early::ImportPair 
&mapping)
 
   for (auto &&definition : data.definitions ())
     {
-      dirty = true;
+      // dirty = true;
 
       ctx.map_usage (Usage (import_id),
                     Definition (definition.first.get_node_id ()));
@@ -535,6 +544,8 @@ Early::finalize_rebind_import (const Early::ImportPair 
&mapping)
        .insert_or_error_out (declared_name,
                              path.get_locus (), import_id, definition.second 
/* TODO: This isn't clear - it would be better if it was called .ns or 
something */);
 
+      dirty = dirty || toplevel.is_dirty ();
+
       // Map the import to the glob container if it exists - this is important
       // for 2-stepped glob imports which refer to glob containers, e.g.
       //
diff --git a/gcc/rust/resolve/rust-early-name-resolver-2.0.h 
b/gcc/rust/resolve/rust-early-name-resolver-2.0.h
index 0fdf1436c..ff3542ce5 100644
--- a/gcc/rust/resolve/rust-early-name-resolver-2.0.h
+++ b/gcc/rust/resolve/rust-early-name-resolver-2.0.h
@@ -181,7 +181,7 @@ private:
    * and it will not trigger assertions for already resolved invocations.
    */
   // TODO: Rename
-  void insert_once (AST::MacroInvocation &invocation, NodeId resolved);
+  void try_insert_once (AST::MacroInvocation &invocation, NodeId resolved);
   // TODO: Rename
   void insert_once (AST::MacroRulesDefinition &definition);
 
-- 
2.54.0

Reply via email to