Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Match ergonomics means bindings have different types in patterns and match arm; cannot deref references in pattern #64586

Open
varkor opened this issue Sep 18, 2019 · 43 comments
Labels
A-patterns Relating to patterns and pattern matching C-enhancement Category: An issue proposing an enhancement or a PR with one. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@varkor
Copy link
Member

varkor commented Sep 18, 2019

Consider the following:

struct S(u32);

fn foo(x: &S) {
    let _: &u32 = match x {
        S(y) => y,
        _ => &0,
    };
}

Here, the variable binding y has type &u32 outside the pattern. However, inside the pattern it has type u32. The fact that these types do not align is confusing, as it means you can't dereference y inside the pattern like this:

fn foo(x: &S) {
    let _: u32 = match x {
        S(&y) => y,
        _ => 0,
    };
}

and instead have to dereference it outside the pattern:

fn foo(x: &S) {
    let _: u32 = match x {
        S(y) => *y,
        _ => 0,
    };
}

Is there any reason this was chosen to be the case, or is required to be the case? As it stands, this behaviour is very counter-intuitive.

@varkor varkor transferred this issue from rust-lang/rfcs Sep 18, 2019
@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Sep 18, 2019
@estebank
Copy link
Contributor

I thought that this was the case where you'd want to use ref.

@varkor
Copy link
Member Author

varkor commented Sep 18, 2019

@estebank: could you clarify?

@estebank
Copy link
Contributor

@varkor my bad, I misunderstood what you were talking about. Using S(ref y) makes y be u32. I believe this is only a problem because u32 is Copy, but if it weren't then y would be &u32 inside the match arm block as well.

@CryZe
Copy link
Contributor

CryZe commented Sep 19, 2019

However, inside the pattern it has type u32.

Yes, that's the whole point of default match bindings though. The fact that S(...) is matched against a reference means that every binding inside also becomes a reference. The only annoying part is that there's no way to force a move instead of getting a reference in that case.

I think we should absolutely have either S(&y) or S(move y) as otherwise you sometimes still have to resort to the old style of matching, which is just confusing for everyone.

@Emerentius
Copy link
Contributor

It's a somewhat common trick to use let () = value to provoke a type error that tells you what type value really is. With match ergonomics this will only tell you the type modulo references.

let () = &&3u8;
    ^^ expected u8, found ()

This confused me for quite a while.

@varkor
Copy link
Member Author

varkor commented Mar 17, 2020

@Centril pointed out that currently, y acts differently to mut y in patterns, with respect to references (which is absurd). Under my proposed behaviour, the following:

fn foo(x: &S) {
    let _: u32 = match x {
        S(mut y) => y,
        _ => 0,
    };
}

would not type-check, because mut y would have type &u32, but unfortunately it does. I'm not sure how much can be done in light of this.

@RalfJung
Copy link
Member

Woah, mut "resets" the binding mode? That is quite absurd indeed.

@Boscop
Copy link

Boscop commented Apr 13, 2020

@varkor

I'm not sure how much can be done in light of this.

Do you mean that this is preventing us from allowing to move y out with the &y syntax? I don't think so, because we already support moving y combined with making y mut like this:

arr.iter().map(|&(mut y)| .. )

I really hope we can enable a way to move out some pattern constituents explicitly in the presence of match-default-bindings, and I really prefer &x over move x for reasons explained here (consistency & orthogonality): #42640 (comment)

Me and many other people already expected &x to work in this way: #42640 (comment)

I am absolutely floored by the fact that default binding modes don't already work like @Boscop expects. That (apparently incorrect) understanding was part of why I came around to liking them.

