derive(PartialEq): Add partial implementation (hehe) #3415

CohenArthur · 2025-02-04T08:42:55Z

This adds a base for deriving PartialEq and Eq but misses some more complex stuff like struct enum items, as we need to add more to our nameres and codegen to handle these rebinding patterns.

~~We also need to rework this part of the expansion pass as derive(Eq) and derive(PartialEq) should both create two impls: one of their trait, and one of the associated Structural* trait~~ this is now done in this PR.

This is ready for reviewing and merging IMO as a first basic implementation of derive(PartialEq) even if it does not support all types of Rust enums

P-E-P · 2025-02-07T12:39:28Z

gcc/rust/ast/rust-ast-builder.cc

+std::unique_ptr<Expr>
+Builder::literal_bool (bool b) const
+{
+  auto str = b ? "true" : "false";


We should probably use the keyword value from rust-keyword-values.h.

P-E-P · 2025-02-07T13:32:53Z

gcc/rust/expand/rust-derive-eq.cc

+  std::unique_ptr<AssociatedItem> &&fn, std::string name,
+  const std::vector<std::unique_ptr<GenericParam>> &type_generics)
+{
+  auto eq = builder.type_path ({"core", "cmp", "Eq"}, true);


Should we make this path (or individual segments) available globally like weak keywords ?

I don't think that's important here as this is only used when generating code, which isn't done too often. we rarely repeat paths, and making individual segments available doesn't really help a lot I feel

We should also probably create two type paths here, one for creating eq_impl and one for creating eq_generics -- otherwise, two AST nodes sharing the same node id throws the 2.0 name resolver for a loop

Thinking it might be best to just ignore duplicate map_usage calls, as long as one usage is only ever mapped to one definition, in which case my previous suggestion can be ignored

I think it's probably cleaner to have different node IDs for different nodes even if it makes the derive code very slightly messier

P-E-P · 2025-02-07T13:38:03Z

gcc/rust/expand/rust-derive-eq.cc

+	  // nothing to do as they contain no inner types
+	  continue;
+	  case EnumItem::Kind::Tuple: {
+	    auto tuple = static_cast<EnumItemTuple &> (*variant);


We want to avoid the copy here auto &tuple = static_cast<EnumItemTuple &> (*variant);.

P-E-P · 2025-02-07T13:38:12Z

gcc/rust/expand/rust-derive-eq.cc

+	    break;
+	  }
+	  case EnumItem::Kind::Struct: {
+	    auto tuple = static_cast<EnumItemStruct &> (*variant);


P-E-P · 2025-02-07T13:41:15Z

gcc/rust/expand/rust-derive-partial-eq.cc

@@ -0,0 +1,313 @@
+// Copyright (C) 2020-2024 Free Software Foundation, Inc.


Date range is incorrect, should be 2025 instead of 2020-2024.

gcc/rust/expand/rust-derive-partial-eq.cc

powerboat9 · 2025-02-07T19:45:13Z

gcc/rust/expand/rust-derive-eq.cc

+DeriveEq::assert_param_is_eq ()
+{
+  auto eq_bound = std::unique_ptr<TypeParamBound> (
+    new TraitBound (builder.type_path ({"core", "cmp", "Eq"}, true), loc));


Should this be a lang item path?

no, Eq isn't a lang item in 1.49:

#[doc(alias = "==")] #[doc(alias = "!=")] #[stable(feature = "rust1", since = "1.0.0")] pub trait Eq: PartialEq<Self> { // this method is used solely by #[deriving] to assert // that every component of a type implements #[deriving] // itself, the current deriving infrastructure means doing this // assertion without using a method on this trait is nearly // impossible. // // This should never be implemented by hand. #[doc(hidden)] #[inline] #[stable(feature = "rust1", since = "1.0.0")] fn assert_receiver_is_total_eq(&self) {} }

powerboat9 · 2025-02-07T19:51:53Z

gcc/testsuite/rust/compile/nr2/exclude

 issue-3402-1.rs
 for-loop1.rs
 for-loop2.rs
 issue-3403.rs
+derive-default1.rs


unnecessary move

oops, that's merge conflicts when rebasing - thanks for catching it, I'll fix it

gcc/rust/ChangeLog: * expand/rust-derive.cc (DeriveVisitor::derive): Call into DeriveEq. * expand/rust-derive-eq.cc: New file. * expand/rust-derive-eq.h: New file. * Make-lang.in: Compile them. gcc/testsuite/ChangeLog: * rust/compile/derive-eq-invalid.rs: New test.

gcc/rust/ChangeLog: * ast/rust-ast-builder.cc (Builder::block): Change return type. (Builder::loop): Use new APIs. * ast/rust-ast-builder.h: Change return type of block functions.

gcc/rust/ChangeLog: * ast/rust-ast-builder.cc (Builder::literal_bool): New method. (Builder::comparison_expr): Likewise. (Builder::boolean_operation): Likewise. * ast/rust-ast-builder.h: Declare them.

gcc/rust/ChangeLog: * expand/rust-derive-clone.cc: Cleanup implementation, avoid repetitions. * expand/rust-derive-clone.h: Likewise.

We are still missing some deriving for enums, as part of our codegen and nameres for rebinding struct field patterns is missing. gcc/rust/ChangeLog: * expand/rust-derive-partial-eq.cc: New file. * expand/rust-derive-partial-eq.h: New file. * expand/rust-derive.cc (DeriveVisitor::derive): Call them. * Make-lang.in: Compile them. gcc/testsuite/ChangeLog: * rust/compile/derive-eq-invalid.rs: Mark PartialEq def as a lang item. * rust/compile/derive-partialeq1.rs: New test. * rust/execute/torture/derive-partialeq1.rs: New test. * rust/compile/nr2/exclude: Exclude all of them.

gcc/rust/ChangeLog: * expand/rust-derive.cc (DeriveVisitor::derive): Return a vector of items. * expand/rust-derive.h: Change return type. * expand/rust-expand-visitor.cc: Insert all generated items into the AST.

gcc/rust/ChangeLog: * expand/rust-derive-partial-eq.cc: Adapt signatures to generate two impls. * expand/rust-derive-partial-eq.h: Likewise. * expand/rust-derive.cc (DeriveVisitor::derive): Adapt to multiple item generation. gcc/testsuite/ChangeLog: * rust/compile/derive-eq-invalid.rs: Declare StructuralPartialEq. * rust/compile/derive-partialeq1.rs: Likewise. * rust/execute/torture/derive-partialeq1.rs: Likewise.

gcc/rust/ChangeLog: * expand/rust-derive-eq.cc: Adapt functions to return two generated impls. * expand/rust-derive-eq.h: Likewise. * expand/rust-derive.cc (DeriveVisitor::derive): Likewise.

gcc/rust/ChangeLog: * expand/rust-derive-eq.cc: Copy `Eq` typepath.

CohenArthur force-pushed the derive-partialeq-eq branch 4 times, most recently from 7243ef7 to d50fc0c Compare February 4, 2025 15:14

P-E-P self-requested a review February 5, 2025 14:11

CohenArthur added this to the Macro expansion fixes core 1.49 milestone Feb 6, 2025

CohenArthur force-pushed the derive-partialeq-eq branch 2 times, most recently from b9c5bdf to 741e633 Compare February 6, 2025 10:07

P-E-P requested changes Feb 7, 2025

View reviewed changes

powerboat9 reviewed Feb 7, 2025

View reviewed changes

CohenArthur force-pushed the derive-partialeq-eq branch from 741e633 to 58c9608 Compare February 17, 2025 12:37

CohenArthur added 9 commits February 20, 2025 09:12

ast-builder: Improve function generation.

eb3d4b1

gcc/rust/ChangeLog: * ast/rust-ast-builder.cc (Builder::block): Change return type. (Builder::loop): Use new APIs. * ast/rust-ast-builder.h: Change return type of block functions.

ast-builder: Add new methods for creating operator expressions

0200fde

gcc/rust/ChangeLog: * ast/rust-ast-builder.cc (Builder::literal_bool): New method. (Builder::comparison_expr): Likewise. (Builder::boolean_operation): Likewise. * ast/rust-ast-builder.h: Declare them.

derive(Clone): chore: Cleanup

c918928

gcc/rust/ChangeLog: * expand/rust-derive-clone.cc: Cleanup implementation, avoid repetitions. * expand/rust-derive-clone.h: Likewise.

expand: Allow built-in derives to generate multiple items

9a241fb

gcc/rust/ChangeLog: * expand/rust-derive.cc (DeriveVisitor::derive): Return a vector of items. * expand/rust-derive.h: Change return type. * expand/rust-expand-visitor.cc: Insert all generated items into the AST.

derive(Eq): Also derive StructuralEq

c9f8757

gcc/rust/ChangeLog: * expand/rust-derive-eq.cc: Adapt functions to return two generated impls. * expand/rust-derive-eq.h: Likewise. * expand/rust-derive.cc (DeriveVisitor::derive): Likewise.

derive(Eq): Create copies of Eq typepath to avoid NodeId confusion

1149921

gcc/rust/ChangeLog: * expand/rust-derive-eq.cc: Copy `Eq` typepath.

CohenArthur force-pushed the derive-partialeq-eq branch from 58c9608 to 1149921 Compare February 20, 2025 09:12

CohenArthur enabled auto-merge February 20, 2025 09:12

CohenArthur added this pull request to the merge queue Feb 20, 2025

Merged via the queue into Rust-GCC:master with commit 35afe39 Feb 20, 2025
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

derive(PartialEq): Add partial implementation (hehe) #3415

derive(PartialEq): Add partial implementation (hehe) #3415

CohenArthur commented Feb 4, 2025 •

edited

Loading

P-E-P Feb 7, 2025

P-E-P Feb 7, 2025

CohenArthur Feb 17, 2025

powerboat9 Feb 17, 2025

powerboat9 Feb 18, 2025

CohenArthur Feb 20, 2025

P-E-P Feb 7, 2025

P-E-P Feb 7, 2025

P-E-P Feb 7, 2025

powerboat9 Feb 7, 2025

CohenArthur Feb 17, 2025

powerboat9 Feb 17, 2025

powerboat9 Feb 7, 2025

CohenArthur Feb 17, 2025

		@@ -0,0 +1,313 @@
		// Copyright (C) 2020-2024 Free Software Foundation, Inc.

derive(PartialEq): Add partial implementation (hehe) #3415

derive(PartialEq): Add partial implementation (hehe) #3415

Conversation

CohenArthur commented Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CohenArthur commented Feb 4, 2025 •

edited

Loading