https://gcc.gnu.org/g:6b0faf2946a390d5e1493391f5f1eed71b469254
commit r15-8859-g6b0faf2946a390d5e1493391f5f1eed71b469254 Author: Owen Avery <powerboat9.ga...@gmail.com> Date: Wed Feb 26 09:56:42 2025 -0500 gccrs: Prevent multiple resolution insertion gcc/rust/ChangeLog: * expand/rust-derive-clone.cc (DeriveClone::clone_impl): Avoid using the same node id multiple times. (DeriveClone::clone_enum_identifier): Likewise. (DeriveClone::clone_enum_tuple): Likewise. * expand/rust-derive-copy.cc (DeriveCopy::copy_impl): Likewise. * resolve/rust-ast-resolve-item.cc (flatten_list): Likewise. * resolve/rust-ast-resolve-path.cc (ResolvePath::resolve_path): Prevent reinsertion of resolutions. * resolve/rust-ast-resolve-type.cc (ResolveRelativeTypePath::go): Likewise. * typecheck/rust-hir-type-check-expr.cc (TypeCheckExpr::resolve_fn_trait_call): Likewise. * resolve/rust-name-resolver.cc (Resolver::insert_resolved_name): Catch multiple resolution insertions. (Resolver::insert_resolved_type): Likewise. gcc/testsuite/ChangeLog: * rust/compile/nr2/exclude: Remove entries. Signed-off-by: Owen Avery <powerboat9.ga...@gmail.com> Diff: --- gcc/rust/expand/rust-derive-clone.cc | 43 ++++-- gcc/rust/expand/rust-derive-copy.cc | 13 +- gcc/rust/resolve/rust-ast-resolve-item.cc | 13 +- gcc/rust/resolve/rust-ast-resolve-path.cc | 200 ++++++++++++++++++++----- gcc/rust/resolve/rust-ast-resolve-type.cc | 82 ++++++++-- gcc/rust/resolve/rust-name-resolver.cc | 5 +- gcc/rust/typecheck/rust-hir-type-check-expr.cc | 21 ++- gcc/testsuite/rust/compile/nr2/exclude | 6 - 8 files changed, 302 insertions(+), 81 deletions(-) diff --git a/gcc/rust/expand/rust-derive-clone.cc b/gcc/rust/expand/rust-derive-clone.cc index d0ffbbdafa74..074ea01b24ad 100644 --- a/gcc/rust/expand/rust-derive-clone.cc +++ b/gcc/rust/expand/rust-derive-clone.cc @@ -91,14 +91,17 @@ DeriveClone::clone_impl ( std::unique_ptr<AssociatedItem> &&clone_fn, std::string name, const std::vector<std::unique_ptr<GenericParam>> &type_generics) { - auto clone = builder.type_path (LangItem::Kind::CLONE); + // we should have two of these, so we don't run into issues with + // two paths sharing a node id + auto clone_bound = builder.type_path (LangItem::Kind::CLONE); + auto clone_trait_path = builder.type_path (LangItem::Kind::CLONE); auto trait_items = vec (std::move (clone_fn)); - auto generics - = setup_impl_generics (name, type_generics, builder.trait_bound (clone)); + auto generics = setup_impl_generics (name, type_generics, + builder.trait_bound (clone_bound)); - return builder.trait_impl (clone, std::move (generics.self_type), + return builder.trait_impl (clone_trait_path, std::move (generics.self_type), std::move (trait_items), std::move (generics.impl)); } @@ -173,9 +176,14 @@ DeriveClone::clone_enum_identifier (PathInExpression variant_path, const std::unique_ptr<EnumItem> &variant) { auto pattern = std::unique_ptr<Pattern> (new ReferencePattern ( - std::unique_ptr<Pattern> (new PathInExpression (variant_path)), false, - false, loc)); - auto expr = std::unique_ptr<Expr> (new PathInExpression (variant_path)); + std::unique_ptr<Pattern> (new PathInExpression ( + variant_path.get_segments (), {}, variant_path.get_locus (), + variant_path.opening_scope_resolution ())), + false, false, loc)); + auto expr = std::unique_ptr<Expr> ( + new PathInExpression (variant_path.get_segments (), {}, + variant_path.get_locus (), + variant_path.opening_scope_resolution ())); return builder.match_case (std::move (pattern), std::move (expr)); } @@ -206,14 +214,19 @@ DeriveClone::clone_enum_tuple (PathInExpression variant_path, auto pattern_items = std::unique_ptr<TupleStructItems> ( new TupleStructItemsNoRange (std::move (patterns))); - auto pattern = std::unique_ptr<Pattern> ( - new ReferencePattern (std::unique_ptr<Pattern> (new TupleStructPattern ( - variant_path, std::move (pattern_items))), - false, false, loc)); - - auto expr - = builder.call (std::unique_ptr<Expr> (new PathInExpression (variant_path)), - std::move (cloned_patterns)); + auto pattern = std::unique_ptr<Pattern> (new ReferencePattern ( + std::unique_ptr<Pattern> (new TupleStructPattern ( + PathInExpression (variant_path.get_segments (), {}, + variant_path.get_locus (), + variant_path.opening_scope_resolution ()), + std::move (pattern_items))), + false, false, loc)); + + auto expr = builder.call (std::unique_ptr<Expr> (new PathInExpression ( + variant_path.get_segments (), {}, + variant_path.get_locus (), + variant_path.opening_scope_resolution ())), + std::move (cloned_patterns)); return builder.match_case (std::move (pattern), std::move (expr)); } diff --git a/gcc/rust/expand/rust-derive-copy.cc b/gcc/rust/expand/rust-derive-copy.cc index e91a3508bdee..b2971ad70918 100644 --- a/gcc/rust/expand/rust-derive-copy.cc +++ b/gcc/rust/expand/rust-derive-copy.cc @@ -42,13 +42,16 @@ DeriveCopy::copy_impl ( std::string name, const std::vector<std::unique_ptr<GenericParam>> &type_generics) { - auto copy = builder.type_path (LangItem::Kind::COPY); + // we should have two of these, so we don't run into issues with + // two paths sharing a node id + auto copy_bound = builder.type_path (LangItem::Kind::COPY); + auto copy_trait_path = builder.type_path (LangItem::Kind::COPY); - auto generics - = setup_impl_generics (name, type_generics, builder.trait_bound (copy)); + auto generics = setup_impl_generics (name, type_generics, + builder.trait_bound (copy_bound)); - return builder.trait_impl (copy, std::move (generics.self_type), {}, - std::move (generics.impl)); + return builder.trait_impl (copy_trait_path, std::move (generics.self_type), + {}, std::move (generics.impl)); } void diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc b/gcc/rust/resolve/rust-ast-resolve-item.cc index 366716a22e2e..d584961354fd 100644 --- a/gcc/rust/resolve/rust-ast-resolve-item.cc +++ b/gcc/rust/resolve/rust-ast-resolve-item.cc @@ -903,7 +903,18 @@ flatten_list (const AST::UseTreeList &list, std::vector<Import> &imports) for (auto import = imports.begin () + start_idx; import != imports.end (); import++) - import->add_prefix (prefix); + { + // avoid duplicate node ids + auto prefix_copy + = AST::SimplePath ({}, prefix.has_opening_scope_resolution (), + prefix.get_locus ()); + for (auto &seg : prefix.get_segments ()) + prefix_copy.get_segments ().push_back ( + AST::SimplePathSegment (seg.get_segment_name (), + seg.get_locus ())); + + import->add_prefix (std::move (prefix_copy)); + } } } diff --git a/gcc/rust/resolve/rust-ast-resolve-path.cc b/gcc/rust/resolve/rust-ast-resolve-path.cc index 656b7e64bb73..b2b10719e190 100644 --- a/gcc/rust/resolve/rust-ast-resolve-path.cc +++ b/gcc/rust/resolve/rust-ast-resolve-path.cc @@ -80,8 +80,16 @@ ResolvePath::resolve_path (AST::PathInExpression &expr) // what is the current crate scope node id? module_scope_id = crate_scope_id; previous_resolved_node_id = module_scope_id; - resolver->insert_resolved_name (segment.get_node_id (), - module_scope_id); + + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_name (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == module_scope_id); + else + resolver->insert_resolved_name (segment.get_node_id (), + module_scope_id); continue; } else if (segment.is_super_path_seg ()) @@ -95,8 +103,16 @@ ResolvePath::resolve_path (AST::PathInExpression &expr) module_scope_id = resolver->peek_parent_module_scope (); previous_resolved_node_id = module_scope_id; - resolver->insert_resolved_name (segment.get_node_id (), - module_scope_id); + + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_name (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == module_scope_id); + else + resolver->insert_resolved_name (segment.get_node_id (), + module_scope_id); continue; } @@ -140,23 +156,45 @@ ResolvePath::resolve_path (AST::PathInExpression &expr) ident_seg.as_string ()); if (resolver->get_name_scope ().lookup (path, &resolved_node)) { - resolver->insert_resolved_name (segment.get_node_id (), - resolved_node); + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_name (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_name (segment.get_node_id (), + resolved_node); resolved_node_id = resolved_node; } // check the type scope else if (resolver->get_type_scope ().lookup (path, &resolved_node)) { - resolver->insert_resolved_type (segment.get_node_id (), - resolved_node); + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_type (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_type (segment.get_node_id (), + resolved_node); resolved_node_id = resolved_node; } else if (segment.is_lower_self_seg ()) { module_scope_id = crate_scope_id; previous_resolved_node_id = module_scope_id; - resolver->insert_resolved_name (segment.get_node_id (), - module_scope_id); + + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_name (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == module_scope_id); + else + resolver->insert_resolved_name (segment.get_node_id (), + module_scope_id); continue; } else @@ -179,15 +217,33 @@ ResolvePath::resolve_path (AST::PathInExpression &expr) resolved_node)) { resolved_node_id = resolved_node; - resolver->insert_resolved_name (segment.get_node_id (), - resolved_node); + + NodeId existing = UNKNOWN_NODEID; + bool ok + = resolver->lookup_resolved_name (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_name (segment.get_node_id (), + resolved_node); } else if (resolver->get_type_scope ().decl_was_declared_here ( resolved_node)) { resolved_node_id = resolved_node; - resolver->insert_resolved_type (segment.get_node_id (), - resolved_node); + + NodeId existing = UNKNOWN_NODEID; + bool ok + = resolver->lookup_resolved_type (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_type (segment.get_node_id (), + resolved_node); } else { @@ -224,15 +280,29 @@ ResolvePath::resolve_path (AST::PathInExpression &expr) // name scope first if (resolver->get_name_scope ().decl_was_declared_here (resolved_node_id)) { - resolver->insert_resolved_name (expr.get_node_id (), - resolved_node_id); + NodeId existing = UNKNOWN_NODEID; + bool ok + = resolver->lookup_resolved_name (expr.get_node_id (), &existing); + + if (ok) + rust_assert (existing == resolved_node_id); + else + resolver->insert_resolved_name (expr.get_node_id (), + resolved_node_id); } // check the type scope else if (resolver->get_type_scope ().decl_was_declared_here ( resolved_node_id)) { - resolver->insert_resolved_type (expr.get_node_id (), - resolved_node_id); + NodeId existing = UNKNOWN_NODEID; + bool ok + = resolver->lookup_resolved_type (expr.get_node_id (), &existing); + + if (ok) + rust_assert (existing == resolved_node_id); + else + resolver->insert_resolved_type (expr.get_node_id (), + resolved_node_id); } else { @@ -284,8 +354,16 @@ ResolvePath::resolve_path (AST::SimplePath &expr) // what is the current crate scope node id? module_scope_id = crate_scope_id; previous_resolved_node_id = module_scope_id; - resolver->insert_resolved_name (segment.get_node_id (), - module_scope_id); + + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_name (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == module_scope_id); + else + resolver->insert_resolved_name (segment.get_node_id (), + module_scope_id); resolved_node_id = module_scope_id; continue; @@ -301,8 +379,16 @@ ResolvePath::resolve_path (AST::SimplePath &expr) module_scope_id = resolver->peek_parent_module_scope (); previous_resolved_node_id = module_scope_id; - resolver->insert_resolved_name (segment.get_node_id (), - module_scope_id); + + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_name (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == module_scope_id); + else + resolver->insert_resolved_name (segment.get_node_id (), + module_scope_id); resolved_node_id = module_scope_id; continue; @@ -318,15 +404,31 @@ ResolvePath::resolve_path (AST::SimplePath &expr) resolved_node)) { resolved_node_id = resolved_node; - resolver->insert_resolved_name (segment.get_node_id (), - resolved_node); + + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_name (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_name (segment.get_node_id (), + resolved_node); } else if (resolver->get_type_scope ().decl_was_declared_here ( resolved_node)) { resolved_node_id = resolved_node; - resolver->insert_resolved_type (segment.get_node_id (), - resolved_node); + + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_type (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_type (segment.get_node_id (), + resolved_node); } else { @@ -347,15 +449,31 @@ ResolvePath::resolve_path (AST::SimplePath &expr) if (resolver->get_name_scope ().lookup (path, &resolved_node)) { resolved_node_id = resolved_node; - resolver->insert_resolved_name (segment.get_node_id (), - resolved_node); + + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_name (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_name (segment.get_node_id (), + resolved_node); } // check the type scope else if (resolver->get_type_scope ().lookup (path, &resolved_node)) { resolved_node_id = resolved_node; - resolver->insert_resolved_type (segment.get_node_id (), - resolved_node); + + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_type (segment.get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_type (segment.get_node_id (), + resolved_node); } } @@ -398,15 +516,29 @@ ResolvePath::resolve_path (AST::SimplePath &expr) // name scope first if (resolver->get_name_scope ().decl_was_declared_here (resolved_node_id)) { - resolver->insert_resolved_name (expr.get_node_id (), - resolved_node_id); + NodeId existing = UNKNOWN_NODEID; + bool ok + = resolver->lookup_resolved_name (expr.get_node_id (), &existing); + + if (ok) + rust_assert (existing == resolved_node_id); + else + resolver->insert_resolved_name (expr.get_node_id (), + resolved_node_id); } // check the type scope else if (resolver->get_type_scope ().decl_was_declared_here ( resolved_node_id)) { - resolver->insert_resolved_type (expr.get_node_id (), - resolved_node_id); + NodeId existing = UNKNOWN_NODEID; + bool ok + = resolver->lookup_resolved_type (expr.get_node_id (), &existing); + + if (ok) + rust_assert (existing == resolved_node_id); + else + resolver->insert_resolved_type (expr.get_node_id (), + resolved_node_id); } else { diff --git a/gcc/rust/resolve/rust-ast-resolve-type.cc b/gcc/rust/resolve/rust-ast-resolve-type.cc index 87643d775ff4..5ab0c445ae70 100644 --- a/gcc/rust/resolve/rust-ast-resolve-type.cc +++ b/gcc/rust/resolve/rust-ast-resolve-type.cc @@ -249,14 +249,28 @@ ResolveRelativeTypePath::go (AST::TypePath &path, NodeId &resolved_node_id) = CanonicalPath::new_seg (segment->get_node_id (), ident_string); if (resolver->get_type_scope ().lookup (path, &resolved_node)) { - resolver->insert_resolved_type (segment->get_node_id (), - resolved_node); + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_type (segment->get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_type (segment->get_node_id (), + resolved_node); resolved_node_id = resolved_node; } else if (resolver->get_name_scope ().lookup (path, &resolved_node)) { - resolver->insert_resolved_name (segment->get_node_id (), - resolved_node); + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_name (segment->get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_name (segment->get_node_id (), + resolved_node); resolved_node_id = resolved_node; } else if (!segment->is_lang_item () && segment->is_lower_self_seg ()) @@ -264,8 +278,16 @@ ResolveRelativeTypePath::go (AST::TypePath &path, NodeId &resolved_node_id) // what is the current crate scope node id? module_scope_id = crate_scope_id; previous_resolved_node_id = module_scope_id; - resolver->insert_resolved_name (segment->get_node_id (), - module_scope_id); + + NodeId existing = UNKNOWN_NODEID; + bool ok = resolver->lookup_resolved_name (segment->get_node_id (), + &existing); + + if (ok) + rust_assert (existing == module_scope_id); + else + resolver->insert_resolved_name (segment->get_node_id (), + module_scope_id); continue; } @@ -283,15 +305,33 @@ ResolveRelativeTypePath::go (AST::TypePath &path, NodeId &resolved_node_id) resolved_node)) { resolved_node_id = resolved_node; - resolver->insert_resolved_name (segment->get_node_id (), - resolved_node); + + NodeId existing = UNKNOWN_NODEID; + bool ok + = resolver->lookup_resolved_name (segment->get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_name (segment->get_node_id (), + resolved_node); } else if (resolver->get_type_scope ().decl_was_declared_here ( resolved_node)) { resolved_node_id = resolved_node; - resolver->insert_resolved_type (segment->get_node_id (), - resolved_node); + + NodeId existing = UNKNOWN_NODEID; + bool ok + = resolver->lookup_resolved_type (segment->get_node_id (), + &existing); + + if (ok) + rust_assert (existing == resolved_node); + else + resolver->insert_resolved_type (segment->get_node_id (), + resolved_node); } else { @@ -327,15 +367,29 @@ ResolveRelativeTypePath::go (AST::TypePath &path, NodeId &resolved_node_id) // name scope first if (resolver->get_name_scope ().decl_was_declared_here (resolved_node_id)) { - resolver->insert_resolved_name (path.get_node_id (), - resolved_node_id); + NodeId existing = UNKNOWN_NODEID; + bool ok + = resolver->lookup_resolved_name (path.get_node_id (), &existing); + + if (ok) + rust_assert (existing == resolved_node_id); + else + resolver->insert_resolved_name (path.get_node_id (), + resolved_node_id); } // check the type scope else if (resolver->get_type_scope ().decl_was_declared_here ( resolved_node_id)) { - resolver->insert_resolved_type (path.get_node_id (), - resolved_node_id); + NodeId existing = UNKNOWN_NODEID; + bool ok + = resolver->lookup_resolved_type (path.get_node_id (), &existing); + + if (ok) + rust_assert (existing == resolved_node_id); + else + resolver->insert_resolved_type (path.get_node_id (), + resolved_node_id); } else { diff --git a/gcc/rust/resolve/rust-name-resolver.cc b/gcc/rust/resolve/rust-name-resolver.cc index 97ee2d37dedb..dddaa0793010 100644 --- a/gcc/rust/resolve/rust-name-resolver.cc +++ b/gcc/rust/resolve/rust-name-resolver.cc @@ -472,6 +472,8 @@ void Resolver::insert_resolved_name (NodeId refId, NodeId defId) { rust_assert (!flag_name_resolution_2_0); + rust_assert (resolved_names.find (refId) == resolved_names.end ()); + resolved_names[refId] = defId; get_name_scope ().append_reference_for_def (refId, defId); insert_captured_item (defId); @@ -492,9 +494,8 @@ Resolver::lookup_resolved_name (NodeId refId, NodeId *defId) void Resolver::insert_resolved_type (NodeId refId, NodeId defId) { - // auto it = resolved_types.find (refId); - // rust_assert (it == resolved_types.end ()); rust_assert (!flag_name_resolution_2_0); + rust_assert (resolved_types.find (refId) == resolved_types.end ()); resolved_types[refId] = defId; get_type_scope ().append_reference_for_def (refId, defId); diff --git a/gcc/rust/typecheck/rust-hir-type-check-expr.cc b/gcc/rust/typecheck/rust-hir-type-check-expr.cc index 4e57d6a8ffbc..b7fdc1396151 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-expr.cc +++ b/gcc/rust/typecheck/rust-hir-type-check-expr.cc @@ -2124,13 +2124,26 @@ TypeCheckExpr::resolve_fn_trait_call (HIR::CallExpr &expr, auto &nr_ctx = const_cast<Resolver2_0::NameResolutionContext &> ( Resolver2_0::ImmutableNameResolutionContext::get ().resolver ()); - nr_ctx.map_usage (Resolver2_0::Usage (expr.get_mappings ().get_nodeid ()), - Resolver2_0::Definition (resolved_node_id)); + auto existing = nr_ctx.lookup (expr.get_mappings ().get_nodeid ()); + if (existing) + rust_assert (*existing == resolved_node_id); + else + nr_ctx.map_usage (Resolver2_0::Usage ( + expr.get_mappings ().get_nodeid ()), + Resolver2_0::Definition (resolved_node_id)); } else { - resolver->insert_resolved_name (expr.get_mappings ().get_nodeid (), - resolved_node_id); + NodeId existing = UNKNOWN_NODEID; + bool ok + = resolver->lookup_resolved_name (expr.get_mappings ().get_nodeid (), + &existing); + + if (ok) + rust_assert (existing == resolved_node_id); + else + resolver->insert_resolved_name (expr.get_mappings ().get_nodeid (), + resolved_node_id); } // return the result of the function back diff --git a/gcc/testsuite/rust/compile/nr2/exclude b/gcc/testsuite/rust/compile/nr2/exclude index a2a833f90738..0252028fb8f7 100644 --- a/gcc/testsuite/rust/compile/nr2/exclude +++ b/gcc/testsuite/rust/compile/nr2/exclude @@ -5,7 +5,6 @@ const_generics_3.rs const_generics_4.rs feature_rust_attri0.rs generics9.rs -issue-1483.rs issue-1901.rs issue-1981.rs issue-2043.rs @@ -34,14 +33,9 @@ undeclared_label.rs use_1.rs while_break_expr.rs issue-3139-2.rs -issue-2953-2.rs issue-2905-2.rs issue-266.rs -derive_clone_enum1.rs -derive_clone_enum2.rs derive_clone_enum3.rs -issue-3139-1.rs -issue-3139-3.rs derive-debug1.rs derive-default1.rs issue-3402-1.rs