From 335f6c2b0918c27eecd93a114668841b74ee06c3 Mon Sep 17 00:00:00 2001 From: Owen Avery Date: Mon, 4 Nov 2024 15:05:03 -0500 Subject: [PATCH] Improve handling of implicit Self parameter in AST gcc/rust/ChangeLog: * ast/rust-item.h (Trait::self_param): Add. (Trait::Trait): Initialize self_param. (Trait::operator=): Copy self_param. (Trait::insert_implicit_self): Remove. (Trait::get_implicit_self): Add. * hir/rust-ast-lower-item.cc (ASTLoweringItem::visit): Make sure implicit self is still lowered to HIR. * resolve/rust-ast-resolve-item.cc (ResolveItem::visit): Adjust handling of implicit self. * resolve/rust-early-name-resolver.cc (EarlyNameResolver::visit): Add commit to Trait visitor mentioning that implicit self is not visited. * resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::visit): Remove call to Trait::insert_implicit_self. gcc/testsuite/ChangeLog: * rust/compile/nr2/exclude: Remove entries. * rust/link/generic_function_0.rs: No longer expect failure. * rust/link/trait_import_0.rs: Likewise. * rust/link/trait_import_1.rs (trait Sized): Add. Signed-off-by: Owen Avery --- gcc/rust/ast/rust-item.h | 23 ++++++------------- gcc/rust/hir/rust-ast-lower-item.cc | 6 +++++ gcc/rust/resolve/rust-ast-resolve-item.cc | 12 +++------- gcc/rust/resolve/rust-early-name-resolver.cc | 2 ++ .../rust-toplevel-name-resolver-2.0.cc | 17 -------------- gcc/testsuite/rust/compile/nr2/exclude | 9 -------- gcc/testsuite/rust/link/generic_function_0.rs | 3 --- gcc/testsuite/rust/link/trait_import_0.rs | 3 --- gcc/testsuite/rust/link/trait_import_1.rs | 3 +++ 9 files changed, 21 insertions(+), 57 deletions(-) diff --git a/gcc/rust/ast/rust-item.h b/gcc/rust/ast/rust-item.h index f6b29130cb8a..3969398f27ef 100644 --- a/gcc/rust/ast/rust-item.h +++ b/gcc/rust/ast/rust-item.h @@ -2831,6 +2831,7 @@ class Trait : public VisItem bool has_auto; Identifier name; std::vector> generic_params; + TypeParam self_param; std::vector> type_param_bounds; WhereClause where_clause; std::vector inner_attrs; @@ -2870,7 +2871,7 @@ class Trait : public VisItem std::vector inner_attrs, location_t locus) : VisItem (std::move (vis), std::move (outer_attrs)), has_unsafe (is_unsafe), has_auto (is_auto), name (std::move (name)), - generic_params (std::move (generic_params)), + generic_params (std::move (generic_params)), self_param ({"Self"}, locus), type_param_bounds (std::move (type_param_bounds)), where_clause (std::move (where_clause)), inner_attrs (std::move (inner_attrs)), @@ -2880,8 +2881,9 @@ class Trait : public VisItem // Copy constructor with vector clone Trait (Trait const &other) : VisItem (other), has_unsafe (other.has_unsafe), has_auto (other.has_auto), - name (other.name), where_clause (other.where_clause), - inner_attrs (other.inner_attrs), locus (other.locus) + name (other.name), self_param (other.self_param), + where_clause (other.where_clause), inner_attrs (other.inner_attrs), + locus (other.locus) { generic_params.reserve (other.generic_params.size ()); for (const auto &e : other.generic_params) @@ -2901,6 +2903,7 @@ class Trait : public VisItem { VisItem::operator= (other); name = other.name; + self_param = other.self_param; has_unsafe = other.has_unsafe; has_auto = other.has_auto; where_clause = other.where_clause; @@ -2968,19 +2971,7 @@ class Trait : public VisItem WhereClause &get_where_clause () { return where_clause; } - void insert_implict_self (std::unique_ptr &¶m) - { - std::vector> new_list; - new_list.reserve (generic_params.size () + 1); - - new_list.push_back (std::move (param)); - for (auto &p : generic_params) - { - new_list.push_back (std::move (p)); - } - - generic_params = std::move (new_list); - } + AST::TypeParam &get_implicit_self () { return self_param; } protected: /* Use covariance to implement clone function as returning this object diff --git a/gcc/rust/hir/rust-ast-lower-item.cc b/gcc/rust/hir/rust-ast-lower-item.cc index cb07b26d5d64..2457e919aa47 100644 --- a/gcc/rust/hir/rust-ast-lower-item.cc +++ b/gcc/rust/hir/rust-ast-lower-item.cc @@ -572,6 +572,12 @@ ASTLoweringItem::visit (AST::Trait &trait) generic_params = lower_generic_params (trait.get_generic_params ()); } + // TODO: separate "Self" from normal generic parameters + // in HIR as well as in AST? + HIR::GenericParam *self_param + = ASTLowerGenericParam::translate (trait.get_implicit_self ()); + generic_params.emplace (generic_params.begin (), self_param); + std::vector> type_param_bounds; if (trait.has_type_param_bounds ()) { diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc b/gcc/rust/resolve/rust-ast-resolve-item.cc index bf47c73495c8..97faeabc9a85 100644 --- a/gcc/rust/resolve/rust-ast-resolve-item.cc +++ b/gcc/rust/resolve/rust-ast-resolve-item.cc @@ -755,20 +755,14 @@ ResolveItem::visit (AST::Trait &trait) resolver->push_new_name_rib (resolver->get_name_scope ().peek ()); resolver->push_new_type_rib (resolver->get_type_scope ().peek ()); - // we need to inject an implicit self TypeParam here - // FIXME: which location should be used for Rust::Identifier `Self`? - AST::TypeParam *implicit_self - = new AST::TypeParam ({"Self"}, trait.get_locus ()); - trait.insert_implict_self ( - std::unique_ptr (implicit_self)); - CanonicalPath Self = CanonicalPath::get_big_self (trait.get_node_id ()); - + ResolveGenericParam::go (trait.get_implicit_self (), prefix, + canonical_prefix); for (auto &generic : trait.get_generic_params ()) ResolveGenericParam::go (*generic, prefix, canonical_prefix); // Self is an implicit TypeParam so lets mark it as such resolver->get_type_scope ().append_reference_for_def ( - Self.get_node_id (), implicit_self->get_node_id ()); + trait.get_node_id (), trait.get_implicit_self ().get_node_id ()); if (trait.has_type_param_bounds ()) { diff --git a/gcc/rust/resolve/rust-early-name-resolver.cc b/gcc/rust/resolve/rust-early-name-resolver.cc index 14d204150408..634523181139 100644 --- a/gcc/rust/resolve/rust-early-name-resolver.cc +++ b/gcc/rust/resolve/rust-early-name-resolver.cc @@ -353,6 +353,8 @@ EarlyNameResolver::visit (AST::TraitItemType &) void EarlyNameResolver::visit (AST::Trait &trait) { + // shouldn't need to visit trait.get_implicit_self () + for (auto &generic : trait.get_generic_params ()) generic->accept_vis (*this); diff --git a/gcc/rust/resolve/rust-toplevel-name-resolver-2.0.cc b/gcc/rust/resolve/rust-toplevel-name-resolver-2.0.cc index a5e6c83a0378..badaaed1b776 100644 --- a/gcc/rust/resolve/rust-toplevel-name-resolver-2.0.cc +++ b/gcc/rust/resolve/rust-toplevel-name-resolver-2.0.cc @@ -103,23 +103,6 @@ TopLevel::visit (AST::Module &module) void TopLevel::visit (AST::Trait &trait) { - // FIXME: This Self injection is dodgy. It even lead to issues with metadata - // export in the past (#2349). We cannot tell appart injected parameters from - // regular ones. Dumping generic parameters highlights this Self in metadata, - // during debug or proc macro collection. This is clearly a hack. - // - // For now I'll keep it here in the new name resolver even if it should - // probably not be there. We need to find another way to solve this. - // Maybe an additional attribute to Trait ? - // - // From old resolver: - //// we need to inject an implicit self TypeParam here - //// FIXME: which location should be used for Rust::Identifier `Self`? - AST::TypeParam *implicit_self - = new AST::TypeParam ({"Self"}, trait.get_locus ()); - trait.insert_implict_self ( - std::unique_ptr (implicit_self)); - insert_or_error_out (trait.get_identifier ().as_string (), trait, Namespace::Types); diff --git a/gcc/testsuite/rust/compile/nr2/exclude b/gcc/testsuite/rust/compile/nr2/exclude index 92fa81517da8..6f8187264aae 100644 --- a/gcc/testsuite/rust/compile/nr2/exclude +++ b/gcc/testsuite/rust/compile/nr2/exclude @@ -1,9 +1,6 @@ attr-mismatch-crate-name.rs attr_deprecated.rs attr_deprecated_2.rs -auto_trait_super_trait.rs -auto_trait_valid.rs -auto_trait_invalid.rs bad=file-name.rs bounds1.rs break-rust2.rs @@ -152,7 +149,6 @@ privacy1.rs privacy3.rs privacy4.rs privacy5.rs -privacy6.rs privacy8.rs macros/proc/attribute_non_function.rs macros/proc/derive_non_function.rs @@ -171,8 +167,6 @@ specify-crate-name.rs stmt_with_block_dot.rs struct-expr-parse.rs traits1.rs -traits10.rs -traits11.rs traits12.rs traits2.rs traits3.rs @@ -181,7 +175,6 @@ traits5.rs traits6.rs traits7.rs traits8.rs -traits9.rs type-bindings1.rs unconstrained_type_param.rs undeclared_label.rs @@ -191,7 +184,6 @@ v0-mangle1.rs v0-mangle2.rs while_break_expr.rs negative_impls.rs -auto_trait.rs exhaustiveness1.rs exhaustiveness2.rs exhaustiveness3.rs @@ -199,7 +191,6 @@ trait13.rs trait14.rs issue-2324-1.rs issue-2324-2.rs -issue-2725.rs issue-2987.rs issue-3045-1.rs issue-3045-2.rs diff --git a/gcc/testsuite/rust/link/generic_function_0.rs b/gcc/testsuite/rust/link/generic_function_0.rs index 179c822c7fc2..58b8eb13db66 100644 --- a/gcc/testsuite/rust/link/generic_function_0.rs +++ b/gcc/testsuite/rust/link/generic_function_0.rs @@ -1,6 +1,3 @@ -// { dg-xfail-if "https://github.com/Rust-GCC/gccrs/issues/2349" { *-*-* } } -// { dg-excess-errors "" { xfail *-*-* } } - extern crate generic_function_1; use generic_function_1::generic_function; diff --git a/gcc/testsuite/rust/link/trait_import_0.rs b/gcc/testsuite/rust/link/trait_import_0.rs index 1b8c90a35742..ac8c5811d22b 100644 --- a/gcc/testsuite/rust/link/trait_import_0.rs +++ b/gcc/testsuite/rust/link/trait_import_0.rs @@ -1,6 +1,3 @@ -// { dg-xfail-if "https://github.com/Rust-GCC/gccrs/issues/2349" { *-*-* } } -// { dg-excess-errors "" { xfail *-*-* } } - extern crate trait_import_1; use trait_import_1::Add; diff --git a/gcc/testsuite/rust/link/trait_import_1.rs b/gcc/testsuite/rust/link/trait_import_1.rs index fc7f5168ede1..e54b0e19d44c 100644 --- a/gcc/testsuite/rust/link/trait_import_1.rs +++ b/gcc/testsuite/rust/link/trait_import_1.rs @@ -1,3 +1,6 @@ +#[lang = "sized"] +pub trait Sized {} + #[lang = "add"] pub trait Add { type Output;