From 4c4a569d37c1d2056b354364679ee79cfac2f4df Mon Sep 17 00:00:00 2001 From: Pierre-Emmanuel Patry Date: Tue, 10 Dec 2024 12:50:43 +0100 Subject: [PATCH 1/7] Prevent hir dump from visiting missing types The hir dump was trying to visit init expression and types even when those were missing. gcc/rust/ChangeLog: * hir/rust-hir-dump.cc (Dump::visit): Add check for type and initial expression. Signed-off-by: Pierre-Emmanuel Patry --- gcc/rust/hir/rust-hir-dump.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gcc/rust/hir/rust-hir-dump.cc b/gcc/rust/hir/rust-hir-dump.cc index 914dbf0e76ca..e4d4f2c38745 100644 --- a/gcc/rust/hir/rust-hir-dump.cc +++ b/gcc/rust/hir/rust-hir-dump.cc @@ -2254,8 +2254,10 @@ Dump::visit (LetStmt &e) put_field ("variable_pattern", e.get_pattern ().as_string ()); - visit_field ("type", e.get_type ()); - visit_field ("init_expr", e.get_init_expr ()); + if (e.has_type ()) + visit_field ("type", e.get_type ()); + if (e.has_init_expr ()) + visit_field ("init_expr", e.get_init_expr ()); end ("LetStmt"); } From 9d29160b69a6c99b03110d259b2811e2cb202a8b Mon Sep 17 00:00:00 2001 From: Pierre-Emmanuel Patry Date: Wed, 11 Dec 2024 12:04:32 +0100 Subject: [PATCH 2/7] Labels shall be pushed withing label namespace Labels were using the wrong namespace. gcc/rust/ChangeLog: * resolve/rust-ast-resolve-expr.cc (ResolveExpr::visit): Change label push function from type rib to label rib. * resolve/rust-ast-resolve-item.cc (ResolveTraitItems::visit): Likewise. (ResolveItem::visit): Likewise. (ResolveExternItem::visit): Likewise. * resolve/rust-ast-resolve-stmt.h: Likewise. * resolve/rust-ast-resolve.cc (NameResolution::go): Likewise. Signed-off-by: Pierre-Emmanuel Patry --- gcc/rust/resolve/rust-ast-resolve-expr.cc | 14 +++++++------- gcc/rust/resolve/rust-ast-resolve-item.cc | 10 +++++----- gcc/rust/resolve/rust-ast-resolve-stmt.h | 2 +- gcc/rust/resolve/rust-ast-resolve.cc | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/gcc/rust/resolve/rust-ast-resolve-expr.cc b/gcc/rust/resolve/rust-ast-resolve-expr.cc index 9c2be4a57160..f2b8d12f3b4c 100644 --- a/gcc/rust/resolve/rust-ast-resolve-expr.cc +++ b/gcc/rust/resolve/rust-ast-resolve-expr.cc @@ -248,7 +248,7 @@ ResolveExpr::visit (AST::IfLetExpr &expr) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); // We know expr.get_patterns () has one pattern at most // so there's no reason to handle it like an AltPattern. @@ -278,7 +278,7 @@ ResolveExpr::visit (AST::IfLetExprConseqElse &expr) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); // We know expr.get_patterns () has one pattern at most // so there's no reason to handle it like an AltPattern. @@ -307,7 +307,7 @@ ResolveExpr::visit (AST::BlockExpr &expr) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); if (expr.has_label ()) { @@ -615,7 +615,7 @@ ResolveExpr::visit (AST::ForLoopExpr &expr) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); // resolve the expression PatternDeclaration::go (expr.get_pattern (), Rib::ItemType::Var); @@ -681,7 +681,7 @@ ResolveExpr::visit (AST::MatchExpr &expr) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); // resolve AST::MatchArm &arm = match_case.get_arm (); @@ -750,7 +750,7 @@ ResolveExpr::visit (AST::ClosureExprInner &expr) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); std::vector bindings = {PatternBinding (PatternBoundCtx::Product, std::set ())}; @@ -780,7 +780,7 @@ ResolveExpr::visit (AST::ClosureExprInnerTyped &expr) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); std::vector bindings = {PatternBinding (PatternBoundCtx::Product, std::set ())}; diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc b/gcc/rust/resolve/rust-ast-resolve-item.cc index 97faeabc9a85..aec56ecd47f0 100644 --- a/gcc/rust/resolve/rust-ast-resolve-item.cc +++ b/gcc/rust/resolve/rust-ast-resolve-item.cc @@ -61,7 +61,7 @@ ResolveTraitItems::visit (AST::Function &function) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); if (function.has_generics ()) for (auto &generic : function.get_generic_params ()) @@ -216,7 +216,7 @@ ResolveItem::visit (AST::Module &module) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); // FIXME: Should we reinsert a child here? Any reason we ResolveTopLevel::go // in ResolveTopLevel::visit (AST::Module) as well as here? @@ -473,7 +473,7 @@ ResolveItem::visit (AST::Function &function) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); if (function.has_generics ()) for (auto &generic : function.get_generic_params ()) @@ -641,7 +641,7 @@ ResolveItem::visit (AST::TraitImpl &impl_block) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); if (impl_block.has_generics ()) for (auto &generic : impl_block.get_generic_params ()) @@ -1019,7 +1019,7 @@ ResolveExternItem::visit (AST::Function &function) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); // resolve the generics if (function.has_generics ()) diff --git a/gcc/rust/resolve/rust-ast-resolve-stmt.h b/gcc/rust/resolve/rust-ast-resolve-stmt.h index 7118b70d7058..45f3283bd4db 100644 --- a/gcc/rust/resolve/rust-ast-resolve-stmt.h +++ b/gcc/rust/resolve/rust-ast-resolve-stmt.h @@ -338,7 +338,7 @@ class ResolveStmt : public ResolverBase resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); if (function.has_generics ()) for (auto &generic : function.get_generic_params ()) diff --git a/gcc/rust/resolve/rust-ast-resolve.cc b/gcc/rust/resolve/rust-ast-resolve.cc index df90cff7815c..75e01ab2b0ce 100644 --- a/gcc/rust/resolve/rust-ast-resolve.cc +++ b/gcc/rust/resolve/rust-ast-resolve.cc @@ -75,7 +75,7 @@ NameResolution::go (AST::Crate &crate) resolver->get_label_scope ().push (scope_node_id); resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - resolver->push_new_label_rib (resolver->get_type_scope ().peek ()); + resolver->push_new_label_rib (resolver->get_label_scope ().peek ()); // get the root segment CanonicalPath crate_prefix From 6e9f06c746bfa830baf5c9fb050c339f80ba92ce Mon Sep 17 00:00:00 2001 From: Pierre-Emmanuel Patry Date: Wed, 11 Dec 2024 14:19:44 +0100 Subject: [PATCH 3/7] Add debug dump to old name resolver It might be necessary to compare both name resolution' internal states during the transition. This new debug representation could help with that. gcc/rust/ChangeLog: * resolve/rust-name-resolver.h: Add new degug dump for old name resolver. Signed-off-by: Pierre-Emmanuel Patry --- gcc/rust/resolve/rust-name-resolver.h | 35 +++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/gcc/rust/resolve/rust-name-resolver.h b/gcc/rust/resolve/rust-name-resolver.h index 5e1901adfd5a..bf5ef7786960 100644 --- a/gcc/rust/resolve/rust-name-resolver.h +++ b/gcc/rust/resolve/rust-name-resolver.h @@ -204,6 +204,41 @@ class Resolver void insert_captured_item (NodeId id); const std::set &get_captures (NodeId id) const; + std::string as_debug_string () const + { + std::stringstream ss; + + ss << "Names:\n"; + for (auto &n : name_ribs) + { + ss << "\tNodeID: " << n.first << " Rib: " << n.second->debug_str () + << "\n"; + } + ss << "Types:\n"; + for (auto &n : type_ribs) + { + ss << "\tNodeID: " << n.first << " Rib: " << n.second->debug_str () + << "\n"; + } + ss << "Macros:\n"; + + for (auto &n : macro_ribs) + { + ss << "\tNodeID: " << n.first << " Rib: " << n.second->debug_str () + << "\n"; + } + + ss << "Labels:\n"; + + for (auto &n : label_ribs) + { + ss << "\tNodeID: " << n.first << " Rib: " << n.second->debug_str () + << "\n"; + } + + return ss.str (); + } + protected: bool decl_needs_capture (NodeId decl_rib_node_id, NodeId closure_rib_node_id, const Scope &scope); From 1ad6863a9b7e46cd9c94cd83abc2f67385d23201 Mon Sep 17 00:00:00 2001 From: Pierre-Emmanuel Patry Date: Wed, 11 Dec 2024 15:12:00 +0100 Subject: [PATCH 4/7] Add unit struct to name namespace in old resolver We missed the name namespace for unit struct in the old resolver. gcc/rust/ChangeLog: * resolve/rust-ast-resolve-toplevel.h: Add struct to name namespace. Signed-off-by: Pierre-Emmanuel Patry --- gcc/rust/resolve/rust-ast-resolve-toplevel.h | 23 +++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/gcc/rust/resolve/rust-ast-resolve-toplevel.h b/gcc/rust/resolve/rust-ast-resolve-toplevel.h index ceac7edd3415..491fa690eb50 100644 --- a/gcc/rust/resolve/rust-ast-resolve-toplevel.h +++ b/gcc/rust/resolve/rust-ast-resolve-toplevel.h @@ -242,14 +242,21 @@ class ResolveTopLevel : public ResolverBase auto path = prefix.append (decl); auto cpath = canonical_prefix.append (decl); - resolver->get_type_scope ().insert ( - path, struct_decl.get_node_id (), struct_decl.get_locus (), false, - Rib::ItemType::Type, - [&] (const CanonicalPath &, NodeId, location_t locus) -> void { - rich_location r (line_table, struct_decl.get_locus ()); - r.add_range (locus); - rust_error_at (r, "redefined multiple times"); - }); + auto duplicate_item + = [&] (const CanonicalPath &, NodeId, location_t locus) -> void { + rich_location r (line_table, struct_decl.get_locus ()); + r.add_range (locus); + rust_error_at (r, "redefined multiple times"); + }; + + resolver->get_type_scope ().insert (path, struct_decl.get_node_id (), + struct_decl.get_locus (), false, + Rib::ItemType::Type, duplicate_item); + + if (struct_decl.is_unit_struct ()) + resolver->get_name_scope ().insert (path, struct_decl.get_node_id (), + struct_decl.get_locus (), false, + Rib::ItemType::Type, duplicate_item); NodeId current_module = resolver->peek_current_module_scope (); mappings.insert_module_child_item (current_module, decl); From 9957c24448ceee4ddb31c57db02a6daae1acdfe5 Mon Sep 17 00:00:00 2001 From: Pierre-Emmanuel Patry Date: Thu, 12 Dec 2024 13:16:14 +0100 Subject: [PATCH 5/7] Remove query mode on CompileItem Query mode was a hack to catch up some compile errors early, it was deemed to be removed at some time. Recent changes to NR1 highlighted an incompatibility with it hence it's removal. gcc/rust/ChangeLog: * backend/rust-compile-item.h: Remove query mode. * backend/rust-compile-resolve-path.cc (HIRCompileBase::query_compile): Likewise. Signed-off-by: Pierre-Emmanuel Patry --- gcc/rust/backend/rust-compile-item.h | 6 ++---- gcc/rust/backend/rust-compile-resolve-path.cc | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/gcc/rust/backend/rust-compile-item.h b/gcc/rust/backend/rust-compile-item.h index 1c877fa328dc..70660e16147c 100644 --- a/gcc/rust/backend/rust-compile-item.h +++ b/gcc/rust/backend/rust-compile-item.h @@ -31,15 +31,13 @@ class CompileItem : private HIRCompileBase, protected HIR::HIRStmtVisitor public: static tree compile (HIR::Item *item, Context *ctx, TyTy::BaseType *concrete = nullptr, - bool is_query_mode = false, location_t ref_locus = UNDEF_LOCATION) { CompileItem compiler (ctx, concrete, ref_locus); item->accept_vis (compiler); - if (is_query_mode && compiler.reference == error_mark_node) - rust_internal_error_at (ref_locus, "failed to compile item: %s", - item->as_string ().c_str ()); + if (compiler.reference == error_mark_node) + rust_debug ("failed to compile item: %s", item->as_string ().c_str ()); return compiler.reference; } diff --git a/gcc/rust/backend/rust-compile-resolve-path.cc b/gcc/rust/backend/rust-compile-resolve-path.cc index 5f6ba7cce433..170dde6055de 100644 --- a/gcc/rust/backend/rust-compile-resolve-path.cc +++ b/gcc/rust/backend/rust-compile-resolve-path.cc @@ -205,11 +205,9 @@ HIRCompileBase::query_compile (HirId ref, TyTy::BaseType *lookup, if (auto resolved_item = ctx->get_mappings ().lookup_hir_item (ref)) { if (!lookup->has_substitutions_defined ()) - return CompileItem::compile (*resolved_item, ctx, nullptr, true, - expr_locus); + return CompileItem::compile (*resolved_item, ctx, nullptr, expr_locus); else - return CompileItem::compile (*resolved_item, ctx, lookup, true, - expr_locus); + return CompileItem::compile (*resolved_item, ctx, lookup, expr_locus); } else if (auto hir_extern_item = ctx->get_mappings ().lookup_hir_extern_item (ref)) From 610e11240761710c66442508617dac3ed7b3a16c Mon Sep 17 00:00:00 2001 From: Pierre-Emmanuel Patry Date: Thu, 12 Dec 2024 14:39:12 +0100 Subject: [PATCH 6/7] Remove some tests from nr2 exclusion file Those test are now passing. gcc/testsuite/ChangeLog: * rust/compile/nr2/exclude: Remove passing tests. Signed-off-by: Pierre-Emmanuel Patry --- gcc/testsuite/rust/compile/nr2/exclude | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/gcc/testsuite/rust/compile/nr2/exclude b/gcc/testsuite/rust/compile/nr2/exclude index 6c589e4ab4e5..85e2adc135c8 100644 --- a/gcc/testsuite/rust/compile/nr2/exclude +++ b/gcc/testsuite/rust/compile/nr2/exclude @@ -12,18 +12,13 @@ macros/builtin/include3.rs macros/builtin/include4.rs canonical_paths1.rs cfg1.rs -cfg3.rs -cfg4.rs -cfg5.rs closure_no_type_anno.rs complex-path1.rs complex_qualified_path_in_expr.rs const-issue1440.rs const_generics_3.rs const_generics_4.rs -const_generics_5.rs const_generics_7.rs -derive_empty.rs derive_macro1.rs derive_macro3.rs derive_macro4.rs @@ -74,7 +69,6 @@ issue-2070.rs issue-2105.rs issue-2135.rs issue-2136-1.rs -issue-2136-2.rs issue-2139.rs issue-2142.rs issue-2165.rs @@ -102,8 +96,6 @@ iterators1.rs lookup_err1.rs macros/mbe/macro-issue1233.rs macros/mbe/macro-issue1400.rs -macros/mbe/macro13.rs -macros/mbe/macro15.rs macros/mbe/macro20.rs macros/mbe/macro23.rs macros/mbe/macro40.rs @@ -112,7 +104,6 @@ macros/mbe/macro44.rs macros/mbe/macro50.rs macros/mbe/macro54.rs macros/mbe/macro6.rs -macros/mbe/macro_rules_macro_rules.rs macros/mbe/macro_use1.rs match-never-ltype.rs match-never-rtype.rs @@ -153,7 +144,6 @@ redef_error2.rs redef_error4.rs redef_error5.rs redef_error6.rs -self-path1.rs self-path2.rs sizeof-stray-infer-var-bug.rs specify-crate-name.rs From e5c1373090ec2e00b1da025a5c1240aa2e26f92d Mon Sep 17 00:00:00 2001 From: Pierre-Emmanuel Patry Date: Thu, 12 Dec 2024 18:30:09 +0100 Subject: [PATCH 7/7] Clone expr instead of taking it We're reusing the value, it could therefore not be taken be should be cloned. gcc/rust/ChangeLog: * typecheck/rust-hir-type-check-enumitem.cc (TypeCheckEnumItem::visit): Clone expr instead of taking it. Signed-off-by: Pierre-Emmanuel Patry --- gcc/rust/typecheck/rust-hir-type-check-enumitem.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gcc/rust/typecheck/rust-hir-type-check-enumitem.cc b/gcc/rust/typecheck/rust-hir-type-check-enumitem.cc index 5e6087d78b22..0988da635165 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-enumitem.cc +++ b/gcc/rust/typecheck/rust-hir-type-check-enumitem.cc @@ -142,10 +142,11 @@ TypeCheckEnumItem::visit (HIR::EnumItemDiscriminant &item) rust_assert (canonical_path.has_value ()); RustIdent ident{*canonical_path, item.get_locus ()}; - variant = new TyTy::VariantDef (item.get_mappings ().get_hirid (), - item.get_mappings ().get_defid (), - item.get_identifier ().as_string (), ident, - item.take_discriminant_expression ()); + variant + = new TyTy::VariantDef (item.get_mappings ().get_hirid (), + item.get_mappings ().get_defid (), + item.get_identifier ().as_string (), ident, + item.get_discriminant_expression ().clone_expr ()); } void