Consider the following code where val is a mutable reference.
#[derive(Debug, Clone, Copy)]
enum Value {
Variant(Variant)
}
#[derive(Debug, Clone, Copy)]
struct Variant {
x: i32
}
let val = &mut Value::Variant(Variant { x: 42 });
The following works fine.
match val {
Value::Variant(v) => v.x = 777,
}
But adding mut causes v to be copied with no error or lint 
.
match val {
Value::Variant(mut v) => v.x = 123,
}
Match statements are pretty weird. This is a leftover from before match ergonomics.
Possibly the dead_code lint could be improved to pick up that v is not used after being copied.
I'd file a ticket about this, stating that the relevant lint should trigger in this case.
I'm trying to understand why the compiler would try to move the contents of the variant, and it just makes no sense to me. Shouldn't this be a bug in pattern matching rather than the linter?
No, it's not a bug. I doubt it would be designed this way today, but for historical reasons this is how it works and it can't be changed now.
FWIW, this behavior is documented in the Reference (emphasis mine):
If a binding pattern does not explicitly have
ref,ref mut, ormut, then it uses the default binding mode to determine how the variable is bound. The default binding mode starts in "move" mode which uses move semantics. When matching a pattern, the compiler starts from the outside of the pattern and works inwards. Each time a reference is matched using a non-reference pattern, it will automatically dereference the value and update the default binding mode. References will set the default binding mode toref. Mutable references will set the mode toref mutunless the mode is alreadyrefin which case it remainsref. If the automatically dereferenced value is still a reference, it is dereferenced and this process repeats.
So there's four types of binding patterns:
name: an immutable binding with the default binding mode, which resolves to either a move binding or reference binding using match ergonomics.mut name: a mutable move binding.ref name: an immutable reference binding.ref mut name: a mutable reference binding.It's definitely pretty odd that mut name always resolves to a move binding instead of using the default binding mode.
Not even in upcoming Rust revision? I think it's the first place where I see Rust construct which is not just unexpected, but dangerously unexpected.
Would be interesting to try to create Rust version of fractal of bad design. List of things which are strangely designed but can not be fixed without breaking compatibility (like Ranges which are iterators instead of IntoIter or confusing From/Into pair of traits), mostly, but things which can be fixed probably should go there, too.
There's danger than someone may use such list to say that rust is as bad as PHP, but, frankly, if someone who claims that language where "123" == "0123" is as safe as one where mut in match makes a copy… are we really sure we want to see such guy (or gal) as Rust user?
Well, to be frank, I think that match ergonomics is at fault here. By itself, the following set of syntactical rules would be (and was) consistent:
The part that makes this inconsistent is the whole dance around default modes which change state based on non-local information (i.e., the type of the discriminant as opposed to the structure of the binding).
I consider it a bug and am in good company. If there's to be a lint, I hope it's a "it used to work this way but we're breaking it to work the correct way" level of lint.
Certainly the behavior could safely be changed over an edition, couldn’t it? Migration lints should be trivial, just desugar the match ergonomics, e.g. turn something like the Value::Variant(mut v) pattern above into &mut Value::Variant(mut v) to keep the old behavior.
Edit: Oh, I see, editions were already mentioned in the discussion that @quinedot linked to, anyway.
Edit2: Hmm, what should mut v even mean here anyway? A the mutable reference itself being mutable would be weird, too, since there’s no way to do this without match ergonomics either. So all that’s necessary is to rule out mut v in a match ergonomics pattern. Perhaps also rule out ref mut v, too, since that’s redundant; maybe rule out all the modifiers because you ought to rather not use match ergonomics in such complicated cases? Either way, in order to just rule out stuff, an ordinary lint is probably sufficient, especially if the goal is merely to avoid footguns. Can become deny-by-default lint eventually, those aren’t really a breaking change anyway… or maybe change the deny-by-default-ness over an edition if that feels more appropriate (I don’t know what any precedent on such things is).
Yes, when viewed in isolation it sounds logical. But recall absurd std::optional<T&> saga. Where all the guys who actually played with it in real code had one opinion and people who are writing papers, not code, had another.
Rules may sound logical on paper — but when someone who don't remember all the minute details of reference guide writes code… landmines are landmines. And that one is pretty big one.
i.e., the type of the discriminant as opposed to the structure of the binding
Yeah, but the whole thing confuses even pretty knowledgeable guys as we saw. If this can not be fixed with editions (and this kind of things sounds precisely like issue editions are supposed to fix) then at least robust clippy lints would be nice.
That's something which some kind of survey may help to resolve. Maybe better rules couldn't be even invented (which would be sad), bit @alice was talking that it wouldn't be designed thus way today, perhaps she have concrete offers?
We have three years, should be about correct amount of time to design that. And no, I'm not joking: since we are talking about ergonomics it's not enough to just develop some kind of internally consistent rules, they have to be usable in real code so there are lots of work collecting examples and looking on how people use match modifiers.
It was perfectly clear and easy to write actual code. I, as someone who has written a non-trivial amount of Rust code for various purposes, find the old set of rules easier to use. I'm not the guy who writes papers about theoretical features without having used the language.
After pondering the text of the reference, I think I understand why mut v binding works that way. We have a non-reference pattern corresponding to Variant::Variant, so the val matched expression is accepted, it is dereferenced within the Variant::Variant pattern and the default binding mode is set to ref mut. This propagates through struct pattern Variant, and the mut v binding is matched against the v field of the dereferenced *val. Since the default binding mode is used only for simple identifier bindings (e.g. v), the binding mut v is treated as-is on the dereferenced value, and so it tries to move the v field inside *val.
Frankly, this makes no sense. In my mental model the match ergonomics work by turning identifier bindings inside of non-reference patterns into ref or ref mut bindings. So mut v should either not compile, or act something like mut ref mut v, if it makes any sense. I.e. it should be a mutable &mut T binding (like let mut x: &mut T). Similarly for ref v and ref mut v bindings: they don't make much sense inside of match-ergonomics pattern, and should probably fail to compile, or have &&T/../&mut &mut T types.
I consider the reference underspecified in that section. It doesn't really explain what default binding mode means, or what should happen with explicit bindings in the presence of default binding mode. The behaviour of mixed patterns should be explicitly specified, and not left as a byproduct edge case in the pattern-matching algorithm.
I also don't really believe that someone thought long and hard about those edge cases and said "yeah, that's totally reasonable and expected behaviour". The entire point of match ergonomics was to effectively deprecate ref (mut) patterns in the language, due to their confusing for many users semantics. Mixing old and new style patterns is more in a "why would you even do it" category, but since ref and ref mut kinda sorta behave as expected, this was overlooked.
If we introduce this then &mut mut ref mut x patterns will be a thing, technically 
.... for re-bottowing a mutable reference and marking the variable holding the new re-borrowed reference itself mutable, too. I mean could be useful 
, but I suppose in this case, we'd also have mut ref x patterns (bind by immutable reference, but the variable holding that immutable reference is mutable) which is actually maybe somewhat problematic because it's easily confused with ref mut x. Especially since (IMHO) having it the other way around feels even slightly more appropriate (ignoring that it'd be, of course, a breaking change now), always keeping the modifier for the variable right next to the variable, and having “mut ref” stand for “bind by mutable reference”.
Be careful relying on the reference to be accurate here. Probably it's just based on the RFC [1], but the implementation deviated from the RFC.
For example, reference patterns do sometimes change the binding mode (or are otherwise magic).
// Tries to move the string in contrast with
// ```
// match *&Some(&String::new()) {
// Some(&ref x) => println!("{x}"),
// ```
match &Some(&String::new()) {
Some(&x) => println!("{x}"),
_ => {},
}
I haven't taken the time to try to suss out the specifics of the current behavior, and I've never seen an official specification; as far as I know, there is none. Like many parts of the language [2], it's not properly documented or specified.
Rust really needs a spec quite badly, IMO.
I came up with some pseudocode a while back, which I'm fairly sure is accurate. This particular behavior comes from &[mut] reference patterns: they always reset the inner default binding mode to move. It's not explained very well, probably because the RFC only talked about the default binding mode behavior for non-reference patterns, an omission which I noticed at the time.
After reading through some comments here I have the feeling that a lot of the confusion around this pattern matching comes from distinguishing what is part of the "pattern" which is used for the match and what is part of the variable declaration, which is also written inside said pattern.
Maybe an option would be to be more explicit and just enforce the '@' annotation, to separate both parts.
Some examples (on mobile, possibly badly formatted):
match &mut 5u8 {
a@ _ => {...} \\ would match anything, a will have type &mut u8
&mut (a@_) => {...} \\ would match the u8 behind the reference, resulting in a copy (only works since u8 is copy). a will have type u8
mut a @ _ => {...} \\ same as first case but the variable a is mut
}
Hope this clarifies the idea.
Building on top of this maybe some "trivial" cases (whichever that might be) could be made more ergonomic.
This looks super complicated.
I think it's fair to say that very few people understand these rules. Which to me indicates they are overcomplicated. It means programmers have to write code by trial and error, which is an unsatisfying way to work.
I wish we could do away with match ergonomics or otherwise simplify the rules in a future edition.
Nothing says "ergonomic" like several people arguing over multiple prolonged forum threads over what the exact behavior is or ought to be 