#[non_exhaustive] stucts can be matched exhaustively using constants as patterns

[Skgland]

In rust-lang/rfcs#3535 (comment) it was noticed that it is possible to match a #[non_exhaustive] struct exhaustively outside the originating crate,
if constants for all posible states are accessible or definable in the other crate.

As a result adding fields such that the struct has new states is a breaking change, as the existing constants won't be able to cover all states anymore.
Which makes #[non_exhaustive] ineffective.

For Example:

// 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 => {}
    }
}

Changing the bar crate to the following is then a breaking change:

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

This breaks the foo crate, eventhough Bar was marked #[non_exhaustive] to allow expanding.

Reproduction Repo

Meta

rustc +stable --version
rustc 1.74.1 (a28077b28 2023-12-04)
rustc +beta --version
rustc 1.76.0-beta.1 (0e09125c6 2023-12-21)
rustc +nightly --version
rustc 1.77.0-nightly (d6d7a9386 2023-12-22)

[clarfonthey]

I'm not sure whether I'd call this a bug or just a weird set of mechanics combining together in a way that is very counter-intuitive. The purpose of #[non_exhaustive] for structs, and in fact what the exhaustiveness is referring to, is the fact that downstream users cannot exhaustively list all the fields and thus cannot construct them.

But if you construct values for them, there's no problem. And if, in fact, you construct all possible values for them, there's nothing stopping them from exhaustively matching with them.

In fact, based upon the original non_exhaustive RFC, this should probably have warned the user already, since private fields already render the constructor private and thus the attribute adds nothing.

However, I can definitely see the argument that non_exhaustive should also prevent exhaustive matching regardless of extra constants, meaning it would be useful for private fields. I do think that would have to be done with an edition bump though, so, I think it'd be worth lang-team pitching in soon so either an RFC can be drafted or the change can just be implemented before the new edition comes out.

[Skgland]

I'm not sure whether I'd call this a bug or just a weird set of mechanics combining together in a way that is very counter-intuitive. The purpose of #[non_exhaustive] for structs, and in fact what the exhaustiveness is referring to, is the fact that downstream users cannot exhaustively list all the fields and thus cannot construct them.

The RFC states that they shouldn't be exhaustively matchable.

From RFC 2008 Summary:

Adding this hint to structs or enum variants will prevent downstream crates from constructing or exhaustively matching, to ensure that adding new fields is not a breaking change.

This is clearly violated here as adding fields to #[non_exhaustive] structs is a braking change due them being exhautively matchable.

[clarfonthey]

Right, I see where the ambiguity is, here. The point is that you can't exhaustively match the fields of a struct labelled with #[non_exhaustive], although this isn't specifically talking about the variants. I can't speak for everyone who read the RFC, but I can at least speak as the RFC author that this is what I meant here. This is further bolstered by the fact that the definition of "exhaustively match" for structs is clarified in this example:

And we can even exhaustively match on it, like so:

if let Ok(Config { window_width, window_height }) = load_config() {
    // ...
}

I certainly don't think that the RFC is clear enough we should break the status quo since, for better or worse, this is the behaviour the compiler has chosen since consts in patterns were allowed. It would make the most sense to change things on an edition boundary, and I'm very open to it; it's just that I don't think that we should necessarily retroactively change what current editions are doing on this particular technicality.

[Skgland]

This is further bolstered by the fact that the definition of "exhaustively match" for structs is clarified in this example:

And we can even exhaustively match on it, like so:

if let Ok(Config { window_width, window_height }) = load_config() {
    // ...
}

I think that quote is taken out of context,
as that part of the RFC talks about usage in the defining crate, were #[non_exhaustive] is not considered.

We can still construct our config within the defining crate like so:

let config = Config { window_width: 640, window_height: 480 };

And we can even exhaustively match on it, like so:

if let Ok(Config { window_width, window_height }) = load_config() {
    // ...
}

Directly after that it states the following for users/downstream crates

But users outside the crate won't be able to construct their own values, because otherwise, adding extra fields would be a breaking change.

Users can still match on Configs non-exhaustively, as usual:

let &Config { window_width, window_height, .. } = config;
But without the .., this code will fail to compile.

The examples here may not be helpfull in the case that is problematic here as the (implied) else branch of the if let effectively acts as a _ wildcard pattern and
let would fail for with a constant pattern unless the struct has only one value i.e. its effectively a unit type.

So this problem almost exclusively effects match which is not given as an example here.

I noticed it also states:

Although it should not be explicitly forbidden by the language to mark a struct with some private fields as non-exhaustive, it should emit a warning to tell the user that the attribute has no effect.

But as we can see in this issue, this contradicts the already cited statement from the summary.

Adding this hint to structs or enum variants will prevent downstream crates from constructing or exhaustively matching, to ensure that adding new fields is not a breaking change.

Also while my given example has private fields,
the same problem should apply if all fields were made public, as such not being subject to the contradiction.

Comming back to the next part of your comment:

I certainly don't think that the RFC is clear enough we should break the status quo since, for better or worse, this is the behaviour the compiler has chosen since consts in patterns were allowed. It would make the most sense to change things on an edition boundary, and I'm very open to it; it's just that I don't think that we should necessarily retroactively change what current editions are doing on this particular technicality.

While disagreeing with the RFC author on this seams a bit silly, I don't find the RFC to be ambiguouse.

The consitent way to resolve the contradiction caused by the interaction with constants as patterns would be to treat private fields the same as public fields, to presereve the goal of adding fields is not a breaking change.

I agree that breaking existing code is bad and this might need an editon to fix.
I expect this to not be a large problem in practice though
which might make an initial Future Incompatibility Warning
with later error a option, crater might tell.

To be clear I also wouldn't forbid constant as patterns for
#[non_exhaustive] structs as a constant will never fail to match all the structs fields.
But the exhaustive-ness check sould ignore const patterns for checking whether all values are covered.

[clarfonthey]

Right, like I said, I think that we can agree that changing this going forward is good. The main reason why I suggest making it an edition change is because it wasn't really specified in the RFC, is technically breaking, and it doesn't feel like it's compelling to break now. But I guess we could just run crater to see if it breaks anything.

Will leave that decision up to the folks implementing. I can draft up an RFC for this if people think that it would be good to have, or lang-team can decide if this would be reasonable to just implement without an RFC.