Disallow matching on all-private structs

language design
1

Currently this compiles:

mod foo {
    pub struct Bar {
        _private: (),
    }
    
    pub fn make_bar() -> Bar {
        Bar {
            _private: (),
        }
    }
}

fn main() {
    let foo::Bar { .. } = foo::make_bar();
}

The problem with this is if foo is actually an external crate and decides to change Bar to an enum it becomes semver-breaking change. This is unexpected and impractical.

The natural expectation is that since all fields are private you can make type forward-compatible by making it private. I was relying on this heavily when writing errors - I always encapsulated enums in structs thinking I will change them to enums when I'm confident which variants I want to expose.

The current situation makes forward compatibility impossible and the value of matching against struct with private fields is essentially zero. People who want to make extra sure the type returned from a function didn't change can use let _: Bar = foo::make_bar(); instead.

I see these possible solutions:

  • Deprecate such matching on private fields and later turn it into hard error. Declare that changing struct with private fields to enum is semver-compatible - my preferred, I expect there to be ~zero people actually matching like that so shouldn't be too much negative impact.
  • Deprecate such matching but never turn it into hard error
  • Allow matching with Name { .. } on enums - apart from being surprising it has the disadvantage of increasing MSRV
  • Add an attribute for structs that disallows such matching. This doesn't break existing code but increases MSRV and it's likely that people would still prefer to use this more often than not. It's also "public by default" kind of situation while Rust has generally "private by default" approach.
  • Do one of the first two and add an attribute to allow such matching. This can be added later any time if it's actually useful.

Given serious compatibility implications I think this deserves at least a warn by default lint in the rustc itself (not clippy) but if starting with clippy first is preferred it's still better than nothing.

6 Likes
2

I think this is clearly the best option; it's maybe somewhat odd, but it's not a breaking change to the language.

One preƫxisting feature to compare to is tuple structs. While it's most common to match tuple structs as Name(_, _), it's also possible to match them as Name { .. }.

Whatever the change to the language is, changing from an all private struct to an enum is going to be a breaking change to your library on any rustc before the change happens, so there's no difference w.r.t. old versions to which option is chosen.

1 Like
3

Oh, tuple structs are weird. Well, maybe combination of warning and allowing would be good so that the change is less painful for old MSRV crates if people update.

4

That's because they can be constructed and matched as braced structs with numbered fields.

Example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=087e30cc82d34347814507ac3959ae53

struct Name(u32, u64);

fn main() {
    let val = Name { 0: 10, 1: 30 };
    let Name { 0: foo, 1: bar } = val;
    assert_eq!(foo, 10);
    assert_eq!(bar, 30);
}
5 Likes
5

This has been the case since rust 1.19 which stabilized RFC 1506.

1 Like
6

(I knew about treating tuple structs as braced structs before.)

Quite relevant motivation from RFC 1506 which I think can be extended to enums here:

This also means that S{..} patterns can be used to match structures and variants of any kind. The desire to have such "match everything" patterns is sometimes expressed given that number of fields in structures and variants can change from zero to non-zero and back during development. An extra benefit is ability to match/construct tuple structs using their type aliases.

Either people are on a recent enough rustc that it's not an issue, or they're on an old enough rustc that it's an issue, and they won't be getting the warning because the rustc is too old. I see zero relevance as to what new rustcs can do to improve the forwards-compatibility of old rustcs.

Rustc only supports one version: the current version. While we do make some decisions so that we don't completely break e.g. old versions of Cargo, libraries supporting an older MSRV is currently solely a concern of the libraries, and not of rustc, which very formally only supports the most recent stable.

7

The one (minor) use for this is that you can use it as a type annotation, like

match foo() {
    Ok(v @ Vec { .. }) => ...

So I agree that this would make sense to discourage (and maybe remove in a later edition), but I think I'd like to get type annotations in pattern bindings first.

8

Not really, you can make code for old MSRV with new compiler. I often do this to make it possible to compile my stuff on stock Debian packages but lint with stable clippy and such to make the code better.

9

Are structs with no fields "all-private"? Logic says yes of course. OTOH such structs are publicly constructible despite being all-private (since they are also all-public), so arguably they should also be matchable.

7 Likes
10

Yes, that's a weird edge case with two seemingly contradicting things "all fields are private" and "all fields are public" But as you note they have a private constructor so there should be an exception to allow matching on them.

The idea is to define it as "cases when the author clearly took action to attempt to future-proof". Empty structs are well-known to be constructible, so there was no such attempt. Unless they have #[non_exhaustive] attribute, which is an exception of exception. :smiley:

if (struct_fields.is_empty() && !has_non_exhaustive) || struct_fields.iter().any(|field| field.is_public()) {
    allow_matching();
} else {
    disallow_matching();
}
11

s/private/public

But otherwise I agree :slight_smile:

1 Like
12

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.