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 2024 #3627
base: master
Are you sure you want to change the base?
Match ergonomics 2024 #3627
Conversation
(including "inherited" references). | ||
|
||
```rust | ||
let &foo = &mut 42; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
foo
is still immutable right?
let &foo = &mut 42;
foo = 53; // error[E0384]: cannot assign twice to immutable variable `foo`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. (It would be immutable with let &mut foo = &mut 42;
also.)
Co-authored-by: kennytm <kennytm@gmail.com>
The " |
It could be split off, yes. I mention this in the rationale section, but one of the motivating factors for including it is the "no inherited |
The "no inherited |
It is not. On edition 2024, this example would work without the rule, but errors with it: (I'll edit the RFC to make that more clear) |
I'm not sure why (a version of) "no inherited Really I'm not sure why we would even want to reject it in the first place. There is still a This is exactly the sort of thing that would be good to work out as part of deref patterns. Going back to the comparison with places, the reason you can get a Accepting this example is a different sort of consistency than the "immutability takes precedence" one. Instead, it's "inherited reference type matches skipped reference type." This also brings back the locality that " If we want to defer any decision at all here, I believe we could instead forbid matching |
If there are several skipped reference types with different mutabilities, you have to choose: eg in |
let _: &mut u8 = foo; | ||
``` | ||
|
||
## Edition 2024: `&` and `&mut` can match against inherited references |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! With this, is there even a reason to keep supporting &mut
in patterns?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, without it we can't get desugar match ergonomics that get mutable references into places: let &mut Struct { ref mut field } = ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, okay. Sounds worthy of a clippy lint for if it's not combined with ref mut
then :)
The innermost one, of course! Why would it be anything else? That's how default binding modes already work, and we're merely exposing that to the user so they can exit the current mode, not trying to reinvent how the mode is entered. |
No it's not! Outer shared + inner mutable results in |
What I'm saying is that it is how initially entering a default binding mode already works. That's why the example The inconsistency we're both talking about is the difference in behavior between "behind an explicit You're proposing we resolve it in favor of the binding mode (essentially letting explicit |
Here's another way of looking at it: if Simple case: let &mut x = &mut 42; // x: i32
let x = &mut 42; // x: &mut i32 Roundabout case: let &[&mut x] = &[&mut 42]; // x: i32
//let &[x] = &[&mut 42]; // ERROR, but…
let &[ref x] = &[&mut 42]; // x: &&mut i32 -> we can get an &mut in the end However: let &[[&mut x]] = &[&mut [42]]; // If we allow this, with x: i32
//let &[[x]] = &[&mut [42]]; // and then remove the &mut… -> ERROR move check, if the default binding mode is to be `ref mut`
// nothing we do will get us &mut i32 in any form |
Indeed, that is a demonstration of the fact that we are limited to a single layer of inherited reference/a single binding mode. If we relaxed that (not that I think this would be useful in practice), you could get a let &[[&mut x]] = &[&mut [42]]; // x: i32
let &[[x]] = &[&mut [42]]; // ERROR move check, unless the pattern equivalent of auto(de)ref coerces &mut i32 -> x: &i32
let &[[ref x]] = &[&mut [42]]; // HYPOTHETICAL: A `ref` binding + an inherited `&mut` -> x: &&mut i32 My proposal is not so much breaking the user's ability to remove a If I understand what you're getting at, your proposal would avoid this edge case by preemptively replacing |
It is semantically impossible to relax that. E.g. let var = Some(0);
if let Some(x) = &&var {
//...
} Here |
Well, that was only a hypothetical to demonstrate a different way of thinking about the limitation Jules pointed out. But I don't think it's entirely impossible to materialize those extra reference layers- we could introduce temporaries for them around the match, it would just be a bit silly, and they would have shortened lifetimes. (Not to mention confusing to be able to layer binding modes like my example.) |
Arrays are homogeneous…
@rfcbot fcp merge We discussed this RFC in the lang-team design meeting. There was general excitement about the contents (subject to one minor caveat which I will file as a concern). The reasoning made sense and the changes felt like they would make the language more true to itself. Therefore, I am moving to fcp merge now. |
Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
@rfcbot concern leaving-us-in-an-inconsitent-state The RFC as proposed leaves us in a bit of inconsistent state. It means that you can write let [mut x] = &mut [22]; ...and get a binding let &mut [ref mut (mut x)] = &mut [22]; ...but the RFC (intentionally) avoids proposing that syntax or any explicit replacement for it. In the meeting we identified three options:
Of these, we felt that option 2 (the current text) was in some ways the worse. Option 1 might be annoying but it's internally consistent and leaves room for Option 3. Option 3 is perhaps the most nice but also adds confusing syntax for what feels like an edge case. In the meeting we felt it was best to start with Option 1 for now and consider Option 3 later. We identified the following desirable properties (we are sacrificing 1 for now when there are inherited references):
|
@rfcbot concern switch-to-option-1 I share @nikomatsakis's concern, and I would specifically like to go with option 1. This does not close the door on doing option 3 in the future, after introducing new syntax for it. |
I've updated the RFC in light of today's lang team design meeting (minutes).
|
I would still prefer to be able to consistently write a As we discussed above, this makes the set of available patterns more local, which relaxes the need to commit to |
This makes the model more complex, since we would need to distinguish "ref binding mode" from "ref binding mode that can be matched with |
In that sense, the current RFC also introduces an additional binding mode: " Another framing, rather than changing the binding modes at all, is to defer this sort of " When the inherited reference corresponds to the actual skipped reference, the combined model gets simpler: pattern and scrutinee types match, and dealing with shared |
@rustbot labels +I-lang-nominated Let's nominate to ensure that we discuss and review the updates to the RFC that we requested and whether those resolve the outstanding concerns. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for working on this RFC, hopefully we'll soon be able to finally solve the papercut of not being able to deref inhereted references 🤞🏻
references. | ||
- On edition ≥ 2024, `mut` on an identifier pattern does not force its binding | ||
mode to by-value. | ||
- On edition ≥ 2024, `&` patterns can match against `&mut` references. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is not a breaking change, I would highly advice this be split from this RFC. Too much changes at the same time make it harder to reason about; the time pressure from the edition is also exists, and we should not try to push changes with the edition ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mention this in a footnote, but &
matching against &mut
alleviates the downsides of the "no ref mut
behind &
" part of the RFC, which motivates them being included together.
|
||
```rust | ||
//! Edition ≥ 2024 | ||
//let (x, mut y) = &(true, false); // ERROR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this an error and not just I see this was requested by T-lang membersmut y: &bool
?
@nikomatsakis @joshtriplett I fail to see the benefit of "option 1". It feels like making things worse, for the ability to delay decision, even though there is only 1 semantic for i.e. making it an error does not actually help anything, and the state where you have neither the match ergonomics way not the "explicit" way does not sound good to me. I see how having an explicit way might be desirable, but I don't think it should be a blocker. |
AIUI, what you're proposing would be backward compatible. There is going to be a follow-up to this RFC at some point (to choose a syntax for mutable by-reference bindings, and possibly also to provide a solution for matching |
The reason I'm making this proposal in the first place is because it enables us to defer " |
@rfcbot reviewed Having mulled it over, I think this RFC strikes the right balance on all the questions we need to decide now. I would also like to see us move forward with an implementation in nightly quickly to give enough time for experimentation with the new rules and with the edition migration.
I would rather have the first option work than the second, especially when considering the alternatives (to binding
While I can see that we might want to allow this in the future, it's not entirely clear to me that this is the case. As the RFC states, "there is not much use for mutable by-reference bindings". They seem rare enough that I would guess a significant percentage of the time, they are not actually what the user wants when written, and when used correctly are unclear to the reader. |
ICYMI: rust-lang/rust#123076 Everything in this RFC, including the migration lint, is either already in nightly under an experimental feature gate, or waiting on PR review. |
d509ffa
to
04501bc
Compare
Reviewing this section again, I've realized that I grossly overstated my case. For the |
As Jules noted, it should be backwards compatible with the RFC to accept the Rather, given that we want to make
|
Rendered
Changes to match ergonomics for the 2024 edition.
@rustbot label T-lang A-patterns A-edition-2024