(That comment was referring to what I wrote here: #50008)

@varkor
Copy link
Member Author

varkor commented Apr 13, 2020

Do you mean that this is preventing us from allowing to move y out with the &y syntax?

We could probably permit some examples that are currently forbidden, but we would not be able to make it behave consistently (e.g. with regards to mut) without a breaking change, so this would be ad hoc at best. It's debatable whether this would be an improvement, and also whether it would be worth considering changing the behaviour in a new edition.

@Centril
Copy link
Contributor

Centril commented Apr 13, 2020

@varkor Before considering using the edition mechanism, I think it would be prudent to crater run a change and see if anyone is relying on the mut x resetting.

@jonas-schievink jonas-schievink added the A-patterns Relating to patterns and pattern matching label Jun 4, 2020
@phaylon
Copy link
Contributor

phaylon commented Jun 27, 2021

Since there's an edition coming up soon, I'm wondering if the binding reset issue should be given consideration again? There was a reddit post of someone running into the issue and it causing confusion.

In the case that there's something blocking that, would it be a good idea to have at least some additional diagnostics? It's hard to make any sense of what is happening for newcomers currently.

@varkor
Copy link
Member Author

varkor commented Jul 11, 2021

I was planning to prototype changes to match ergonomics to address this issue, but I haven't found time to do so yet. I think that would be a good first step in determining the viability of the change.

@RalfJung
Copy link
Member

RalfJung commented Dec 27, 2022

FWIW I hit this fairly regularly. It is quite annoying to have to write *y everywhere, in particular when y ends up with a type like &i32 where it is rather silly to capture y by-reference to begin with. Basically one has to pick between two different kinds of suboptimal code:

struct S(u32, Vec<i32>);

fn foo(x: &S) {
    match x {
        S(y, v) => {
            let y = *y;
            // match arm goes here
        }
    }
}

or

struct S(u32);

fn foo(x: &S) {
    let _: &u32 = match *x {
        S(y, ref v) => {
            // match arm goes here
        }
    }
}

I end up often preferring the latter, even if that means adding ref in some places -- basically in these cases I think the match ergonomic rules are less ergonomic than old-school matches.

I would like to be able to just write

struct S(u32, Vec<i32>);

fn foo(x: &S) {
    match x {
        S(&y, v) => {
            // match arm goes here
        }
    }
}

but sadly that is not possible.

@RalfJung RalfJung changed the title Match ergonomics means bindings have different types inside and outside of patterns Match ergonomics means bindings have different types in patterns and match arm; cannot deref references in pattern Dec 27, 2022
@Boscop
Copy link

Boscop commented Dec 28, 2022

I agree, I run into this all the time and it's always frustrating me that it doesn't work this way, which would be more useful and it would be more in line with expectations.

Is there still a chance that this can be fixed in the next edition?

@Nadrieril
Copy link
Member

Could we try to get this for the 2024 edition? I seem to remember someone ran a crater run to see if changing the "mut resets binding mode" behavior was feasible, anyone else remember that?

@traviscross
Copy link
Contributor

@rustbot labels +I-lang-nominated

Based on the recent discussion about this, let's nominate to discuss it.

@rustbot rustbot added the I-lang-nominated Nominated for discussion during a lang team meeting. label Dec 1, 2023
@Nadrieril
Copy link
Member

Nadrieril commented Dec 1, 2023

EDIT: see the better proposal two posts below this one

Alright, here's the status as far as I can tell. Two things come up regularly (1 2 3 4 5 6 etc) with regards to match ergonomics:

  1. mut resets the default binding mode, which is confusing.
    match &Some(0) {
        Some(x) => {} // x: &i32 here
        Some(mut x) => {} // x: i32 here surprisingly
    }
  2. & cannot be used in patterns where one might expect.
    match &Some(0) {
        Some(x) => {} // x: &i32 here
        Some(&x) => {} // Disallowed despite the type above
    }

The two issues are somewhat independent but I propose we fix them both. More precisely I propose:

  • (breaking) The pattern mut x now behaves exactly like the pattern x except that the binding is made mutable.
  • (non-breaking) The pattern &<pat> is allowed whenever a binding in its place would acquire a reference type, and strips a layer of references in the expected manner.

The first part is a breaking change; example of code that would be broken:

fn foo(opt: &Option<u32>) -> u32 {
    match opt {
        Some(mut x) => {
            x += 1;
            x
        }
        None => 0
    }
}

Under this proposal, it would error with "x has type &u32". The reason I propose the other change too is that it would allow for an easy fix of the above: simply change the pattern to Some(&(mut x)). The questions to investigate are: is there always an easy fix like that, and how much churn would that cause.

I note that there is a different RFC (here) that would offer instead the option to write Some(mut move x) to solve this. That RFC proposes a keyword that adds to the "default binding mode" algorithm. There has been a fair amount (1 2 3 4) of discussion on the topic of these two proposals.

Of note is the question of what the behavior of &<pat> should be with multiple references, see here and here. This is an open design question. Specifically, the question is what the type of x should be in:

match &Some(&Some(0)) {
    Some(Some(&x)) => {} // x: &i32 or i32?
    _ => {}
}

I have a personal preference for &<pat> over move x for a reason of mental model. The mental model I believe users tend to develop is: "matching a &(A, B) with (<pat1>, <pat2>) slips the reference into the tuple, making it morally (&A, &B), after which matching continues recursively". Under that model, writing &<pat> is natural, and I think we should make it do the expected thing (once we figure out what that is). Under that model, ref x can be descibed as: it collapses all the layers of references into one, possibly taking an extra reference if there were none.

@Nadrieril
Copy link
Member

Nadrieril commented Dec 1, 2023

The more I look into it the more I find deep discussion on what is or isn't ergonomic and subtle behavior with nested references. I just discovered that &<pat> (with slightly different semantics) was allowed in the initial match ergonomics implementation (see e.g. here) then removed. I hope we manage to make progress this time around. Consider the proposal above in need of refinement; hopefully you get the general idea.

@Nadrieril
Copy link
Member

Nadrieril commented Dec 4, 2023

Alright, I have a precise proposal now (will edit this post if I discover new edge cases).

Motivation

Two things come up regularly (1 2 3 4 5 6 etc) with regards to match ergonomics:

  1. mut resets the default binding mode, which is confusing.
    match &Some(0) {
        Some(x) => {} // x: &i32 here
        Some(mut x) => {} // x: i32 here surprisingly
    }
  2. & cannot be used in patterns where one might expect.
    match &Some(0) {
        Some(x) => {} // x: &i32 here
        Some(&y) => {} // Disallowed despite the type of `x` above
    }
  3. (I have not seen this come up but) Beyond 1., one cannot use mut uniformly to declare bindings mutable.
    match Some(0) {
        Some(mut x) => {} // mutable `x: i32`
        Some(ref x) => {} // immutable `x: &i32`
        // no way to get a mutable `x: &i32`
    }

I propose to fix all three at once, particularly since together they make it possible to change 1 in such a way that rustfix can fix the backwards-incompatible cases. 1 would be changed over the 2024 edition.

Current behavior

Here is the current behavior of match ergonomics in terms of the mental model I like:

  • Matching on a place of type &(A, B) with (<pat1>, <pat2>) slips the reference into the tuple, making it morally (&A, &B), after which matching continues recursively. Same for other non-reference, non-binding patterns.
  • Moreover we keep at most one layer of such inherited references.
  • A &<pat> pattern is only allowed if the place has actual type &T (i.e. ignoring inherited references). It dereferences the layers of references at that place, namely: the inherited reference if any, plus the actual reference since the place has type &T. It then continues matching <pat> on the dereferenced place (of type T).
  • A x binding gets the type computed from the actual type of the place plus the inherited reference if there is one.
  • A ref x binding adds one layer of reference if there was no inherited reference, otherwise does nothing.
  • As such, &ref x isn't identical to x; compared to x, it discards the inherited reference. See examples below.
  • A mut x binding:
    • Dereferences the inherited reference (if any) (that's the bit that's confusing in this model);
    • Matches the underlying place with a normal binding x;
    • Declares the binding x mutable.
  • All of this works identically for &mut/ref mut. Interactions between inherited &mut and & collapse to &.
  • Of note: a &<pat> pattern is not allowed on a place of type &mut T; mutabilities must match here.

Some illustrative examples (and more: playground):

// No way to get a `x: &&T` here since there isn't a `&T` in memory it could point to.
// Hence why we keep only one layer of inherited references.
if let Some(Some(x)) = &Some(&Some(0)) {
    let _: &u32 = x;
}
if let &Some(Some(x)) = &Some(&Some(0)) {
    let _: &u32 = x;
}
if let Some(&Some(x)) = &Some(&Some(0)) {
    let _: u32 = x;
}
if let &Some(&Some(x)) = &Some(&Some(0)) {
    let _: u32 = x;
}

// We have one layer of inherited ref, and an underlying place of type `&&T`.
if let Some(x) = &&Some(&&0) {
    let _: &&&u32 = x;
}
// The `&` derefs the inherited ref plus one actual ref.
if let Some(&x) = &&Some(&&0) {
    let _: &u32 = x;
}

// Compare `x` and `&ref x`.
if let Some(x) = &Some(&0) {
    let _: &&u32 = x;
}
// Here the `&` derefs both layers, so the `ref` can only recover one.
if let Some(&ref x) = &Some(&0) {
    let _: &u32 = x;
}

// `mut x` dereferences the inherited reference
if let Some(mut x) = &Some(0) {
    let _: u32 = x;
}
if let Some(mut x) = &Some(&0) {
    let _: &u32 = x;
}

// `&mut` behaves just like `&`
if let Some(Some(x)) = &mut Some(&mut Some(0)) {
    let _: &mut u32 = x;
}
// Mixing `&` and `&mut` inherited references gives `&`.
if let Some(Some(x)) = &mut Some(&Some(0)) {
    let _: &u32 = x;
}

Proposal

&<pat> everywhere (non-breaking)

I propose the following extension: we remove the typecheck requirement on &<pat>/&mut <pat>; instead we allow it whenever a corresponding binding would acquire a compatible reference type. The rule becomes:

  • A &<pat> pattern dereferences the layers of references at that place, namely: the inherited reference if any, and the actual reference if the place has type &T/&mut T. It then continues matching <pat> on the dereferenced place. It is disallowed if there was nothing to dereference.

This would allow:

if let Some(Some(&x)) = &Some(&Some(0)) {
    let _: u32 = x;
}
if let Some(Some(&mut x)) = &mut Some(&mut Some(0)) {
    let _: u32 = x;
}
if let &Some(x) = &mut Some(0) {
    let _: &u32 = x;
}

Note: this only affects desugaring into non-match-ergonomic patterns. The desugaring algorithm likely doesn't need to change much, without having looked into it I'd guess a & pattern on a non-reference type simply resets the binding mode to "move", e.g.:

// input
if let Some(Some(&x)) = &Some(&Some(0)) {
    let _: u32 = x;
}
// desugared
if let &Some(&Some(x)) = &Some(&Some(0)) {
    let _: u32 = x;
}

The fact that & "eats" all inherited references has been found confusing (1). This is however the current behavior (and arguably the only sensible behavior); this proposal only extends it to more cases. Such confusion can likely be alleviated with good diagnostics.

ref (mut x) (or general ref <pat>) (non-breaking)

I propose we allow ref (mut x)/ref mut (mut x) as a ref x/ref mut binding that is allowed to be mutable. (Or mut (ref x)/mut (ref mut x) maybe?). This would allow:

if let Some(ref (mut x)) = Some(0) {
    if *x == 0 {
        x = &1; // x: &u32
    }
}
let y = 1;
if let Some(ref mut (mut x)) = Some(0) {
    if *x == 0 {
        x = &mut y; // x: &mut u32
    }
}

There's a more uniform alternative that I'm not really proposing but thought I'd mention: we could allow ref/ref mut not just on bindings but with arbitrary patterns. The rule would become:

  • A ref <pat> adds one layer of reference if there was none, or keeps the existing one, then continues matching with <pat>.

This would allow (on top of the above):

let mut opt = Some(0);
if let ref mut Some(x) = opt {
    *x = 1;
}

Non-dereferencing mut x (breaking)

I propose the following amendment: mut x now keeps the inherited reference. The rule becomes:

  • A mut x binding matches matches the place with a normal binding x and declares it mutable.

This would allow:

if let Some(mut x) = &Some(0) {
    let _: &u32 = x;
}

This would make the following no longer compile:

fn foo(opt: &Option<u32>) -> u32 {
    match opt {
        Some(mut x) => {
            x += 1;
            x
        }
        None => 0
    }
}

Machine-applicable fix

Now, if we take all three proposals, then I claim that the following transformation preserves previous behavior:

  • mut x without inherited reference works like before.
  • mut x with inherited reference on a place not of type &T is changed to &(mut x).
// old:
if let Some(mut x) = &Some(0) {
    let _: u32 = x;
}
// new:
if let Some(&(mut x)) = &Some(0) {
    let _: u32 = x;
}
  • mut x with inherited reference on a place of type &T is changed to &(ref (mut x)).
// old:
if let Some(mut x) = &Some(&0) {
    let _: &u32 = x;
}
// new:
if let Some(&(ref (mut x)) = &Some(&0) {
    let _: &u32 = x;
}

Proposed timeline

  1. Implement the two non-breaking changes.
  2. Warn when using a dereferencing mut x. Propose the fix above instead. The resulting code works identically on both editions.
  3. Over the edition boundary, change the behavior of mut x as proposed above.
  4. Code that hasn't applied the fix will fail to typecheck. The typecheck error can suggest the fix as well.

@RalfJung
Copy link
Member

RalfJung commented Dec 4, 2023

Thanks for the summary and for reviving this issue. :)

(I have not seen this come up but) Beyond 1., one cannot use mut uniformly to declare bindings mutable.

I don't understand the problem here. You can use ref mut to get an &mut i32. But a mutable &i32 makes much less sense; mutating that would not mutate anything inside the subject of the match. Arguably, allowing that would be a footgun, since people might think their change would somehow be reflected back in the match subject.

But if I understand correctly, having this as a pattern is required to have an automatic transition from the current patterns to the new ones where & and mut are fixed?

So IOW this footgun is already present with match ergonomics... before match ergonomics, if I had e.g. Some(mut x) and mutated x, it would always change the match subject. But now if the subject has type &Option<i32>, we do accept that pattern and x has type i32 and it is mutable but mutating it only mutates a local copy of the value -- right? Do we want to support such patterns or should we rather deprecate them and phase them out?

Basically I am suggesting, maybe this code should get a future-compat warningand eventually a breaking change, if we can get away with it:

if let Some(mut x) = &Some(0) {
    let _: u32 = x;
}

And the (potentially manual) fix is

if let Some(&x) = &Some(0) {
    let mut x = x;
    let _: u32 = x;
}

Do we know how common this pattern is out there?

mut x with inherited reference on a place of type &T is changed to &(ref (mut x)).

Wow, this is an even more strange case. So we dereference and then take a reference again and then have a local mutable copy of that...? Again I wonder if that's really something we want to allow in patterns; mutating that x will not change the match subject. I'd much rather see this:

// old:
if let Some(mut x) = &Some(&0) {
    let _: &u32 = x;
}
// new:
if let Some(&x) = &Some(&0) {
    let mut x = &x;
    let _: &u32 = x;
}

This also better reflects the lifetime x will have (if I understand correctly).

@RalfJung
Copy link
Member

RalfJung commented Dec 4, 2023

The fact that & "eats" all inherited references has been found confusing (#42640 (comment)). This is however the current behavior (and arguably the only sensible behavior); this proposal only extends it to more cases. Such confusion can likely be alleviated with good diagnostics.

Yes this does seem confusing on first sight. Could you give examples showing in which sense this is already the current behavior and why it is the only sensible behavior?

@Nadrieril
Copy link
Member

Nadrieril commented Dec 4, 2023

before match ergonomics, if I had e.g. Some(mut x) and mutated x, it would always change the match subject

I don't know what you mean by "before match ergonomics", but that's not current behavior. Some(mut x) moves out of the match scrutinee into a new binding x, which is incidentally declared mutable (playground):

let y = Some(0);
if let Some(mut x) = y {
    x = 2;
}
assert_eq!(y, Some(0));

If you want to change the scrutinee place you need Some(ref mut x). I agree that's a footgun, but it's a uniform footgun across rust.

A different way of saying this: mut x is always exactly about declaring mutability of the binding x. It otherwise works exactly like x. If it wasn't for the weird resetting behavior, the following would always be equivalent today:

  • Matching with ...(mut x)...
  • Matching with ...(x)... then doing let mut x = x;.

And the weird resetting behavior does not change the fact that x is a new place, it's not a place within the match scrutinee, so declaring it mutable cannot be used to affect the match scrutinee. I hope that makes sense.

maybe this code should get a future-compat warning

Idk, I really like the uniformity of "any binding can be declared mutable". Maybe clippy can lint against your case, but I've definitely had cases where it was sensible to declare a mutable x: &T. E.g.

if let Some(mut pat) = &arm.pat {
    while let AtBinding { subpattern } = pat.kind {
        pat = subpattern;
    }
    process(pat)
}

And well, I see the point you're making, my playground example above is likely surprising to newcomers. But that's a different issue which is orthogonal to this one, if you'd like to discuss this further I'd prefer to do it in a separate issue.

This also better reflects the lifetime x will have (if I understand correctly).

That's incorrect. ref borrows from the original place, not from a copy. E.g. today &(ref x) and x have the same lifetime (whenever they have the same type. if they don't you can always add some & before to make them match). My looping example above illustrates that: if arm: &'tcx Arm then pat: &'tcx Pat.

Yes this does seem confusing on first sight. Could you give examples showing in which sense this is already the current behavior and why it is the only sensible behavior?

To be precise, the thing that's the only sensible behavior is the fact that we collapse all layers of inherited references to one. This is necessary, e.g.:

if let Some(Some(x)) = &Some(&Some(0)) {
    let _: &u32 = x;
}

Here there's no &T in memory that a &&T could point to. To illustrate that & eats "all the references":

if let Some(&x) = &&&&&&Some(&&&&0) {
    // The first 6 collapsed into one, then got eaten.
    // After that, `&` ate exactly the outer ref inside the `Some`, leaving 3 refs.
    let _: &&&u32 = x;
}

Having said that, there's only three possibilities for the behavior of &<pat>:

  1. eat the inherited reference if any, and the actual reference
  2. eat the inherited reference if any, and the actual reference if any
  3. eat the inherited reference if any, xor the actual reference if any

1 is the current behavior; it requires an actual reference. 2 is the one I propose. 3 is backwards-incompatible since it would require &&x to copy out the u32 here:

if let Some(&&x) = &Some(&0) {
    let _: u32 = x;
}

@tgross35
Copy link
Contributor

tgross35 commented Dec 4, 2023

It may be worth thinking about how deref patterns work as part of this proposal (not strongly related but does overlap somewhat).

The current idea there is in #87121 (comment). Basically, this allows a one-time Deref per pattern item, i.e. if multiple match arms require a deref the same thing then they must be consecutive so deref is only ever called once. This won't necessarily be with the shown Deref(v) syntax, a & or a * or just automatic have been discussed more.

@Nadrieril
Copy link
Member

Ah, that's an interesting point, thanks for raising it. Assuming you choose &<pat> to work on e.g. Box<T> exactly like on &T, then this with this proposal there would be a breaking change. Under deref patterns (with or without my proposal), x: u32 here for consistency with the &T case:

if let Some(&x) = &Some(Box::new(0)) {
    /// ...
}

Under current rust this is simply a type error. Under my proposal without deref patterns however, x: Box<u32>.

If you choose something different than & then I think there wouldn't be any breaking change. But I'd have to write it down precisely, you're right there are subtle interactions. You could also decide a different behavior for &, there are choices depending on whether you want to allow automatic deref() or not etc.

Whatever the case however, it's fixable over an edition. Do you think deref patterns could be ready for edition 2024?

@RalfJung
Copy link
Member

RalfJung commented Dec 4, 2023

Some(mut x) moves out of the match scrutinee into a new binding x, which is incidentally declared mutable (playground):

Oh, you are right of course... not sure what I was thinking. 🙈

I really like the uniformity of "any binding can be declared mutable".

Yeah that is nice indeed. Though having ref (mut x) and ref mut x mean different things is rather unfortunate...

That's incorrect. ref borrows from the original place, not from a copy. E.g. today &(ref x) and x have the same lifetime (whenever they have the same type. if they don't you can always add some & before to make them match). My looping example above illustrates that: if arm: &'tcx Arm then pat: &'tcx Pat.

