about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-05-09 08:34:14 +0000
committerbors <bors@rust-lang.org>2024-05-09 08:34:14 +0000
commitcb93c24bf36b3367714516fc2308cf6856916eeb (patch)
treef0f6f12d69a3c7c50ac033892aacd75ae8fcd151
parent5f8c17dcc04a2981268df89874203e9bfea50597 (diff)
parentc2a0ef65da366c7e5235b8f9781278d62ff2f8e1 (diff)
downloadrust-cb93c24bf36b3367714516fc2308cf6856916eeb.tar.gz
rust-cb93c24bf36b3367714516fc2308cf6856916eeb.zip
Auto merge of #124157 - wutchzone:partial_eq, r=estebank
Do not add leading asterisk in the `PartialEq`

I think we should address this issue, however I am not exactly sure, if this is the right way to do it. It is related to the #123056.

Imagine the simplified code:

```rust
trait MyTrait {}

impl PartialEq for dyn MyTrait {
    fn eq(&self, _other: &Self) -> bool {
        true
    }
}

#[derive(PartialEq)]
enum Bar {
    Foo(Box<dyn MyTrait>),
}
```

On the nightly compiler, the `derive` produces invalid code with the weird error message:
```
error[E0507]: cannot move out of `*__arg1_0` which is behind a shared reference
  --> src/main.rs:11:9
   |
9  | #[derive(PartialEq)]
   |          --------- in this derive macro expansion
10 | enum Things {
11 |     Foo(Box<dyn MyTrait>),
   |         ^^^^^^^^^^^^^^^^ move occurs because `*__arg1_0` has type `Box<dyn MyTrait>`, which does not implement the `Copy` trait
   |
   = note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)
```

It may be related to the perfect derive problem, although requiring the _type_ to be `Copy` seems unfortunate because it is not necessary. Besides, we are adding the extra dereference only for the diagnostics?
-rw-r--r--compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs5
-rw-r--r--compiler/rustc_builtin_macros/src/deriving/mod.rs2
-rw-r--r--compiler/rustc_builtin_macros/src/format.rs6
-rw-r--r--tests/ui/derives/derives-span-PartialEq-enum-struct-variant.stderr2
-rw-r--r--tests/ui/derives/derives-span-PartialEq-enum.stderr2
-rw-r--r--tests/ui/deriving/deriving-all-codegen.rs14
-rw-r--r--tests/ui/deriving/deriving-all-codegen.stdout40
7 files changed, 54 insertions, 17 deletions
diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
index dc73caa4ad5..a6457f4a433 100644
--- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
+++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
@@ -31,7 +31,7 @@ pub(crate) fn expand_deriving_partial_eq(
                     };
 
                     // We received arguments of type `&T`. Convert them to type `T` by stripping
-                    // any leading `&` or adding `*`. This isn't necessary for type checking, but
+                    // any leading `&`. This isn't necessary for type checking, but
                     // it results in better error messages if something goes wrong.
                     //
                     // Note: for arguments that look like `&{ x }`, which occur with packed
