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?
This commit is contained in:
bors 2024-05-09 08:34:14 +00:00
commit cb93c24bf3
7 changed files with 54 additions and 17 deletions

View File

@ -31,7 +31,7 @@ pub(crate) fn expand_deriving_partial_eq(
}; };
// We received arguments of type `&T`. Convert them to type `T` by stripping // 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. // it results in better error messages if something goes wrong.
// //
// Note: for arguments that look like `&{ x }`, which occur with packed // Note: for arguments that look like `&{ x }`, which occur with packed
@ -53,8 +53,7 @@ pub(crate) fn expand_deriving_partial_eq(
inner.clone() inner.clone()
} }
} else { } else {
// No leading `&`: add a leading `*`. expr.clone()
cx.expr_deref(field.span, expr.clone())
} }
}; };
cx.expr_binary( cx.expr_binary(

View File

@ -123,7 +123,7 @@ fn assert_ty_bounds(
span: Span, span: Span,
assert_path: &[Symbol], 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"); assert!(!ty.kind.is_anon_adt(), "Anonymous structs or unions cannot be type parameters");
// Generate statement `let _: assert_path<ty>;`. // Generate statement `let _: assert_path<ty>;`.
let span = cx.with_def_site_ctxt(span); let span = cx.with_def_site_ctxt(span);

View File

@ -315,7 +315,7 @@ fn make_format_args(
let mut used = vec![false; args.explicit_args().len()]; let mut used = vec![false; args.explicit_args().len()];
let mut invalid_refs = Vec::new(); 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> { enum ArgRef<'a> {
Index(usize), Index(usize),
@ -336,7 +336,7 @@ fn make_format_args(
used[index] = true; used[index] = true;
if arg.kind.ident().is_some() { if arg.kind.ident().is_some() {
// This was a named argument, but it was used as a positional argument. // 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) Ok(index)
} else { } 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 // 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. // 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 { 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 { let (position_sp_to_replace, position_sp_for_msg) = match used_as {
Placeholder(pspan) => (span, pspan), Placeholder(pspan) => (span, pspan),
Precision => { Precision => {

View File

@ -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 --> $DIR/derives-span-PartialEq-enum-struct-variant.rs:9:6
| |
LL | #[derive(PartialEq)] LL | #[derive(PartialEq)]

View File

@ -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 --> $DIR/derives-span-PartialEq-enum.rs:9:6
| |
LL | #[derive(PartialEq)] LL | #[derive(PartialEq)]

View File

@ -156,6 +156,20 @@ enum EnumGeneric<T, U> {
Two(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. // A union. Most builtin traits are not derivable for unions.
#[derive(Clone, Copy)] #[derive(Clone, Copy)]
pub union Union { pub union Union {

View File

@ -876,7 +876,7 @@ impl ::core::cmp::PartialEq for Enum1 {
fn eq(&self, other: &Enum1) -> bool { fn eq(&self, other: &Enum1) -> bool {
match (self, other) { match (self, other) {
(Enum1::Single { x: __self_0 }, Enum1::Single { x: __arg1_0 }) => (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 && __self_discr == __arg1_discr &&
match (self, other) { match (self, other) {
(Mixed::R(__self_0), Mixed::R(__arg1_0)) => (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 { (Mixed::S { d1: __self_0, d2: __self_1 }, Mixed::S {
d1: __arg1_0, d2: __arg1_1 }) => d1: __arg1_0, d2: __arg1_1 }) =>
*__self_0 == *__arg1_0 && *__self_1 == *__arg1_1, __self_0 == __arg1_0 && __self_1 == __arg1_1,
_ => true, _ => true,
} }
} }
@ -1245,11 +1245,11 @@ impl ::core::cmp::PartialEq for Fielded {
__self_discr == __arg1_discr && __self_discr == __arg1_discr &&
match (self, other) { match (self, other) {
(Fielded::X(__self_0), Fielded::X(__arg1_0)) => (Fielded::X(__self_0), Fielded::X(__arg1_0)) =>
*__self_0 == *__arg1_0, __self_0 == __arg1_0,
(Fielded::Y(__self_0), Fielded::Y(__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)) => (Fielded::Z(__self_0), Fielded::Z(__arg1_0)) =>
*__self_0 == *__arg1_0, __self_0 == __arg1_0,
_ => unsafe { ::core::intrinsics::unreachable() } _ => unsafe { ::core::intrinsics::unreachable() }
} }
} }
@ -1368,9 +1368,9 @@ impl<T: ::core::cmp::PartialEq, U: ::core::cmp::PartialEq>
__self_discr == __arg1_discr && __self_discr == __arg1_discr &&
match (self, other) { match (self, other) {
(EnumGeneric::One(__self_0), EnumGeneric::One(__arg1_0)) => (EnumGeneric::One(__self_0), EnumGeneric::One(__arg1_0)) =>
*__self_0 == *__arg1_0, __self_0 == __arg1_0,
(EnumGeneric::Two(__self_0), EnumGeneric::Two(__arg1_0)) => (EnumGeneric::Two(__self_0), EnumGeneric::Two(__arg1_0)) =>
*__self_0 == *__arg1_0, __self_0 == __arg1_0,
_ => unsafe { ::core::intrinsics::unreachable() } _ => 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. // A union. Most builtin traits are not derivable for unions.
pub union Union { pub union Union {
pub b: bool, pub b: bool,