I was refering to your new extension of "&<pat> everywhere". That must do something particular with lifetimes, or does it not?

Maybe not; I think I missed the part where the pattern still requires that there is at least one reference being eaten, but it may be an implicit one.

eat the inherited reference if any, or the actual reference if any

Specifically you mean "and also the actual reference if any, and fail if there is not at least one being eaten"? As in, when there are implicit references and the binding has type &i32, then it eats the implicit ones and the explicit one?

My intuitive expectation would be (3). But honestly the moment there are ever multiple references being 'eaten' it is somewhat confusing, and you gave a good reason for that, so "eat as many references as you can but always at least one" (aka (2)) also makes a lot of sense and might even be easier to explain than (3).

@Nadrieril
Copy link
Member

Nadrieril commented Dec 4, 2023

I was refering to your new extension of "& everywhere". That must do something particular with lifetimes, or does it not?

I guess I was not particularly explicit about that. Since this only affects desugaring into non-match-ergonomic patterns, I'd expect the most general lifetimes that make sense. It most definitely will not be introducing extra places just to take references of them, that'd be silly.

Specifically you mean "and also the actual reference if any"

Ah yes you're correct, I'll edit.

"eat as many references as you can" also makes a lot of sense and might even be easier to explain

It is and isn't, because matching a &&&T with &x gives x: &&T :/ That's pretty important but makes it harder to explain. Regardless, the confusing (imo) behavior is when there is both an inherited reference and an actual one, aka stable behavior. The case in my proposal (inherited reference and no actual reference) we all agree on.

