RFC: constants in patterns

[RalfJung]

When a constant appears as a pattern, this is syntactic sugar for writing a pattern that corresponds to the constant's value by hand. This operation is only allowed when (a) the type of the constant implements PartialEq, and (b) the value of the constant being matched on has "structural equality", which means that PartialEq behaves the same way as that desugared pattern.

Thanks a lot to everyone who gave feedback in the Zulip discussion!

Rendered

[jhpratt]

While it is true that not having StructuralPartialEq be implied by #[derive(PartialEq)] would be massively breaking, I feel that requiring it be separate as soon as practicable (i.e. the next edition) would be ideal. Having a StructuralPartialEq implementation is naturally an API commitment, so it should be explicit insofar as it is possible to do so.

Aside from that and permitting manual implementations of StructuralPartialEq (which is mentioned in the RFC), I'm quite happy with the way everything is described.

[RalfJung]

It's opening a new design space though; t-lang expressed concerns that we already have way too long derive(...) declarations and should look into something like derive(Structural) to get "all the usual derives" or so. This RFC does not aim to solve all the problems in this space, it is an incremental step, and IMO decoupling the structural-equality-promise from #[derive(PartialEq)] is not part of that step. I want to push back against scope creep here.

[jhpratt]

My primary fear is that people will be making API commitments (and then breaking them) without even realizing it. It's a similar problem to traits being object safe without an explicit opt-in. If this existed previously, I would have broken it multiple times deliberately, as it's something I never intended to promise. I know I have done this for object safety, hoping¹ that no one would notice.

I understand the concerns over long derive declarations, even if I personally disagree. Was that a significant factor in weighing the pros & cons of each option?

Footnotes

1. successfully ↩

[RalfJung]

The issue you describe is pre-existing. I agree it is an issue. This RFC does not aim to solve it since it is entirely orthogonal to what the RFC aims to solve. (This RFC just needs some mechanism for type authors to opt-in to "structural equality". The details of that mechanism do not matter. We have an existing mechanism for this. It is flawed but this RFC does not make it worse.) The RFC already acknowledges this problem and mentions it as a "future possibility". There are other real issues that this RFC aims to solve and I don't think we need to unnecessarily entangle this.

Let's not require an RFC that clearly has net benefit to solve every problem in its space, please. This is just a surefire way to ensure nobody will dare suggest RFCs any more. We need room for incremental RFCs that solve a particular issue without also solving closely related (but entirely orthogonal) problems. It has already taken literal years to arrive at this RFC.

[jhpratt]

The issue you describe is pre-existing.

Code that runs into that can be written on stable? I was under the implication that it was nightly-only, where the traits currently live. If it can already be encountered on stable, then sure, I have no issue with continuing as-written.

Let's not require an RFC that clearly has net benefit to solve every problem in its space, please. This is just a surefire way to ensure nobody will dare suggest RFCs any more. We need room for incremental RFCs that solve a particular issue without also solving closely related (but entirely orthogonal) problems. It has already taken literal years to arrive at this RFC.

I know that it's not easy writing an RFC, let along "defending" it. If my understanding is not correct, then feel free to say so directly 🙂 I'm sure I'm not the only one with this question.

[RalfJung]

I didn't realize you assumed that this is nightly-only behavior. :)

This RFC does not allow any new code to work on stable. It only turns some future-compat lints into hard errors.

[clarfonthey]

One potential alternative to this is to allow negative impls for StructuralPartialEq for derived PartialEq, and/or perhaps to allow typing derive(PartialEq, !StructuralPartialEq).

If we think of StructuralPartialEq "like an auto trait" since it's infectious, this should make some amount of sense.

[RalfJung]

Given that we already had a lang design meeting that reached pretty much this conclusion, I am going to nominate this for t-lang discussion.

[CAD97]

It seems somewhat unfortunate that validity in patterns is value sensitive and not purely type directed. It's at least excusable with floats, since the behavior of NaN is reasonably well known to be non-total, but for other types it certainly surprised me that this was already the case.

