RFC: Foo { .. } pattern matches non-struct types

[theemathas]

View all comments

Special case struct patterns containing only a rest pattern (e.g., Foo { .. }) so that they can match values of any type with the appropriate name, not just structs (e.g., it could match an enum Foo value). This is done so that structs containing only private fields can be changed to other types without breaking backwards compatibility.

Prior discussion on IRLO

Rendered

[programmerjake]

a related pet peeve: you can use Ty { 0: abc, 1: def } syntax on tuple structs or type aliases of tuple structs but not on type aliases of actual tuples, which makes macros more annoying

[WaffleLapkin]

I'm against this change.

A pattern is generally an inverse of an expression with the same syntax — & expression creates a reference, & pattern dereference it; S { a } expression creates a struct S from pieces, S { a } pattern destructures it; ... (an exception is ref vs *). This proposal muddies this concept, since it allows matching an option with Option { .. }, while an option can't be constructed with Option { ... } (... being a placeholder for literally anything).

I'm inclined to say that having the intuition of "if I can match it with a pattern, then I can create the value with a similar syntax" is good, and so breaking it is not so good.

I might have had a different opinion be this a proposal for a language with a good server story. But since rust's semver story is quite bad, I don't think this small improvement can justify the special case.

This is not to say we shouldn't improve rust's semver story. But in this particular case the downsides outweigh the improvement in an edge case.

[WaffleLapkin]

This is because all structures are just that, structures.

Unit structures are just a sugar for a struct with a constant:

struct Unit;
// <=>
struct Unit {}
const Unit: Unit = Unit {};

Similarly tuple structs are just a sugar for a struct with fields named as integers, a function, and a pattern (these are not expressible in surface level rust, but still):

struct Tuple(u8);
// <=>
struct Tuple { 0: u8 }
fn Tuple(_0: u8) -> Tuple { Tuple { 0: _0 } }
pattern Tuple(_0) = Tuple { 0: _0 }

So, for most intents and purposes all kinds of structs are the same.

(also this probably has a typo, I assume it should have said "not just named structs"?)

[scottmcm]

For more, see https://rust-lang.github.io/rfcs/1506-adt-kinds.html.

[theemathas]

After reading RFC 1506, I'm still rather confused on the motivation. It seems that the reason that a pattern like Tuple { 0: x } has the same meaning as Tuple(x) is for... macros? Am I missing something?

[camsteffen]

There is a benefit with this change other than semver compatibility - it can be useful to have an explicit type in a pattern instead of _, either for readability or to guard yourself against refactors where that type might change. I actually ran into a case recently where I wanted to do this:

match something {
    SomeStruct(SomeEnum { .. }, ....) =>  .., // future-proof: it's important that SomeEnum exists here
    ...

...instead I decided to do:

match something {
    SomeStruct(val, ....) =>  {
        let _: SomeEnum = val; // future-proof: it's important that SomeEnum exists here
        ...

[WaffleLapkin]

@camsteffen IMO it would be better to have explicit syntax for annotating types in patterns, if that's the goal.

[riking]

This should also mention motivation for macros generating match statements where they want to verify a user-passed type.

[theemathas]

Can't a match arm like this already be generated?

ref foo_val => {
    let _: &Foo = foo_val;
}

[traviscross]

What are the use cases for using the Foo { .. } pattern when Foo is a struct with only private fields? If we were to start linting against that, what would we be losing?

E.g.:

//~^ WARN matching against structs with only private fields is fragile
//~| NOTE this code would break if `Foo` became an `enum` or a `union`

[theemathas]

I did a cursory search manually for code on github that uses a pattern like Foo { .. }. It appears that there is some amount of people that uses such a pattern, probably to make sure that the value has the expected type. Here are some examples.

Code that uses this pattern on a struct with all-private fields from an external crate:

• https://github.com/buckyos/CYFS/blob/main/src/component/cyfs-lib/src/ws/session.rs#L245
• https://github.com/postgresml/pgcat/blob/main/src/dns_cache.rs#L374
• https://github.com/worldcoin/orb-software/blob/main/mcu-util/src/orb/security_board.rs#L402
• https://github.com/0xPolygonZero/zk_evm/blob/develop/zero/src/env.rs#L13
• https://github.com/calyxir/calyx/blob/main/calyx-backend/src/mlir.rs#L40
• https://github.com/Shizcow/dmenu-rs/blob/master/src/stest/src/main.rs#L19

Code that uses this pattern on a struct with public fields from an external crate:

• https://github.com/rust-lang/hashbrown/blob/master/src/map.rs#L5948
• https://github.com/ZettaScaleLabs/stabby/blob/main/stabby-macros/src/traits.rs#L1271
• https://github.com/Gankra/abi-cafe/blob/main/src/toolchains/c/declare.rs#L271-L276
• https://github.com/FuelLabs/sway/blob/master/forc-plugins/forc-doc/src/doc/mod.rs#L225

Code that uses this pattern on a struct defined in the same crate:

• https://github.com/rust-lang/rust/blob/master/library/alloc/tests/c_str2.rs#L112-L116
• https://github.com/google/zerocopy/blob/main/src/wrappers.rs#L494-L495
• https://github.com/slackhq/hakana/blob/main/src/code_info/ttype/comparison/atomic_type_comparator.rs#L218
• https://github.com/oli-obk/ui_test/blob/main/src/status_emitter/text.rs#L373
• https://github.com/Gompei/slack-rust/blob/main/src/socket/event.rs#L23-L35
• https://github.com/nascentxyz/pyrometer/blob/master/crates/graph/src/solvers/atoms.rs#L331-L332
• https://github.com/privacy-scaling-explorations/mpz/blob/dev/crates/mpz-circuits/src/types.rs#L88-L93

It seems to me that there is at least a decent number of rust users, including in some high-profile crates, that consider the Foo { .. } pattern to be at least worth using.

Is there a way to programmatically find uses of such a pattern?

[Jules-Bertholet]

I'm inclined to say that having the intuition of "if I can match it with a pattern, then I can create the value with a similar syntax" is good

This general guideline is nice, but it doesn’t really apply here. Foo { .. } to me does not suggest "this is how you make a Foo", but instead "I am deliberately omitting all details of how to make a Foo". And if a struct has private fields (the exact case this RFC is meant to address), you won’t be directly creating any values of it, with any syntax.

rust's semver story is quite bad

So we should work on making it better. Every little improvement will save some people some amount of pain when upgrading. This doesn’t have to be all-or-nothing.

[camsteffen]

This general guideline is nice, but it doesn’t really apply here. Foo { .. } to me does not suggest "this is how you make a Foo", but instead "I am deliberately omitting all details of how to make a Foo".

Right, my brain is already trained to think of it that way because of tuple structs. It's like "I'm telling the computer that I am specifying a type and not declaring a binding".

[Nadrieril]

For many years you could write (x: Type) in a pattern on nightly. I think that was never implemented fully and ended up being removed but sounds like this is the main reason people write Struct { .. } patterns for private structs today.

[lolbinarycat]

For many years you could write (x: Type) in a pattern on nightly. I think that was never implemented fully and ended up being removed but sounds like this is the main reason people write Struct { .. } patterns for private structs today.

this feature is called type ascription.

[Jules-Bertholet]

I implemented this RFC: rust-lang/rust#159295

[theemathas]

Lang-nominating, as @Jules-Bertholet requested T-lang to have a look at it to make a decision to accept or reject at #t-lang > Any chance for a review of RFC 3753?

[theemathas]

Related discussion: #t-lang > type ascription *in patterns*

[joshtriplett]

I think the RFC should explicitly say, in the top summary section, that this is not being added because we think the pattern is a thing people should write, just that it's a pattern we allow people to write and thus we need this to make sure it isn't a breaking change for crate maintainers.

I also think we should lint against such a pattern.

But after further consideration, 👍 for doing this to allow for backwards-compatible library crate evolution.

[scottmcm]

Is the expectation that I can also write @ u32 { .. }? Is it really any other type?

View changes since the review

[theemathas]

It looks kinda wonky, but yes. I would personally find it to be weirder if I can do this for enums but not i32.

[joshtriplett]

cc @obi1kenobi (cargo-semver-checks)

[traviscross]

We talked about this in the lang call at some length without specific resolution. Mostly people were a bit conflicted.

For my part, I see the motivation: allowing upstreams to change the item-kind of a type without breaking downstreams who might have relied on this syntax (and there aren't always better options for type ascription). That almost brought me to +0 on this, but I agree that if we were to do this, we should do it consistently for other types, and that just gets a bit weird. I feel as though people have good reason to expect errors in these cases, and it just feels a bit loose to allow this.

Given that, I'm just not sure the motivation here is strong enough. It seems OK to me to say that it's a breaking change to move from a struct to something else — for now, at least. Maybe later we could ensure we have better ways to ascribe the type in all cases and then move toward disallowing the { … } when the fields are private.

[Jules-Bertholet]

To get some data about how widespread the problematic Struct { .. } patterns in question are, I wrote a lint to detect them, which should be Crater-able: rust-lang/rust#159356. It found several instances inside rustc itself, e.g. https://github.com/search?q=repo%3Arust-lang%2Frust%20%22ErrorGuaranteed%20%7B%20..%20%7D%22&type=code.

[scottmcm]

Similar to what TC said above, I'm torn on this. I agree that library evolution is important, but also this is just really icky in the implications. We'd never even consider allowing x @ ::core::primitives::u32 { .. } on its own, for example.

So to me it all hinges on the reasonableness of doing this in downstream code in the first place. Is this actually useful? For anything other than meme posts on URLO?

If there's no reason to write it and we lint it on all editions, I might just add this to the "well yes technically it can be source breaking but we deem it fine", the same as we do with "well yes, technically any inherent method addition can be source breaking but we deem it fine anyway" and various other such things.

Or maybe we need a new feature -- type ascriptions in patterns anyone? -- and this is evidence of that, rather than a "we should add something icky that we don't want anyone to use".

[tmandry]

I think to the extent this is a real problem we should provide a solution. I'm curious to see the crater results to quantify that problem, particularly the cases where the struct is defined in another crate.

As for the solution, I agree with @scottmcm that we might be able to get away with linting on the pattern and reducing any potential breakage such that it's "morally okay" to do without a semver bump. That's assuming we can avoid linting on any uses we deem legitimate.

I'm open to this RFC as the solution if the lint doesn't seem so good.