@tgross35
Copy link
Contributor

tgross35 commented Dec 4, 2023

If you choose something different than & then I think there wouldn't be any breaking change. But I'd have to write it down precisely, you're right there are subtle interactions. You could also decide a different behavior for &, there are choices depending on whether you want to allow automatic deref() or not etc.

This is still unresolved but last I asked, it seemed like most were in favor of working without any extra syntax. At least as long as there are no potential problems.

Whatever the case however, it's fixable over an edition. Do you think deref patterns could be ready for edition 2024?

How ready :) String deref patterns are in the compiler but not stable. I think the generic proposal could be implemented at any time, just hasn't had anybody able to pick it up.

@RalfJung
Copy link
Member

RalfJung commented Dec 4, 2023

It is and isn't, because matching a &&&T with &x gives x: &&T

Ah right. We are eating at most one "real" &. Yeah that is unfortunate...

@Nadrieril
Copy link
Member

last I asked, it seemed like most were in favor of working without any extra syntax

Hm, that would still require some interaction with &. And the most natural one would be that &T and Box<T> are matched the same way, which is what poses problems...

How ready :)

Ideal would be fully ready. If you have deref patterns for std types by 2024 and you later allow it for user types, same kind of breakage would happen. You could circumvent that by requiring DerefPure and making its implementation a breaking change but that's annoying.

