https://gcc.gnu.org/g:93203063a9994900730fe9596546d6a8c5da6cda
commit r17-1877-g93203063a9994900730fe9596546d6a8c5da6cda Author: Enes Cevik <[email protected]> Date: Sun May 3 15:02:08 2026 +0300 gccrs: resolve: Use definition IDs directly and fix ICEs This patch fixes multiple ICEs and test failures in the Name Resolution 2.0 - Map directly to definition IDs instead of intermediate import IDs. - Remove redundant `flatten()` calls. Flattening stripped generic arguments from aliases like `Self`, causing typecheck errors. - Prevent ICEs on invalid `use self;` imports by adding early returns. - Add `Namespace::Types` to pattern lookups to correctly emit E0532 instead of crashing. - Remove an unsafe debug loop in `Rib::get` that caused ICEs on ambiguous names. - Use `value_or` for safe optional unwrapping in module resolution. gcc/rust/ChangeLog: * checks/lints/rust-lint-marklive.cc (MarkLive::visit_path_segment): Add Types namespace lookup. * checks/lints/unused/rust-unused-checker.cc (UnusedChecker::visit): Use safe optional unwrapping. * checks/lints/unused/rust-unused-collector.cc (UnusedCollector::visit): Likewise. * checks/lints/unused/rust-unused-collector.h: Use helper for multiple namespace lookups. * resolve/rust-early-name-resolver-2.0.cc (Early::finalize_simple_import): Prevent ICE on single self imports. (Early::finalize_rebind_import): Return early to prevent ICE. * resolve/rust-name-resolution-context.hxx: Use value_or for safe leaf module lookup. * rust-session-manager.cc (Session::compile_crate): Remove obsolete flatten() calls. * typecheck/rust-hir-type-check-pattern.cc (TypeCheckPattern::visit): Look up enum variants in Types namespace. Signed-off-by: Enes Cevik <[email protected]> Diff: --- gcc/rust/checks/lints/rust-lint-marklive.cc | 7 +++- .../checks/lints/unused/rust-unused-checker.cc | 22 ++++++---- .../checks/lints/unused/rust-unused-collector.cc | 11 +++-- .../checks/lints/unused/rust-unused-collector.h | 48 ++++++++++++++++++---- gcc/rust/resolve/rust-early-name-resolver-2.0.cc | 8 ++-- gcc/rust/resolve/rust-name-resolution-context.hxx | 15 +++++-- gcc/rust/rust-session-manager.cc | 6 --- gcc/rust/typecheck/rust-hir-type-check-pattern.cc | 8 ++-- 8 files changed, 84 insertions(+), 41 deletions(-) diff --git a/gcc/rust/checks/lints/rust-lint-marklive.cc b/gcc/rust/checks/lints/rust-lint-marklive.cc index 3998cb70f08f..8fc81b3631fa 100644 --- a/gcc/rust/checks/lints/rust-lint-marklive.cc +++ b/gcc/rust/checks/lints/rust-lint-marklive.cc @@ -165,8 +165,11 @@ MarkLive::visit_path_segment (HIR::PathExprSegment seg) // We should mark them alive all and ignoring other kind of segments. // If the segment we dont care then just return false is fine // TODO: Should we look that up in all namespaces? - if (auto id = resolver.lookup (ast_node_id, Resolver2_0::Namespace::Values)) - ref_node_id = *id; + + if (auto nslookup + = resolver.lookup (ast_node_id, Resolver2_0::Namespace::Values, + Resolver2_0::Namespace::Types)) + ref_node_id = nslookup->id; else return false; if (auto hid = mappings.lookup_node_to_hir (ref_node_id)) diff --git a/gcc/rust/checks/lints/unused/rust-unused-checker.cc b/gcc/rust/checks/lints/unused/rust-unused-checker.cc index 06bbfd446a24..571e31e430ae 100644 --- a/gcc/rust/checks/lints/unused/rust-unused-checker.cc +++ b/gcc/rust/checks/lints/unused/rust-unused-checker.cc @@ -105,21 +105,25 @@ UnusedChecker::visit (HIR::IdentifierPattern &pattern) "variable %qs should have a snake case name", var_name.c_str ()); } -void +void UnusedChecker::visit (HIR::AssignmentExpr &expr) - { const auto &lhs = expr.get_lhs (); auto var_name = lhs.to_string (); NodeId ast_node_id = lhs.get_mappings ().get_nodeid (); - NodeId def_id - = nr_context.lookup (ast_node_id, Resolver2_0::Namespace::Values).value (); - HirId id = mappings.lookup_node_to_hir (def_id).value (); - if (unused_context.is_variable_assigned (id, lhs.get_mappings ().get_hirid ()) - && var_name[0] != '_') - rust_warning_at (lhs.get_locus (), OPT_Wunused_variable, - "unused assignment %qs", var_name.c_str ()); + if (auto def_id + = nr_context.lookup (ast_node_id, Resolver2_0::Namespace::Values)) + { + if (auto id = mappings.lookup_node_to_hir (*def_id)) + { + if (unused_context.is_variable_assigned ( + *id, lhs.get_mappings ().get_hirid ()) + && var_name[0] != '_') + rust_warning_at (lhs.get_locus (), OPT_Wunused_variable, + "unused assignment %qs", var_name.c_str ()); + } + } } void diff --git a/gcc/rust/checks/lints/unused/rust-unused-collector.cc b/gcc/rust/checks/lints/unused/rust-unused-collector.cc index 5596dd97a90b..96920efb94ce 100644 --- a/gcc/rust/checks/lints/unused/rust-unused-collector.cc +++ b/gcc/rust/checks/lints/unused/rust-unused-collector.cc @@ -62,10 +62,13 @@ UnusedCollector::visit (HIR::StructExprFieldIdentifier &ident) void UnusedCollector::visit (HIR::AssignmentExpr &expr) { - auto def_id = get_def_id (expr.get_lhs (), Resolver2_0::Namespace::Values); - HirId id = expr.get_lhs ().get_mappings ().get_hirid (); - unused_context.remove_mut (def_id); - unused_context.add_assign (def_id, id); + if (auto def_id + = get_def_id (expr.get_lhs (), Resolver2_0::Namespace::Values)) + { + HirId id = expr.get_lhs ().get_mappings ().get_hirid (); + unused_context.remove_mut (*def_id); + unused_context.add_assign (*def_id, id); + } visit_outer_attrs (expr); expr.get_rhs ().accept_vis (*this); } diff --git a/gcc/rust/checks/lints/unused/rust-unused-collector.h b/gcc/rust/checks/lints/unused/rust-unused-collector.h index 5059f76892d2..a506bff8000e 100644 --- a/gcc/rust/checks/lints/unused/rust-unused-collector.h +++ b/gcc/rust/checks/lints/unused/rust-unused-collector.h @@ -57,25 +57,55 @@ private: virtual void visit (HIR::ContinueExpr &expr) override; template <typename T> - HirId get_def_id (T &path_expr, Resolver2_0::Namespace ns) + tl::optional<HirId> get_def_id (T &path_expr, Resolver2_0::Namespace ns) { NodeId ast_node_id = path_expr.get_mappings ().get_nodeid (); - NodeId id = nr_context.lookup (ast_node_id, ns).value (); - HirId def_id = mappings.lookup_node_to_hir (id).value (); - return def_id; + + if (auto id = nr_context.lookup (ast_node_id, ns)) + { + if (auto def_id = mappings.lookup_node_to_hir (*id)) + { + return *def_id; + } + } + return tl::nullopt; + } + + template <typename T> + tl::optional<HirId> get_def_id (T &path_expr, Resolver2_0::Namespace ns1, + Resolver2_0::Namespace ns2) + { + NodeId ast_node_id = path_expr.get_mappings ().get_nodeid (); + + if (auto nslookup = nr_context.lookup (ast_node_id, ns1, ns2)) + { + NodeId id = nslookup->id; + + if (auto def_id = mappings.lookup_node_to_hir (id)) + { + return *def_id; + } + } + + return tl::nullopt; } template <typename T> void mark_path_used (T &path_expr) { - auto def_id = get_def_id (path_expr, Resolver2_0::Namespace::Values); - unused_context.add_variable (def_id); - unused_context.remove_assign (def_id); + if (auto def_id = get_def_id (path_expr, Resolver2_0::Namespace::Values, + Resolver2_0::Namespace::Types)) + { + unused_context.add_variable (*def_id); + unused_context.remove_assign (*def_id); + } } template <typename T> void mark_label_used (T &path_expr) { - auto def_id = get_def_id (path_expr, Resolver2_0::Namespace::Labels); - unused_context.add_label (def_id); + if (auto def_id = get_def_id (path_expr, Resolver2_0::Namespace::Labels)) + { + unused_context.add_label (*def_id); + } } }; } // namespace Analysis 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 148385300e9f..1d63b63920ff 100644 --- a/gcc/rust/resolve/rust-early-name-resolver-2.0.cc +++ b/gcc/rust/resolve/rust-early-name-resolver-2.0.cc @@ -467,7 +467,8 @@ Early::finalize_simple_import (const Early::ImportPair &mapping) Definition (definition.definition.get_node_id ()), definition.ns); - toplevel.insert_or_error_out (identifier, import.get_locus (), import_id, + toplevel.insert_or_error_out (identifier, import.get_locus (), + definition.definition.get_node_id (), definition.ns); dirty = dirty || toplevel.is_dirty (); @@ -523,7 +524,7 @@ Early::finalize_rebind_import (const Early::ImportPair &mapping) { // Erroneous `self` or `{self}` use declaration if (segments.size () == 1) - break; + return; declared_name = segments[segments.size () - 2].as_string (); import_id = segments[segments.size () - 2].get_node_id (); } @@ -548,7 +549,8 @@ Early::finalize_rebind_import (const Early::ImportPair &mapping) Definition (definition.definition.get_node_id ()), definition.ns); - toplevel.insert_or_error_out (declared_name, path.get_locus (), import_id, + toplevel.insert_or_error_out (declared_name, path.get_locus (), + definition.definition.get_node_id (), definition.ns); dirty = dirty || toplevel.is_dirty (); diff --git a/gcc/rust/resolve/rust-name-resolution-context.hxx b/gcc/rust/resolve/rust-name-resolution-context.hxx index c16bae87443e..359dd9c65e5b 100644 --- a/gcc/rust/resolve/rust-name-resolution-context.hxx +++ b/gcc/rust/resolve/rust-name-resolution-context.hxx @@ -353,13 +353,20 @@ NameResolutionContext::resolve_segments ( .lookup_glob_container (rib_lookup->get_node_id ()) .has_value ()) { - auto leaf_module + NodeId leaf_module = stack.find_leaf_definition (rib_lookup->get_node_id ()) - .value () + .value_or (Definition (rib_lookup->get_node_id ())) .id; - child = stack.dfs_node (stack.root, leaf_module).value (); - break; + if (auto new_child = stack.dfs_node (stack.root, leaf_module)) + { + child = new_child.value (); + break; + } + else + { + return tl::nullopt; + } } else { diff --git a/gcc/rust/rust-session-manager.cc b/gcc/rust/rust-session-manager.cc index b3bc99249647..5eda91da9f2a 100644 --- a/gcc/rust/rust-session-manager.cc +++ b/gcc/rust/rust-session-manager.cc @@ -764,15 +764,9 @@ Session::compile_crate (const char *filename) if (last_step == CompileOptions::CompileStep::NameResolution) return; - // FIXME: Or run it within Late at the end? - name_resolution_ctx.flatten (); - // resolution pipeline stage Resolver2_0::Late (name_resolution_ctx).go (parsed_crate); - // FIXME: Or run it within Late at the end? - name_resolution_ctx.flatten (); - if (options.dump_option_enabled (CompileOptions::RESOLUTION_DUMP)) dump_name_resolution (name_resolution_ctx); diff --git a/gcc/rust/typecheck/rust-hir-type-check-pattern.cc b/gcc/rust/typecheck/rust-hir-type-check-pattern.cc index 8c99790f4d92..7d12bc7b4554 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-pattern.cc +++ b/gcc/rust/typecheck/rust-hir-type-check-pattern.cc @@ -57,11 +57,11 @@ TypeCheckPattern::visit (HIR::PathInExpression &pattern) auto &nr_ctx = Resolver2_0::FinalizedNameResolutionContext::get (); - // FIXME: Not sure if this is enough to look up enum variants - if (auto id = nr_ctx.lookup (pattern.get_mappings ().get_nodeid (), - Resolver2_0::Namespace::Values)) + if (auto nslookup = nr_ctx.lookup (pattern.get_mappings ().get_nodeid (), + Resolver2_0::Namespace::Values, + Resolver2_0::Namespace::Types)) { - ref_node_id = *id; + ref_node_id = nslookup->id; maybe_item = true; }
