Illegal Eq derive in an enum with Option<f64>

[stepantubanov]

I tried this code:

#[derive(PartialEq, Eq)]
pub enum Value {
    Boolean(Option<bool>),
    Float(Option<f64>),
}

fn main() {
    let a = Value::Float(Some(f64::NAN));
    
    // Eq should always have a == a
    // https://doc.rust-lang.org/std/cmp/trait.Eq.html
    assert!(a == a);
}

I expected to see this happen: code fails to compile.
Instead, this happened: code compiles, and assertion fails.

Changing order of enum variants to make Float go first fixes the issue:

// error[E0277] the trait bound `f64: Eq` is not satisfied
#[derive(PartialEq, Eq)]
                    -- in this derive macro expansion
pub enum Value {
    Float(Option<f64>),
    Boolean(Option<bool>),
}

Meta

rustc --version --verbose:

rustc 1.64.0 (a55dd71d5 2022-09-19)
binary: rustc
commit-hash: a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52
commit-date: 2022-09-19
host: aarch64-apple-darwin
release: 1.64.0
LLVM version: 14.0.6

It's reproducible in Playground in Stable, Beta and Nightly.

[fuzzypixelz]

I have played around a bit with the example and got rustc to compile an even bigger class of bad programs:

pub struct Wrap<T> {
    inner: T,
}

impl<T> PartialEq for Wrap<T>
where
    T: PartialEq,
{
    fn eq(&self, other: &Self) -> bool {
        self.inner.eq(&other.inner)
    }
}

impl<T> Eq for Wrap<T> where T: Eq {}

#[derive(PartialEq, Eq)]
pub struct SuperWrap<T> {
    thing: Wrap<T>,
    float: Wrap<f64>
}

fn reflexivity<T>(a: &T) where T: Eq {
    assert!(a == a)
}

fn main() {
    let a = SuperWrap {
        thing: Wrap { inner: 42_u64 },
        float: Wrap { inner: f64::NAN },
    };
    reflexivity(&a);
}

You can reproduce this in the Playground.

As you can see, the compiler is tricked into thinking that SuperWrap<u32> implements Eq.
This works with sum and product types, as long as:

• Wrap<T> is the first field (or variant)
• T and f64 are "wrapped" with the same type, i.e Box, Vec or Option.

I have little experience with rustc internals. Maybe someone more experienced could direct us to where this magic happens?

[Noratrieb]

Expanding the macro yields a

impl ::core::cmp::Eq for Value {
    #[inline]
    #[doc(hidden)]
    #[no_coverage]
    fn assert_receiver_is_total_eq(&self) -> () {
        let _: ::core::cmp::AssertParamIsEq<Option<bool>>;
    }
}

looks like it only asserts Eqness for the first field - whoops!

@rustbot claim

[Noratrieb]

Gonna hand it over to you @fuzzypixelz as discussed on zulip
@rustbot release

[fuzzypixelz]

@rustbot claim

[SNCPlay42]

On 1.63.0 this failed to compile, as expected (godbolt):

error[E0277]: the trait bound `f64: Eq` is not satisfied
 --> <source>:4:11
  |
1 | #[derive(PartialEq, Eq)]
  |                     -- in this derive macro expansion
...
4 |     Float(Option<f64>),
  |           ^^^^^^^^^^^ the trait `Eq` is not implemented for `f64`
  |
  = help: the following other types implement trait `Eq`:
            i128
            i16
            i32
            i64
            i8
            isize
            u128
            u16
          and 4 others
  = note: required because of the requirements on the impl of `Eq` for `Option<f64>`
note: required by a bound in `AssertParamIsEq`
  = note: this error originates in the derive macro `Eq` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

@rustbot label regression-from-stable-to-stable

[Noratrieb]

this was recently introduced when @nnethercote was doing derive optimizations.

[nnethercote]

I'll take a look today, thanks for the report.

[Noratrieb]

Don't worry, we've found the issue already, just wanted to let you know.

[nnethercote]

See #103176 for the fix.

[apiraino]

WG-prioritization assigning priority a posteriori (Zulip discussion) mostly to help tracking the fix progress.

@rustbot label -I-prioritize +P-high