Given the size of the feature I'd expect it to stay nightly only for a while anyway, I don't expect full deref patterns could be ready for the edition. Would be unfortunate to wait for 2027 though. Maybe that can motivate someone to pick up the work? :D

@traviscross
Copy link
Contributor

traviscross commented Dec 21, 2023

@rustbot labels -I-lang-nominated

The proposal above was discussed in the 2023-12-13 T-lang design meeting. The consensus was general agreement about the goals. We probably want & patterns everywhere, and we probably want to fix the problem of mut resetting the binding mode; i.e. we want to make it such that adding mut doesn't change the type of the binding.

@Nadrieril expressed that with agreement on those goals, he can invest further time in writing code, performing crater runs, etc.

On the basis of what we learn from that additional work, we hope to build the confidence needed in the details to stabilize some solution.

As we had reduced attendance by the time the consensus formed, we decided to confirm this on Zulip, and we also rechecked this via meeting consensus on the 2023-12-20 T-lang triage call.1

Everyone involved also wanted to express our appreciation of @Nadrieril for working to push this forward.

Please renominate this for T-lang when there are further questions that the team can answer or further evidence or analysis for us to consider.

Footnotes

  1. It's still lacking affirmative +1s from all members, but the consensus has a quorum, and no member has yet raised concerns about support for these as the goals (with the details yet to be finalized).

