-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Destructuring assignment #2909
Destructuring assignment #2909
Conversation
I really like the idea of having this in the language; it's a very good ergonomics boost. I do have a few concerns however:
Otoh, this is an awesome RFC and I've wanted this feature many, many times. Thanks @varkor! |
What about doing a conditional destructuring assignment with enums? In this example fn foo(n: i32)->Option<i32>{
if n%2==0 {
None
}else{
Some(n/2)
}
}
let mut a=0;
// `a` is now 49
Some(a)=foo(99);
// `a` is still 49
Some(a)=foo(98);
// `a` is now 48
Some(a)=foo(97); |
Are there no complexities from default/implicit binding modes here? Or do you implicitly reject them when you reject |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@strega-nil Thanks for the feedback!
I'm not a fan of this as it's inconsistent with normal assignment. There is no marker for "this is going to be an assignment" either. The left-hand side of an assignment can already be complex, i.e.
It is still an expression as it desugars to a block expression. The RFC should probably be clearer about this. Could you edit it, @varkor?
I agree we should allow this. What do you think @varkor? The current prototype implementation actually allows univariant enums, in fact. |
The latter: It's rejected because we don't allow |
- It [has been suggested](https://github.com/rust-lang/rfcs/issues/372#issuecomment-365606878) that | ||
mixed declarations and assignments could be permitted, as in the following: |
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.
The inverse also seems reasonable- add syntax for an identifier pattern to assign to an existing variable rather than create a new one. That would support all patterns "for free," might largely make destructing assignment redundant, and might be a smaller extension to the language.
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.
Could you give an example of what you're imagining? The only way I could see this working is by using a new keyword to specify a destructuring assignment, but functionally this would be exactly the same. Note that the syntax has to be at least somewhat similar to what we have here, because we don't just allow assignment to patterns: we allow assignment to fields, etc. which are not valid 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.
I'm imagining extending the existing pattern syntax something like this:
// old_ident is already in scope, and is written to here:
let (<magic keyword> old_ident, new_ident) = some_tuple;
This approach doesn't need any sort of overlap between the expression and pattern grammars, it just introduces a new kind of identifier pattern to the grammar.
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.
This would cover assignment to identifiers, but not any other kind of assignable expression (i.e. lvalue), like paths, function calls returning mutable references, etc. I can add it as an alternative, though.
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'm not sure why it wouldn't cover other assignable expressions- <magic keyword>
can prime the parser to expect any place expression.
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.
Ah, in that case, then I agree that this should work; I shall add it as an alternative. However, I don't think this would be a natural extension to the language.
- It requires a new keyword or overloading an existing one.
- It is something that needs to be learnt (whereas I would argue that attempting destructuring assignment with the syntax we propose is something people already try naturally).
- It changes the meaning of
let
(which has previously been associated only with binding new variables). - To be consistent, we would probably want to allow
let <magic keyword> x = value;
, which introduces another way to simply writex = value;
. - It is longer and no more readable than the proposed syntax.
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 find @varkor's arguments very convincing. And in any case, this is a future extension, so I don't think it makes sense to discuss such details in this RFC.
EDIT: Sorry, I didn't understand this was an alternative to the whole destructuring assignment syntax. In that case it is relevant, of course.
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 think that's a good summary of the downsides. Just want to make sure it's considered as an alternative, given the relative scopes of the two approaches.
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.
This idea had been raised before in the internals.
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.
In addition to what @varkor wrote above, I would suggest that people could simply pre-declare the variables they would like to declare in a mixed assignment. So instead of
// old_ident is already in scope, and is written to here:
let (<magic keyword> old_ident, new_ident) = some_tuple;
one would use
// old_ident is already in scope
let new_indent; // new_indent is now also in scope
(old_ident, new_ident) = some_tuple; // write to both
Regarding changing the syntax: I agree with @fanzier. The point is to make destructuring assignment be just like normal assignment. This matches the syntax in all other languages with destructuring assignment that I'm familiar with.
We can't use actual patterns on the left-hand side of an assignment (for example, this would not permit you to write |
[*a] = [1, 2]; // error: pattern requires 1 element but array has 2 | ||
``` | ||
|
||
Whilst `[*a]` is not strictly speaking a pattern, it behaves similarly to one in this context. We |
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'm not familiar with the details of Rust's grammar, but would it be technically incorrect to say that we're proposing a restricted subset of patterns? Or creating a sort of "assignment pattern" grammar?
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: we allow some expressions that cannot be written in patterns, like field accesses. We could describe the permitted expressions using a restricted grammar, which would look somewhat similar to that for patterns.
At a high level I think my only concern is that I have no feel for what we aren't including here. In one sense that's a strong vote of confidence in the RFC's selection of what patterns we care about. But I would still like to spell out what "full-blown patterns" has that this RFC does not. The only thing I could come up with is nesting like |
Huzzah! The @rust-lang/lang team has decided to accept this RFC. You can follow along with future developments at the tracking issue rust-lang/rust#71126. |
Why not? Wouldn't it be useful to allow destructuring of ranges this way? You might say: "But it's possible with
I think we should allow destructuring of ranges (in a way that also works for |
Btw, the "Rendered." link in the first post is 404. |
@Boscop Destructuring ranges via Further, you can also use this in irrefutable contexts (i.e. Destructuring assignment is defined to work via a desugaring to pattern matching. So even if you wanted it to behave differently, you'd need to come up with some extra rule for why range syntax place expressions don't just desugar to patterns like everything else. |
…enkov Implement destructuring assignment for tuples This is the first step towards implementing destructuring assignment (RFC: rust-lang/rfcs#2909, tracking issue: rust-lang#71126). This PR is the first part of rust-lang#71156, which was split up to allow for easier review. Quick summary: This change allows destructuring the LHS of an assignment if it's a (possibly nested) tuple. It is implemented via a desugaring (AST -> HIR lowering) as follows: ```rust (a,b) = (1,2) ``` ... becomes ... ```rust { let (lhs0,lhs1) = (1,2); a = lhs0; b = lhs1; } ``` Thanks to `@varkor` who helped with the implementation, particularly around default binding modes. r? `@petrochenkov`
…ing, r=petrochenkov Implement destructuring assignment for structs and slices This is the second step towards implementing destructuring assignment (RFC: rust-lang/rfcs#2909, tracking issue: rust-lang#71126). This PR is the second part of rust-lang#71156, which was split up to allow for easier review. Note that the first PR (rust-lang#78748) is not merged yet, so it is included as the first commit in this one. I thought this would allow the review to start earlier because I have some time this weekend to respond to reviews. If `@petrochenkov` prefers to wait until the first PR is merged, I totally understand, of course. This PR implements destructuring assignment for (tuple) structs and slices. In order to do this, the following *parser change* was necessary: struct expressions are not required to have a base expression, i.e. `Struct { a: 1, .. }` becomes legal (in order to act like a struct pattern). Unfortunately, this PR slightly regresses the diagnostics implemented in rust-lang#77283. However, it is only a missing help message in `src/test/ui/issues/issue-77218.rs`. Other instances of this diagnostic are not affected. Since I don't exactly understand how this help message works and how to fix it yet, I was hoping it's OK to regress this temporarily and fix it in a follow-up PR. Thanks to `@varkor` who helped with the implementation, particularly around the struct rest changes. r? `@petrochenkov`
…ing, r=petrochenkov Implement destructuring assignment for structs and slices This is the second step towards implementing destructuring assignment (RFC: rust-lang/rfcs#2909, tracking issue: rust-lang#71126). This PR is the second part of rust-lang#71156, which was split up to allow for easier review. Note that the first PR (rust-lang#78748) is not merged yet, so it is included as the first commit in this one. I thought this would allow the review to start earlier because I have some time this weekend to respond to reviews. If ``@petrochenkov`` prefers to wait until the first PR is merged, I totally understand, of course. This PR implements destructuring assignment for (tuple) structs and slices. In order to do this, the following *parser change* was necessary: struct expressions are not required to have a base expression, i.e. `Struct { a: 1, .. }` becomes legal (in order to act like a struct pattern). Unfortunately, this PR slightly regresses the diagnostics implemented in rust-lang#77283. However, it is only a missing help message in `src/test/ui/issues/issue-77218.rs`. Other instances of this diagnostic are not affected. Since I don't exactly understand how this help message works and how to fix it yet, I was hoping it's OK to regress this temporarily and fix it in a follow-up PR. Thanks to ``@varkor`` who helped with the implementation, particularly around the struct rest changes. r? ``@petrochenkov``
…trochenkov Implement destructuring assignment for structs and slices This is the second step towards implementing destructuring assignment (RFC: rust-lang/rfcs#2909, tracking issue: #71126). This PR is the second part of #71156, which was split up to allow for easier review. Note that the first PR (#78748) is not merged yet, so it is included as the first commit in this one. I thought this would allow the review to start earlier because I have some time this weekend to respond to reviews. If ``@petrochenkov`` prefers to wait until the first PR is merged, I totally understand, of course. This PR implements destructuring assignment for (tuple) structs and slices. In order to do this, the following *parser change* was necessary: struct expressions are not required to have a base expression, i.e. `Struct { a: 1, .. }` becomes legal (in order to act like a struct pattern). Unfortunately, this PR slightly regresses the diagnostics implemented in #77283. However, it is only a missing help message in `src/test/ui/issues/issue-77218.rs`. Other instances of this diagnostic are not affected. Since I don't exactly understand how this help message works and how to fix it yet, I was hoping it's OK to regress this temporarily and fix it in a follow-up PR. Thanks to ``@varkor`` who helped with the implementation, particularly around the struct rest changes. r? ``@petrochenkov``
Make `_` an expression, to discard values in destructuring assignments This is the third and final step towards implementing destructuring assignment (RFC: rust-lang/rfcs#2909, tracking issue: #71126). This PR is the third and final part of #71156, which was split up to allow for easier review. With this PR, an underscore `_` is parsed as an expression but is allowed *only* on the left-hand side of a destructuring assignment. There it simply discards a value, similarly to the wildcard `_` in patterns. For instance, ```rust (a, _) = (1, 2) ``` will simply assign 1 to `a` and discard the 2. Note that for consistency, ``` _ = foo ``` is also allowed and equivalent to just `foo`. Thanks to ````@varkor```` who helped with the implementation, particularly around pre-expansion gating. r? ````@petrochenkov````
I would have preferred a The most obvious advantage of that is that the same syntax would have easily been usable in any position where a pattern can appear; this is not discussed at all in the alternatives section, and it’s far from obvious how it could possibly be replicated with this RFC. As for prior art,
All those listed languages embrace mutability in their idiomatic forms, where Rust tries to gently steer the user away from it. In light of this, destructuring assignment taking more characters to express is a feature, not a bug. |
@fstirlitz What situations are you imagining here? I don't think this would actually be useful in a lot of places other than assignment. In Rust, patterns can occur in:
I totally agree with you about Rust steering users away from using mutation and that's a good thing. However, assignment has always been an exception to this. We just write In addition, allowing |
@fanzier I think where let (a, reassign b, c) = foo(); The alternative with destructuring assignment is let (a, c);
(a, b, c) = foo(); Which I think is hardly better than what you have to do currently: let (a, b1, c) = foo();
b = b1; This requires creating a new variable, whose only purpose is to assign it to another variable. It's almost impossible to come up with a good variable name for it. And while it's clear what this code does, it doesn't convey the intent very well. |
@Aloso Good point, I agree with that. But instead of let (reassign a, b) = foo(); I think this would be nicer to write: (a, let b) = foo(); The latter uses an existing keyword and does not change the pattern syntax at all (as opposed to |
This is the one I primarily had in mind. In other languages, I have on occasion needed to write loops where I needed to remember the last processed element after the loop terminates. With a reassignment modifier (I remember let mut last_item = default_value;
while let Some(in last_item) = next_item() {
println!("process {}", last_item);
}
println!("last {}", last_item); It might also be useful with general Maybe even the
Not really. The fact that you need to declare a binding as And it’s not necessarily a grave problem if this reassignment syntax is a little less convenient than in other languages, if it’s meant to be used rarer than in those languages. Another argument for integrating reassignment into the general pattern syntax is a bit more abstract. It has to do with pursuing a holistic design instead of a piecemeal one. This RFC is unfortunately of the latter kind. It carves out special syntax meant to address just one use case, without leaving an obvious path to extending it to other use cases. You’re already running into the question of how to mix reassignment to existing bindings with creating new ones. Solving this use case, and others like it, would effectively require creating a parallel syntax for reassignment patterns, that is distinct from pattern syntax in all other contexts, follows different rules, will have to be separately learnt and will considerably complicate the parser. Even as it is, it complicates the parser enough. Consider: (a, b, c, d) ( f() );
(a, b, c, d) = f();
// ↑ It’s not until the token above the arrow that the parser can be sure that one line is (syntactically) a function call, and the other an assignment. This is a pain to parse, and I would know, because I once had to implement something similar myself. (I am sure hoping my test suite catches all corner cases here.) I don’t know if this RFC induces any outright ambiguities, but I wouldn’t be very surprised if it did. (If it manages to avoid those, it seems it’s by the skin of its teeth, and it may already prevent some other syntax extensions without introducing ambiguities. See https://internals.rust-lang.org/t/the-magic-type-doesnt-work-with-arrays/14679, which again, I have not studied in depth yet, but it does seem worrying.) I don’t think this complexity is worth a feature that is meant to be used relatively rarely. I cannot help but draw parallels between this RFC and #2544 (by the same author, perhaps not so coincidentally), which meant to replace a small syntax wart (turbofish) with a worse syntax wart (most vexing parse) in the name of short-term convenience. I feel like this RFC is similarly short-sighted. |
I completely disagree. Destructuring assignment is the natural syntax for reassignment. This is particularly evident when considering other languages with destructuring assignment, which use exactly the same pattern. Adding a new keyword would be antithetical to a minimalist and ergonomic approach to language design.
I have rarely seen this pattern come up in real-world code. The discussion about mixed reassignment/declaration seems mostly hypothetical in this thread so far. I'm not convinced it's a common enough pattern to introduce dedicated syntax.
Please don't make unsubstantiated claims. This RFC has already been implemented without any parsing ambiguities. Nor has there been any suggestion that it introduces any. Unless you have a particular example, making a comment like this is not helpful.
In my experience, destructuring assignment is a relatively common pattern.
Please be careful when you write comments like this: it may not be what you intended, but it sounds like you are casting aspersions; also note that this RFC is co-authored with @fanzier. As evidenced by that thread, opinions on #2544 were highly polarised. Just because you were not in agreement does not mean there were not important issues the proposal was intended to fix (and that many people agreed with). As far as I can tell, you have raised no concrete problems with this destructuring assignment proposal, other than hypothetical problematic interactions with mixed reassignment/declaration or with reassignment in control flow (whose value is disputable). This makes it difficult to view comments like "this RFC is similarly short-sighted" as constructive. If you think mixed reassignment/declaration is an important feature, I suggest you begin a dedicated RFC, in which you can present your arguments that such patterns are common enough to warrant dedicated syntax. |
If I understand, we de facto have reassign already because
In several years time, someone can run a search over crates.io for this pattern, no reason for debate now. |
When I first saw this RFC I was worried about something like that too. But then as I read it, I was please to see that it elegantly doesn't have the issue at all because it's not a different grammar for this, and doesn't need to try one thing then the other. Note that "can't tell it's an assignment for a while" is normal in Rust. Even before this RFC, |
I'm using destructuring assignment since long time in Python, and now I'm using it in Nightly Rust too, almost as often as "if let". On the other hand, I think so far in my Nightly Rust code I've felt the need for destructuring with mixed declaration/reasignment only once or twice, so probably I won't miss this much. |
This RFC is co-authored with @fanzier.
We allow destructuring on assignment, as in
let
declarations. For instance, the following are nowaccepted:
This brings assignment in line with
let
declaration, in which destructuring is permitted. Thiswill simplify and improve idiomatic code involving mutability.
A working prototype may be found at rust-lang/rust#71156.
Rendered.
Thanks to @joshtriplett for providing feedback on the RFC.
Closes #372.