[COMMITTED 57/83] gccrs: Cleanup bad usage of using ast node-ids instead of DefId for indexes

arthur.cohen@opensrcsec.com arthur.cohen@opensrcsec.com
Wed Sep 16 12:30:16 GMT 2026


From: Philip Herron <herron.philip@googlemail.com>

I used a node-id mapping just out of handyness because the trait-impls or
impls to defid's uses the name resolver to NodeIds but to use DefId we need
to do an indexing pass at the end of hir lowering then to get the DefId
mappings.

gcc/rust/ChangeLog:

	* hir/rust-ast-lower-item.cc (register_adt_impl): remove
	(ASTLoweringItem::visit): likewise
	* rust-session-manager.cc (Session::compile_crate): call new indexer
	* typecheck/rust-hir-dot-operator.cc
	(MethodResolver::try_select_predicate_candidates): use indexes
	* typecheck/rust-hir-path-probe-expr.cc (PathProbeExpr::probe_adt_impls): likewise
	* typecheck/rust-hir-path-probe-impl-trait.cc
	(PathProbeImplTrait::process_trait_impl_items_for_candidates): likewise
	* typecheck/rust-hir-path-probe-type.cc (TypePathProbe::probe_adt): likewise
	* typecheck/rust-tyty-bounds.cc (TypeBoundsProbe::scan): likewise
	* util/rust-hir-map.cc (Mappings::Mappings): new indexer
	(Mappings::insert_hir_impl_block): likewise
	(Mappings::insert_adt_impl_mapping): likewise
	(Mappings::build_impl_indexes): likewise
	(Mappings::insert_trait_impl_mapping): likewise
	(Mappings::insert_trait_item_mapping): likewise
	* util/rust-hir-map.h: likewise

Signed-off-by: Philip Herron <herron.philip@googlemail.com>
---
 gcc/rust/hir/rust-ast-lower-item.cc           | 30 -------
 gcc/rust/rust-session-manager.cc              |  1 +
 gcc/rust/typecheck/rust-hir-dot-operator.cc   |  9 +-
 .../typecheck/rust-hir-path-probe-expr.cc     |  6 +-
 .../rust-hir-path-probe-impl-trait.cc         |  4 +-
 .../typecheck/rust-hir-path-probe-type.cc     |  4 +-
 gcc/rust/typecheck/rust-tyty-bounds.cc        |  2 +-
 gcc/rust/util/rust-hir-map.cc                 | 87 +++++++++++++++----
 gcc/rust/util/rust-hir-map.h                  | 24 ++---
 9 files changed, 94 insertions(+), 73 deletions(-)

diff --git a/gcc/rust/hir/rust-ast-lower-item.cc b/gcc/rust/hir/rust-ast-lower-item.cc
index f7cbcf1ac28..2ae5242357f 100644
--- a/gcc/rust/hir/rust-ast-lower-item.cc
+++ b/gcc/rust/hir/rust-ast-lower-item.cc
@@ -27,31 +27,10 @@
 #include "rust-ast-lower-pattern.h"
 #include "rust-ast-lower-block.h"
 #include "rust-item.h"