@rustbot rustbot removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Dec 21, 2023
@Nadrieril
Copy link
Member

Since there was general agreement that we wanted to fix this, can I add this (or #105647 more likely) to the 2024 edition project board?

@traviscross
Copy link
Contributor

@Nadrieril: Thanks, i've added it to the board. Next time we discuss the board, we'll prioritize it.

@Nadrieril
Copy link
Member

Nadrieril commented Jan 16, 2024

Thank you! I'd be happy to attend if my input would be helpful. Also I'm expecting the final proposal to require fewer moving parts than the one I wrote earlier here; I'm experimenting atm.

@Nadrieril
Copy link
Member

Actually, the thing that requires an edition is fixing #105647, not this one. The two issues may or may not be independent in the end

@Nadrieril
Copy link
Member

Hi! I decided I don't have the bandwidth to handle this in time for the edition. If anyone wants to pick up the edition-related work, it's actually much simpler than I thought when I wrote up that proposal. I wrote a short summary over on #105647 (comment).

@Jules-Bertholet
Copy link
Contributor

I've made some progress on implementation here: #122978

@rpjohnst
Copy link
Contributor

rpjohnst commented Mar 26, 2024

(From @Nadrieril on the tracking issue:)

Noting here that I no longer think we need the "&<pat> everywhere" part of the proposal. This makes the proposed fix less local but saves us in design and possible conflicts with deref patterns.

To me "&<pat> everywhere" is the most important part of all this. It was the original papercut we found with match ergonomics and it's still the one I hit the most. It would be disappointing for it to get swept into a larger initiative only to be dropped for, yet again, release management reasons.

I mention this because I also slightly prefer the (backwards incompatible) "eat-one-layer" variant of "&<pat> everywhere." My mental model here is that the & should eat the implicit reference from the current binding mode, and drop you back in the "fully explicit" mode to handle the rest. After all this is the entire reason for using & in this context!

The fact that eat-one-layer is more forward-compatible with deref patterns, essentially because it matches this mental model, makes it worth considering here, I think.

@Jules-Bertholet
Copy link
Contributor

Jules-Bertholet commented Mar 27, 2024

To me "&<pat> everywhere" is the most important part of all this. It was the original papercut we found with match ergonomics and it's still the one I hit the most. It would be disappointing for it to get swept into a larger initiative only to be dropped for, yet again, release management reasons.

I mention this because I also slightly prefer the (backwards incompatible) "eat-one-layer" variant of "&<pat> everywhere." My mental model here is that the & should eat the implicit reference from the current binding mode, and drop you back in the "fully explicit" mode to handle the rest. After all this is the entire reason for using & in this context!

FWIW I agree with this, I hope eat-one-layer is what we can get eventually. But there are challenges: it's a silent change in behavior, which is generally seen as undesirable even across edition boundaries, and also it requires more complex migration lint logic compared to the eat-two-layers proposal.

Also, there are situations where eat-one-layer is not obviously more intuitive:

fn main() {
    if let Some(Some(x)) = &Some(&Some(0)) {
        let _: &u8 = x; // under all proposals
    }

    if let Some(&Some(x)) = &Some(&Some(0)) {
        let _: u8 = x; // under today and eat-two-layers
        let _: &u8 = x; // under eat one layer - the added & ends up being a noop
    }
}

@rpjohnst
Copy link
Contributor

One thought that might simplify both the migration and that unintuitive case: it does not seem very useful to use a & pattern to exit ref mode, then immediately fall back into ref mode. Perhaps this is never useful?

Could old editions somehow accept both the eat-both-layers form and the eat-one-layer form, then lint on the former? For example:

if let Some(&x) = &Some(&0) {
    let _: i32 = x; // existing behavior; eat-both-layers; now an edition lint
    let _: &i32 = x; // new edition behavior; eat-one-layer
}
if let Some(&&x) = &Some(&0) {
    let _: i32 = x; // currently errors; eat-one-layer; now accepted in all editions
}

This is still a change in behavior, but there is at least the usual path of "fix lints to get something that works the same way in both editions." Applying this to the unintuitive example, we can lint (or even error) on the no-op of & + falling back into ref mode:

if let Some(&Some(x)) = &Some(&Some(0)) {
    let _: i32 = x; // existing behavior; eat-both-layers; now an edition lint
    let _: &i32 = x; // new edition behavior; eat-one-layer; still a lint (or error)
}
if let Some(&&Some(x)) = &Some(&Some(0)) {
    let _: i32 = x; // currently errors; eat-one-layer; now accepted in all editions 
}

@Nadrieril
Copy link
Member

To me "&<pat> everywhere" is the most important part of all this.

Fair, but beware that this is also the most controversial part of it. There's a risk of wasting a lot of energy on this when the rest is less controversial and shippable.

@Jules-Bertholet
Copy link
Contributor

Jules-Bertholet commented Mar 30, 2024

I would also note: the eat-two-layers proposal is backward compatible and strictly better than what we have today, and we can always do the transition to eat-one-layer in 2027.

@WaffleLapkin
Copy link
Member

I would strongly advice going with eat-one-layer in 2024 edition. IMO eating one layer is a lot more intuitive. And generally the earlier you do a change, the better.

it's a silent change in behavior, which is generally seen as undesirable even across edition boundaries (#64586 (comment))

While this is true, I'm hoping we can provide migration tools that change the code so that it works as if it was in the previous edition.

Also, there are situations where eat-one-layer is not obviously more intuitive: (#64586 (comment))

I think that the non-intuitive part here is not eat-one-layer-&, but the fact that we implicitly add dereferences with match ergonomics (because double ref is not possible there).

@Nadrieril
Copy link
Member

it's a silent change in behavior

Is it? Most of the time it will cause a type error. Cases where this can cause a silent change in behavior seem pretty rare (things like dyn casting maybe?).

IMO eating one layer is a lot more intuitive

I'm personally not convinced either way. In eat-both-layers, &<pat> removes all references (ish). That's pretty convenient to use. In eat-one-layer I have to count references more.

Regarding migration, we can easily provide a tool that makes 2021 code forwards-compatible with 2024 (by adding a few extra & to ensure the & patterns all eat a single layer). What seems harder is if someone takes 2021 code into 2024, which now causes type errors, and I don't know how easy it would be to detect these and provide a fix.

The situation is similar with the mut x change in fact: it's easy to provide a migration tool for people on 2021 who want to upgrade, harder to provide helpful errors for people who move to 2024 by hand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-patterns Relating to patterns and pattern matching C-enhancement Category: An issue proposing an enhancement or a PR with one. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
Status: Idea
Development

No branches or pull requests