consts in patterns in const contexts with feature effects causes warning (soon: error) about not implementing PartialEq

[bend-n]

Code

#![feature(effects)]
#[derive(Eq, PartialEq)]
pub struct Y(u8);
pub const GREEN: Y = Y(4);
pub const fn is_green(x: Y) -> bool {
    match x { GREEN => true, _ => false }
}

Current output

warning: to use a constant of type `Y` in a pattern, the type must implement `PartialEq`
 --> src/main.rs:7:15
  |
7 |     match x { GREEN => true, _ => false }
  |               ^^^^^
  |
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
  = note: `#[warn(const_patterns_without_partial_eq)]` on by default

Desired output

Rationale and extra context

the T is Derive(PartialEq); why does it warn?

Other cases

No response

Anything else?

as requested by @fmease

[compiler-errors]

the T is Derive(PartialEq); why does it warn?

Because T does not implement const PartialEq.

[bend-n]

Yes, i understand that, but it still seems like a incorrect diagnostic.

[fmease]

#[derive_const] doesn't help btw., it doesn't get recognized by the lint either (it throws an error).

[compiler-errors]

@fmease: What do you mean by that? That lint isn't recognizing a derive, it's looking for a PartialEq impl.

For the record, this code works:

#![feature(derive_const, effects)]

#[derive_const(PartialEq)]
#[derive(Eq)] // `Eq` is not a const trait, so we must regular-derive it.
pub struct Y(u8);
pub const GREEN: Y = Y(4);
pub const fn is_green(x: Y) -> bool {
    match x { GREEN => true, _ => false }
}

[bend-n]

Er, excuse me, but what is derive_const? the unstable docs just say "this feature is internal to the rust compiler".

[fmease]

Revisiting this, yesterday I totally missed that the match was inside a const fn, I just saw fn 🤦. So it definitely makes sense that const_patterns_without_partial_eq expects an impl const.

Therefore this reduces to a pure diagnostics issue (actually I'm not super sure, see RalfJ's RFC, rust-lang/rfcs#3535). Ideally, we would emit something like:

- warning: to use a constant of type `Y` in a pattern, the type must implement `PartialEq`
+ warning: to use a constant of type `Y` in a pattern in a const context, the type must implement `const PartialEq`
   --> src/main.rs:7:15
    |
  7 |     match x { GREEN => true, _ => false }
    |               ^^^^^
    |
    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
    = note: `#[warn(const_patterns_without_partial_eq)]` on by default

[fmease]

the unstable docs just say "this feature is internal to the rust compiler".

Oh, that's unfortunate, it's an auto-generated message. To clarify, it says likely internal (via). In this case, I don't think it's internal but simply an experimental feature without a tracking issue since the keyword generics / effects story is still ongoing.

#[derive_const] is similar to #[derive] except that the generated impl becomes an impl const (see feature const_trait_impl for more).

[fmease]

Hmm, nonetheless it's weird that we don't lint against that without effects, for forward compat we should (?), although there wouldn't be a way to fix it on stable. I also don't know how the lint code would be able to check that properly without effects enabled. Again, see rust-lang/rfcs#3535, section Unresolved Questions. By the way, sorry for my rambling and back and forth, it's still morning over here and I'm a tad tired ^^'.

[RalfJung]

The lint will become a hard error when #120805 lands.

Note that this code works on stable and has worked for quite a while:

#[derive(Eq, PartialEq)]
pub struct TypeThatIsPartialEq(u8);
pub const GREEN: TypeThatIsPartialEq = TypeThatIsPartialEq(4);

pub const fn is_green(x: TypeThatIsPartialEq) -> bool {
    match x { GREEN => true, _ => false }
}

So, I think there's a bug with the effects feature here somewhere, in how it interacts with the const-pattern check, which leads to the code being rejected with feature(effects).

[fmease]

No longer reproduces.

[RalfJung]

Cc @compiler-errors who changed a bunch of stuff here recently.
Not sure if there was a proper fix or the issue is just masked for now.

[compiler-errors]

@RalfJung: This is a real fix, because we no longer erroneously require that T: const PartialEq hold for it to be used in a pattern in a const function.

It's still somewhat conceptually dissatisfying/strange, though. In an abstract sense, it kinda feels right that we enforce that T: const PartialEq hold if it's used in a pattern in a const function, since it seems antithetical to the whole concept of matching being equivalent to emitting PartialEq calls if we don't?

[RalfJung]

In an abstract sense, it kinda feels right that we enforce that T: const PartialEq hold if it's used in a pattern in a const function, since it seems antithetical to the whole concept of matching being equivalent to emitting PartialEq calls if we don't?

Agreed. But it'd be a breaking change to do that now... so this is basically just a back-compat hack.