Given it's already the case and it's not like it isn't already formally API breaking to change the value of a const item for multiple other reasons already, it's fine, it just surprised me (and I was writing a PR review with other useful comments anyway).

[RalfJung]

Not sure if it's unfortunate, people certainly like and use it.

[CAD97]

(It would be breaking, which means we almost certainly shouldn't do it, but) Given the logic that a const pattern is “logically desugared” into the corresponding literal pattern, it could be justified in forbidding matching over any type with inaccessible fields (or #[non_exhaustive]).

(Fun: because this sees into private fields, you can exhaustively match a type with all private fields. I unfortunately don't have my set up to test multiple crates right now, but I will be extremely amused if it turns out you can exhaustively match a #[non_exhaustive] struct. Downright dtolnay/rust-quiz material.)

[RalfJung]

it could be justified in forbidding matching over any type with inaccessible fields (or #[non_exhaustive]).

This is somewhat deliberate I think: a library can choose to expose certain constants that people can match on despite there being private fields.

I don't think this will let you do exhaustive matching on non_exhaustive enums. For structs there's no separate notion of exhaustiveness anyway, it's all per-field?

[Nadrieril]

For structs and struct variants, #[non_exhaustive] means "I may add fields in the future so you must always deconstruct with , ..". That doesn't change anything for constants-interpreted-as-patterns. I can't find a test for that case though, I'll add one when I get the chance

[Nemo157]

I will be extremely amused if it turns out you can exhaustively match a #[non_exhaustive] struct.

You can 🤦

// bar/src/lib.rs
#[non_exhaustive]
#[derive(PartialEq, Eq)]
pub struct Bar(bool);
pub const TRUE: Bar = Bar(true);
pub const FALSE: Bar = Bar(false);

// foo/src/main.rs
fn main() {
    match bar::TRUE {
        bar::TRUE => {}
        bar::FALSE => {}
    }
}

> cargo check
    Checking bar v0.1.0 (/tmp/scratch.rust.2023-12-17T12-41.GFlTlw/foo/bar)
    Checking foo v0.1.0 (/tmp/scratch.rust.2023-12-17T12-41.GFlTlw/foo)
    Finished dev [unoptimized + debuginfo] target(s) in 0.11s

[RalfJung]

That doesn't seem surprising to me, since this does not deconstruct the value at all, it just compares for equality.

But if this should lint/error then please file an issue.

[Skgland]

I don't think that should error. #[non_exhaustive] means "adding fields musn't be a breaking change", which is the case when using constants. The tricky case is accessing private fields, which is addressed in your RFC.

How would you add a non-unit type field without breaking the match?
Adding such fields would add more possible states, that would not be coverable by the two existing constant patterns.

[Nadrieril]

Oh hm fair point. Maybe it should be opt-in like for private fields

[obi1kenobi]

This appears to be a new (at least to me) semver hazard. I think it'd be very surprising to maintainers that it's possible to cause a breaking change by adding/tweaking a private field on a #[non_exhaustive] type.

I think this should either be a future-incompat warning -> error, or at least a lint. If one of the folks here who discovered this issue agrees and would like to open an issue for that, please tag me -- I'd like to keep on top of it for the sake of cargo-semver-checks. If nobody is interested in opening that issue, I'd be happy to do it.

[RalfJung]

Yeah please link the issue from here once it got created. :)

[Skgland]

As I looks like no has opened an issue yet, I have opened rust-lang/rust#119264

[clarfonthey]

While I know it's not a priority and not required for the initial RFC, I think that StructuralOrd should at least be mentioned in the future possibilities. For example, being able to do Jan..=Mar patterns for a custom month enum would be nice, and it isn't incompatible with the current proposal.

The main difference is that while matching enum variants for equality doesn't require StructuralEq, ranges would require StructuralOrd.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[traviscross]

We've accepted this RFC and it has now been merged.

Huge thanks to @RalfJung for doing the detailed work necessary to put this together and to move Rust forward.

We've opened a tracking issue over in rust-lang/rust#120362. Keep an eye on that issue for future updates.