[gccrs COMMIT 5/5] gccrs: Cleanup bad usage of using ast node-ids instead of DefId for indexes
gerris.rs@gmail.com
gerris.rs@gmail.com
Sun Sep 6 19:15:27 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>
---
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/32c84030f87e29d0ddf080ff746dcd131b909c70
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/4847
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 f7cbcf1ac..2ae524235 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 29b12bb13..45fe3ede7 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 a680a4aaa..89a596a28 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 8307082af..d95c931b1 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 7b6871fb6..bd1edba2f 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 98cd7adc4..79179627d 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 ab5ce2a04..b4e30f162 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 22cfba5ee..1151c5cae 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 4a2a1a8fb..344e13414 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.55.0
More information about the Gcc-rust
mailing list