-#include "rust-finalized-name-resolution-context.h"
 
 namespace Rust {
 namespace HIR {
 
-static void
-register_adt_impl (Analysis::Mappings &mappings, HIR::Type *impl_type,
-		   HIR::ImplBlock *impl)
-{
-  auto &nr_ctx = Resolver2_0::FinalizedNameResolutionContext::get ();
-  auto resolved = nr_ctx.lookup (impl_type->get_mappings ().get_nodeid (),
-				 Resolver2_0::Namespace::Types);
-  if (!resolved.has_value ())
-    return;
-
-  auto ast_item = mappings.lookup_ast_item (resolved.value ());
-  if (!ast_item.has_value ())
-    return;
-
-  auto kind = ast_item.value ()->get_item_kind ();
-  if (kind == AST::Item::Kind::Struct || kind == AST::Item::Kind::Enum
-      || kind == AST::Item::Kind::Union)
-    mappings.insert_adt_impl_mapping (resolved.value (), impl);
-}
-
 HIR::Item *
 ASTLoweringItem::translate (AST::Item &item)
 {
@@ -614,7 +593,6 @@ ASTLoweringItem::visit (AST::InherentImpl &impl_block)
   translated = hir_impl_block;
 
   mappings.insert_hir_impl_block (hir_impl_block);
-  register_adt_impl (mappings, impl_type, hir_impl_block);
   for (auto &impl_item_id : impl_item_ids)
     {
       mappings.insert_impl_item_mapping (impl_item_id, hir_impl_block);
@@ -788,14 +766,6 @@ ASTLoweringItem::visit (AST::TraitImpl &impl_block)
 
   mappings.insert_hir_impl_block (hir_impl_block);
 
-  register_adt_impl (mappings, impl_type, hir_impl_block);
-
-  auto &nr_ctx = Resolver2_0::FinalizedNameResolutionContext::get ();
-  auto trait_node_id = nr_ctx.lookup (trait_ref->get_mappings ().get_nodeid (),
-				      Resolver2_0::Namespace::Types);
-  rust_assert (trait_node_id.has_value ());
-  mappings.insert_trait_impl_mapping (trait_node_id.value (), hir_impl_block);
-
   for (auto &impl_item_id : impl_item_ids)
     {
       mappings.insert_impl_item_mapping (impl_item_id, hir_impl_block);
diff --git a/gcc/rust/rust-session-manager.cc b/gcc/rust/rust-session-manager.cc
index 29b12bb13b8..45fe3ede70c 100644
--- a/gcc/rust/rust-session-manager.cc
+++ b/gcc/rust/rust-session-manager.cc
@@ -792,6 +792,7 @@ Session::compile_crate (const char *filename)
 
   // add the mappings to it
   HIR::Crate &hir = mappings.insert_hir_crate (std::move (lowered));
+  mappings.build_impl_indexes ();
   if (options.dump_option_enabled (CompileOptions::HIR_DUMP))
     {
       dump_hir (hir);
diff --git a/gcc/rust/typecheck/rust-hir-dot-operator.cc b/gcc/rust/typecheck/rust-hir-dot-operator.cc
index a680a4aaaf9..89a596a2827 100644
--- a/gcc/rust/typecheck/rust-hir-dot-operator.cc
+++ b/gcc/rust/typecheck/rust-hir-dot-operator.cc
@@ -149,8 +149,7 @@ MethodResolver::assemble_inherent_impl_candidates (
       // see:
       // https://gcc-rust.zulipchat.com/#narrow/stream/266897-general/topic/Method.20Resolution/near/338646280
       // https://github.com/rust-lang/rust/blob/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/compiler/rustc_typeck/src/check/method/probe.rs#L650-L660
-      bool impl_self_is_ptr
-	= impl_self->get_kind () == TyTy::TypeKind::POINTER;
+      bool impl_self_is_ptr = impl_self->get_kind () == TyTy::TypeKind::POINTER;
       bool impl_self_is_ref = impl_self->get_kind () == TyTy::TypeKind::REF;
       if (receiver_is_raw_ptr && impl_self_is_ptr)
 	{
@@ -321,7 +320,7 @@ MethodResolver::assemble_trait_impl_candidates (
 						 process_impl);
   else
     mappings.iterate_trait_impl_blocks (
-      specified_trait->get_mappings ().get_nodeid (), process_impl);
+      specified_trait->get_mappings ().get_defid (), process_impl);
 }
 
 bool
@@ -333,8 +332,8 @@ MethodResolver::try_select_predicate_candidates (TyTy::BaseType &receiver)
       if (specified_trait != nullptr)
 	{
 	  const TraitReference *parent = predicate.lookup.get_parent ()->get ();
-	  if (parent->get_mappings ().get_nodeid ()
-	      != specified_trait->get_mappings ().get_nodeid ())
+	  if (parent->get_mappings ().get_defid ()
+	      != specified_trait->get_mappings ().get_defid ())
 	    continue;
 	}
 
diff --git a/gcc/rust/typecheck/rust-hir-path-probe-expr.cc b/gcc/rust/typecheck/rust-hir-path-probe-expr.cc
index 8307082af97..d95c931b1aa 100644
--- a/gcc/rust/typecheck/rust-hir-path-probe-expr.cc
+++ b/gcc/rust/typecheck/rust-hir-path-probe-expr.cc
@@ -76,9 +76,9 @@ PathProbeExpr::probe_adt_impls (TyTy::ADTType *adt)
       return;
     }
 
-  NodeId adt_node_id = adt_item.value ()->get_mappings ().get_nodeid ();
+  DefId adt_id = adt_item.value ()->get_mappings ().get_defid ();
   mappings.iterate_adt_impl_items (
-    adt_node_id,
+    adt_id,
     [this] (HirId id, HIR::ImplItem *item, HIR::ImplBlock *impl) -> bool {
       if (impl->has_trait_ref ())
 	return true;
@@ -90,7 +90,7 @@ PathProbeExpr::probe_adt_impls (TyTy::ADTType *adt)
     return;
 
   mappings.iterate_adt_impl_items (
-    adt_node_id,
+    adt_id,
     [this] (HirId id, HIR::ImplItem *item, HIR::ImplBlock *impl) -> bool {
       if (!impl->has_trait_ref ())
 	return true;
diff --git a/gcc/rust/typecheck/rust-hir-path-probe-impl-trait.cc b/gcc/rust/typecheck/rust-hir-path-probe-impl-trait.cc
index 7b6871fb69d..bd1edba2f04 100644
--- a/gcc/rust/typecheck/rust-hir-path-probe-impl-trait.cc
+++ b/gcc/rust/typecheck/rust-hir-path-probe-impl-trait.cc
@@ -44,8 +44,8 @@ PathProbeImplTrait::Probe (TyTy::BaseType *receiver,
 void
 PathProbeImplTrait::process_trait_impl_items_for_candidates ()
 {
-  NodeId trait_node_id = trait_reference->get_mappings ().get_nodeid ();
-  mappings.iterate_trait_impl_items (trait_node_id,
+  DefId trait_id = trait_reference->get_mappings ().get_defid ();
+  mappings.iterate_trait_impl_items (trait_id,
 				     [this] (HirId id, HIR::ImplItem *item,
 					     HIR::ImplBlock *impl) -> bool {
 				       process_impl_item_candidate (id, item,
diff --git a/gcc/rust/typecheck/rust-hir-path-probe-type.cc b/gcc/rust/typecheck/rust-hir-path-probe-type.cc
index 98cd7adc431..79179627dae 100644
--- a/gcc/rust/typecheck/rust-hir-path-probe-type.cc
+++ b/gcc/rust/typecheck/rust-hir-path-probe-type.cc
@@ -77,8 +77,8 @@ TypePathProbe::probe_adt (TyTy::ADTType *adt)
       return;
     }
 
-  NodeId adt_node_id = adt_item.value ()->get_mappings ().get_nodeid ();
-  mappings.iterate_adt_impl_items (adt_node_id,
+  DefId adt_id = adt_item.value ()->get_mappings ().get_defid ();
+  mappings.iterate_adt_impl_items (adt_id,
 				   [this] (HirId id, HIR::ImplItem *item,
 					   HIR::ImplBlock *impl) -> bool {
 				     return process_impl_item (id, item, impl);
diff --git a/gcc/rust/typecheck/rust-tyty-bounds.cc b/gcc/rust/typecheck/rust-tyty-bounds.cc
index ab5ce2a04cf..b4e30f16265 100644
--- a/gcc/rust/typecheck/rust-tyty-bounds.cc
+++ b/gcc/rust/typecheck/rust-tyty-bounds.cc
@@ -106,7 +106,7 @@ TypeBoundsProbe::scan ()
     mappings.iterate_trait_impl_blocks (process_impl);
   else
     mappings.iterate_trait_impl_blocks (
-      specified_trait->get_mappings ().get_nodeid (), process_impl);
+      specified_trait->get_mappings ().get_defid (), process_impl);
 
   for (auto &path : possible_trait_paths)
     {
diff --git a/gcc/rust/util/rust-hir-map.cc b/gcc/rust/util/rust-hir-map.cc
index 22cfba5eea3..1151c5cae19 100644
--- a/gcc/rust/util/rust-hir-map.cc
+++ b/gcc/rust/util/rust-hir-map.cc
@@ -26,6 +26,7 @@
 #include "rust-macro-builtins.h"
 #include "rust-mapping-common.h"
 #include "rust-attribute-values.h"
+#include "rust-finalized-name-resolution-context.h"
 
 namespace Rust {
 namespace Analysis {
@@ -99,7 +100,8 @@ static const HirId kDefaultCrateNumBegin = 0;
 
 Mappings::Mappings ()
   : crateNumItr (kDefaultCrateNumBegin), currentCrateNum (UNKNOWN_CRATENUM),
-    hirIdIter (kDefaultHirIdBegin), nodeIdIter (kDefaultNodeIdBegin)
+    hirIdIter (kDefaultHirIdBegin), nodeIdIter (kDefaultNodeIdBegin),
+    hirImplIndexesBuilt (false)
 {
   Analysis::NodeMapping node (0, 0, 0, 0);
   builtinMarker
@@ -487,7 +489,8 @@ Mappings::insert_hir_impl_block (HIR::ImplBlock *item)
 	    continue;
 
 	  auto name = function->get_function_name ().as_string ();
-	  hirInherentImplItemMappings[name].emplace_back (impl_item.get (), item);
+	  hirInherentImplItemMappings[name].emplace_back (impl_item.get (),
+							  item);
 	}
     }
   hirImplBlockTypeMappings[impl_type_id] = item;
@@ -845,15 +848,63 @@ Mappings::iterate_impl_items (
 }
 
 void
-Mappings::insert_adt_impl_mapping (NodeId adt_node_id, HIR::ImplBlock *impl)
+Mappings::insert_adt_impl_mapping (DefId adt_id, HIR::ImplBlock *impl)
 {
-  hirAdtImplMappings[adt_node_id].push_back (impl);
+  hirAdtImplMappings[adt_id].push_back (impl);
   hirIndexedAdtImpls.insert (impl);
 }
 
+void
+Mappings::build_impl_indexes ()
+{
+  gcc_checking_assert (!hirImplIndexesBuilt);
+  if (hirImplIndexesBuilt)
+    return;
+  hirImplIndexesBuilt = true;
+
+  auto &nr_ctx = Resolver2_0::FinalizedNameResolutionContext::get ();
+  auto resolve_item = [&] (const HIR::Type &type) -> HIR::Item * {
+    auto node_id = nr_ctx.lookup (type.get_mappings ().get_nodeid (),
+				  Resolver2_0::Namespace::Types);
+    if (!node_id.has_value ())
+      return nullptr;
+
+    auto hir_id = lookup_node_to_hir (node_id.value ());
+    gcc_checking_assert (hir_id.has_value ());
+    if (!hir_id.has_value ())
+      return nullptr;
+
+    auto item = lookup_hir_item (hir_id.value ());
+    return item.has_value () ? item.value () : nullptr;
+  };
+
+  iterate_impl_blocks ([&] (HirId, HIR::ImplBlock *impl) -> bool {
+    HIR::Item *self_item = resolve_item (impl->get_type ());
+    if (self_item != nullptr)
+      {
+	auto kind = self_item->get_item_kind ();
+	if (kind == HIR::Item::ItemKind::Struct
+	    || kind == HIR::Item::ItemKind::Enum
+	    || kind == HIR::Item::ItemKind::Union)
+	  insert_adt_impl_mapping (self_item->get_mappings ().get_defid (),
+				   impl);
+      }
+
+    if (impl->has_trait_ref ())
+      {
+	HIR::Item *trait_item = resolve_item (impl->get_trait_ref ());
+	if (trait_item != nullptr
+	    && trait_item->get_item_kind () == HIR::Item::ItemKind::Trait)
+	  insert_trait_impl_mapping (trait_item->get_mappings ().get_defid (),
+				     impl);
+      }
+    return true;
+  });
+}
+
 void
 Mappings::iterate_adt_impl_items (
-  NodeId adt_node_id,
+  DefId adt_id,
   std::function<bool (HirId, HIR::ImplItem *, HIR::ImplBlock *)> cb)
 {
   auto iterate_impl = [&] (HIR::ImplBlock *impl) {
@@ -867,7 +918,7 @@ Mappings::iterate_adt_impl_items (
     return true;
   };
 
-  auto adt_impls = hirAdtImplMappings.find (adt_node_id);
+  auto adt_impls = hirAdtImplMappings.find (adt_id);
   if (adt_impls != hirAdtImplMappings.end ())
     for (auto *impl : adt_impls->second)
       if (!iterate_impl (impl))
@@ -886,23 +937,22 @@ Mappings::iterate_adt_impl_items (
 }
 
 void
-Mappings::insert_trait_impl_mapping (NodeId trait_node_id, HIR::ImplBlock *impl)
+Mappings::insert_trait_impl_mapping (DefId trait_id, HIR::ImplBlock *impl)
 {
-  hirTraitImplMappings[trait_node_id].push_back (impl);
+  hirTraitImplMappings[trait_id].push_back (impl);
 }
 
 void
 Mappings::iterate_trait_impl_blocks_for_item (
-  const std::string &name,
-  std::function<bool (HirId, HIR::ImplBlock *)> cb)
+  const std::string &name, std::function<bool (HirId, HIR::ImplBlock *)> cb)
 {
   auto traits = hirTraitItemNameMappings.find (name);
   if (traits == hirTraitItemNameMappings.end ())
     return;
 
-  for (auto trait_node_id : traits->second)
+  for (auto trait_id : traits->second)
     {
-      auto impls = hirTraitImplMappings.find (trait_node_id);
+      auto impls = hirTraitImplMappings.find (trait_id);
       if (impls == hirTraitImplMappings.end ())
 	continue;
 
@@ -921,8 +971,7 @@ Mappings::insert_trait_item_mapping (HirId trait_item_id, HIR::Trait *trait)
 
   auto item = lookup_hir_trait_item (trait_item_id);
   rust_assert (item.has_value ());
-  if (item.value ()->get_item_kind ()
-      != HIR::TraitItem::TraitItemKind::FUNC)
+  if (item.value ()->get_item_kind () != HIR::TraitItem::TraitItemKind::FUNC)
     return;
 
   auto *function = static_cast<HIR::TraitItemFunc *> (item.value ());
@@ -931,15 +980,15 @@ Mappings::insert_trait_item_mapping (HirId trait_item_id, HIR::Trait *trait)
 
   auto name = function->get_decl ().get_function_name ().as_string ();
   hirTraitItemNameMappings[name].push_back (
-    trait->get_mappings ().get_nodeid ());
+    trait->get_mappings ().get_defid ());
 }
 
 void
 Mappings::iterate_trait_impl_items (
-  NodeId trait_node_id,
+  DefId trait_id,
   std::function<bool (HirId, HIR::ImplItem *, HIR::ImplBlock *)> cb)
 {
-  auto trait_impls = hirTraitImplMappings.find (trait_node_id);
+  auto trait_impls = hirTraitImplMappings.find (trait_id);
   if (trait_impls == hirTraitImplMappings.end ())
     return;
 
@@ -955,9 +1004,9 @@ Mappings::iterate_trait_impl_items (
 
 void
 Mappings::iterate_trait_impl_blocks (
-  NodeId trait_node_id, std::function<bool (HirId, HIR::ImplBlock *)> cb)
+  DefId trait_id, std::function<bool (HirId, HIR::ImplBlock *)> cb)
 {
-  auto trait_impls = hirTraitImplMappings.find (trait_node_id);
+  auto trait_impls = hirTraitImplMappings.find (trait_id);
   if (trait_impls == hirTraitImplMappings.end ())
     return;
 
diff --git a/gcc/rust/util/rust-hir-map.h b/gcc/rust/util/rust-hir-map.h
index 4a2a1a8fbad..344e13414fe 100644
--- a/gcc/rust/util/rust-hir-map.h
+++ b/gcc/rust/util/rust-hir-map.h
@@ -220,20 +220,22 @@ public:
 	return;
   }
 
-  void insert_adt_impl_mapping (NodeId adt_node_id, HIR::ImplBlock *impl);
+  void insert_adt_impl_mapping (DefId adt_id, HIR::ImplBlock *impl);
+
+  void build_impl_indexes ();
 
   void iterate_adt_impl_items (
-    NodeId adt_node_id,
+    DefId adt_id,
     std::function<bool (HirId, HIR::ImplItem *, HIR::ImplBlock *)> cb);
 
-  void insert_trait_impl_mapping (NodeId trait_node_id, HIR::ImplBlock *impl);
+  void insert_trait_impl_mapping (DefId trait_id, HIR::ImplBlock *impl);
 
   void iterate_trait_impl_items (
-    NodeId trait_node_id,
+    DefId trait_id,
     std::function<bool (HirId, HIR::ImplItem *, HIR::ImplBlock *)> cb);
 
   void
-  iterate_trait_impl_blocks (NodeId trait_node_id,
+  iterate_trait_impl_blocks (DefId trait_id,
 			     std::function<bool (HirId, HIR::ImplBlock *)> cb);
 
   template <typename Callback> void iterate_trait_impl_blocks (Callback &&cb)
@@ -244,9 +246,8 @@ public:
 	return;
   }
 
-  void iterate_trait_impl_blocks_for_item (const std::string &name,
-					  std::function<bool (
-					    HirId, HIR::ImplBlock *)> cb);
+  void iterate_trait_impl_blocks_for_item (
+    const std::string &name, std::function<bool (HirId, HIR::ImplBlock *)> cb);
 
   void iterate_impl_blocks (std::function<bool (HirId, HIR::ImplBlock *)> cb);
 
@@ -435,10 +436,11 @@ private:
 	   std::vector<std::pair<HIR::ImplItem *, HIR::ImplBlock *>>>
     hirInherentImplItemMappings;
   std::map<HirId, HIR::ImplBlock *> hirImplBlockTypeMappings;
-  std::map<NodeId, std::vector<HIR::ImplBlock *>> hirAdtImplMappings;
+  std::map<DefId, std::vector<HIR::ImplBlock *>> hirAdtImplMappings;
   std::set<HIR::ImplBlock *> hirIndexedAdtImpls;
-  std::map<NodeId, std::vector<HIR::ImplBlock *>> hirTraitImplMappings;
-  std::map<std::string, std::vector<NodeId>> hirTraitItemNameMappings;
+  std::map<DefId, std::vector<HIR::ImplBlock *>> hirTraitImplMappings;
+  std::map<std::string, std::vector<DefId>> hirTraitItemNameMappings;
+  bool hirImplIndexesBuilt;
   std::map<HirId, HIR::TraitItem *> hirTraitItemMappings;
   std::map<HirId, HIR::ExternBlock *> hirExternBlockMappings;
   std::map<HirId, std::pair<HIR::ExternalItem *, HirId>> hirExternItemMappings;
-- 
2.50.1



More information about the Gcc-rust mailing list