@@ -53,8 +53,7 @@ pub(crate) fn expand_deriving_partial_eq(
                                 inner.clone()
                             }
                         } else {
-                            // No leading `&`: add a leading `*`.
-                            cx.expr_deref(field.span, expr.clone())
+                            expr.clone()
                         }
                     };
                     cx.expr_binary(
diff --git a/compiler/rustc_builtin_macros/src/deriving/mod.rs b/compiler/rustc_builtin_macros/src/deriving/mod.rs
index d6e2d1d4d07..e3a93ae13e4 100644
--- a/compiler/rustc_builtin_macros/src/deriving/mod.rs
+++ b/compiler/rustc_builtin_macros/src/deriving/mod.rs
@@ -123,7 +123,7 @@ fn assert_ty_bounds(
     span: Span,
     assert_path: &[Symbol],
 ) {
-    // Deny anonymous structs or unions to avoid wierd errors.
+    // Deny anonymous structs or unions to avoid weird errors.
     assert!(!ty.kind.is_anon_adt(), "Anonymous structs or unions cannot be type parameters");
     // Generate statement `let _: assert_path<ty>;`.
     let span = cx.with_def_site_ctxt(span);
diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs
index 2450ac8f4b3..a4d980465e4 100644
--- a/compiler/rustc_builtin_macros/src/format.rs
+++ b/compiler/rustc_builtin_macros/src/format.rs
@@ -315,7 +315,7 @@ fn make_format_args(
 
     let mut used = vec![false; args.explicit_args().len()];
     let mut invalid_refs = Vec::new();
-    let mut numeric_refences_to_named_arg = Vec::new();
+    let mut numeric_references_to_named_arg = Vec::new();
 
     enum ArgRef<'a> {
         Index(usize),
@@ -336,7 +336,7 @@ fn make_format_args(
                     used[index] = true;
                     if arg.kind.ident().is_some() {
                         // This was a named argument, but it was used as a positional argument.
-                        numeric_refences_to_named_arg.push((index, span, used_as));
+                        numeric_references_to_named_arg.push((index, span, used_as));
                     }
                     Ok(index)
                 } else {
@@ -544,7 +544,7 @@ fn make_format_args(
     // Only check for unused named argument names if there are no other errors to avoid causing
     // too much noise in output errors, such as when a named argument is entirely unused.
     if invalid_refs.is_empty() && !has_unused && !unnamed_arg_after_named_arg {
-        for &(index, span, used_as) in &numeric_refences_to_named_arg {
+        for &(index, span, used_as) in &numeric_references_to_named_arg {
             let (position_sp_to_replace, position_sp_for_msg) = match used_as {
                 Placeholder(pspan) => (span, pspan),
                 Precision => {
diff --git a/tests/ui/derives/derives-span-PartialEq-enum-struct-variant.stderr b/tests/ui/derives/derives-span-PartialEq-enum-struct-variant.stderr
index d0b14ef94c1..e88a523ef4f 100644
--- a/tests/ui/derives/derives-span-PartialEq-enum-struct-variant.stderr
+++ b/tests/ui/derives/derives-span-PartialEq-enum-struct-variant.stderr
@@ -1,4 +1,4 @@
-error[E0369]: binary operation `==` cannot be applied to type `Error`
+error[E0369]: binary operation `==` cannot be applied to type `&Error`
   --> $DIR/derives-span-PartialEq-enum-struct-variant.rs:9:6
    |
 LL | #[derive(PartialEq)]
diff --git a/tests/ui/derives/derives-span-PartialEq-enum.stderr b/tests/ui/derives/derives-span-PartialEq-enum.stderr
index f69451ac793..80b225446b4 100644
--- a/tests/ui/derives/derives-span-PartialEq-enum.stderr
+++ b/tests/ui/derives/derives-span-PartialEq-enum.stderr
@@ -1,4 +1,4 @@
-error[E0369]: binary operation `==` cannot be applied to type `Error`
+error[E0369]: binary operation `==` cannot be applied to type `&Error`
   --> $DIR/derives-span-PartialEq-enum.rs:9:6
    |
 LL | #[derive(PartialEq)]
diff --git a/tests/ui/deriving/deriving-all-codegen.rs b/tests/ui/deriving/deriving-all-codegen.rs
index 498930fc0c6..6fa4f74f2a5 100644
--- a/tests/ui/deriving/deriving-all-codegen.rs
+++ b/tests/ui/deriving/deriving-all-codegen.rs
@@ -156,6 +156,20 @@ enum EnumGeneric<T, U> {
     Two(U),
 }
 
+// An enum that has variant, which does't implement `Copy`.
+#[derive(PartialEq)]
+enum NonCopyEnum {
+    // The `dyn NonCopyTrait` implements `PartialEq`, but it doesn't require `Copy`.
+    // So we cannot generate `PartialEq` with dereference.
+    NonCopyField(Box<dyn NonCopyTrait>),
+}
+trait NonCopyTrait {}
+impl PartialEq for dyn NonCopyTrait {
+    fn eq(&self, _other: &Self) -> bool {
+        true
+    }
+}
+
 // A union. Most builtin traits are not derivable for unions.
 #[derive(Clone, Copy)]
 pub union Union {
diff --git a/tests/ui/deriving/deriving-all-codegen.stdout b/tests/ui/deriving/deriving-all-codegen.stdout
index 9f8a9f30ff6..6b69b57c516 100644
--- a/tests/ui/deriving/deriving-all-codegen.stdout
+++ b/tests/ui/deriving/deriving-all-codegen.stdout
@@ -876,7 +876,7 @@ impl ::core::cmp::PartialEq for Enum1 {
     fn eq(&self, other: &Enum1) -> bool {
         match (self, other) {
             (Enum1::Single { x: __self_0 }, Enum1::Single { x: __arg1_0 }) =>
-                *__self_0 == *__arg1_0,
+                __self_0 == __arg1_0,
         }
     }
 }
@@ -1119,10 +1119,10 @@ impl ::core::cmp::PartialEq for Mixed {
         __self_discr == __arg1_discr &&
             match (self, other) {
                 (Mixed::R(__self_0), Mixed::R(__arg1_0)) =>
-                    *__self_0 == *__arg1_0,
+                    __self_0 == __arg1_0,
                 (Mixed::S { d1: __self_0, d2: __self_1 }, Mixed::S {
                     d1: __arg1_0, d2: __arg1_1 }) =>
-                    *__self_0 == *__arg1_0 && *__self_1 == *__arg1_1,
+                    __self_0 == __arg1_0 && __self_1 == __arg1_1,
                 _ => true,
             }
     }
@@ -1245,11 +1245,11 @@ impl ::core::cmp::PartialEq for Fielded {
         __self_discr == __arg1_discr &&
             match (self, other) {
                 (Fielded::X(__self_0), Fielded::X(__arg1_0)) =>
-                    *__self_0 == *__arg1_0,
+                    __self_0 == __arg1_0,
                 (Fielded::Y(__self_0), Fielded::Y(__arg1_0)) =>
-                    *__self_0 == *__arg1_0,
+                    __self_0 == __arg1_0,
                 (Fielded::Z(__self_0), Fielded::Z(__arg1_0)) =>
-                    *__self_0 == *__arg1_0,
+                    __self_0 == __arg1_0,
                 _ => unsafe { ::core::intrinsics::unreachable() }
             }
     }
@@ -1368,9 +1368,9 @@ impl<T: ::core::cmp::PartialEq, U: ::core::cmp::PartialEq>
         __self_discr == __arg1_discr &&
             match (self, other) {
                 (EnumGeneric::One(__self_0), EnumGeneric::One(__arg1_0)) =>
-                    *__self_0 == *__arg1_0,
+                    __self_0 == __arg1_0,
                 (EnumGeneric::Two(__self_0), EnumGeneric::Two(__arg1_0)) =>
-                    *__self_0 == *__arg1_0,
+                    __self_0 == __arg1_0,
                 _ => unsafe { ::core::intrinsics::unreachable() }
             }
     }
@@ -1426,6 +1426,30 @@ impl<T: ::core::cmp::Ord, U: ::core::cmp::Ord> ::core::cmp::Ord for
     }
 }
 
+// An enum that has variant, which does't implement `Copy`.
+enum NonCopyEnum {
+
+    // The `dyn NonCopyTrait` implements `PartialEq`, but it doesn't require `Copy`.
+    // So we cannot generate `PartialEq` with dereference.
+    NonCopyField(Box<dyn NonCopyTrait>),
+}
+#[automatically_derived]
+impl ::core::marker::StructuralPartialEq for NonCopyEnum { }
+#[automatically_derived]
+impl ::core::cmp::PartialEq for NonCopyEnum {
+    #[inline]
+    fn eq(&self, other: &NonCopyEnum) -> bool {
+        match (self, other) {
+            (NonCopyEnum::NonCopyField(__self_0),
+                NonCopyEnum::NonCopyField(__arg1_0)) => __self_0 == __arg1_0,
+        }
+    }
+}
+trait NonCopyTrait {}
+impl PartialEq for dyn NonCopyTrait {
+    fn eq(&self, _other: &Self) -> bool { true }
+}
+
 // A union. Most builtin traits are not derivable for unions.
 pub union Union {
     pub b: bool,