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

Tracking Issue for destructuring_assignment #71126

Closed
7 tasks done
varkor opened this issue Apr 14, 2020 · 25 comments · Fixed by #90521
Closed
7 tasks done

Tracking Issue for destructuring_assignment #71126

varkor opened this issue Apr 14, 2020 · 25 comments · Fixed by #90521
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-destructuring_assignment `#![feature(destructuring_assignment)]` finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@varkor
Copy link
Member

varkor commented Apr 14, 2020

This is a tracking issue for the RFC "Destructuring assignment" (rust-lang/rfcs#2909).
The feature gate for the issue is #![feature(destructuring_assignment)].

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also uses as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Implementation history

The initial implementation was carried out in three steps by @fanzier (split out from an initial implementation, #71156):

@varkor varkor added T-lang Relevant to the language team, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Apr 14, 2020
@varkor
Copy link
Member Author

varkor commented Apr 17, 2020

We've opened an RFC for destructuring assignment: rust-lang/rfcs#2909.

@varkor varkor changed the title [WIP] Tracking Issue for destructuring_assignment Tracking Issue for destructuring_assignment Oct 27, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Nov 9, 2020
…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`
m-ou-se added a commit to m-ou-se/rust that referenced this issue Nov 12, 2020
…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`
m-ou-se added a commit to m-ou-se/rust that referenced this issue Nov 12, 2020
…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``
@varkor
Copy link
Member Author

varkor commented Nov 16, 2020

Now that #78748, #78836, and #79016 have been merged, destructuring assignment is now implemented. Thanks to @fanzier for their hard work implementing this feature!

@varkor varkor added the F-destructuring_assignment `#![feature(destructuring_assignment)]` label Nov 19, 2020
flip1995 pushed a commit to flip1995/rust that referenced this issue Nov 20, 2020
…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``
flip1995 pushed a commit to flip1995/rust that referenced this issue Nov 20, 2020
…etrochenkov

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: rust-lang#71126). This PR is the third and final part of rust-lang#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````
@bstrie
Copy link
Contributor

bstrie commented Feb 17, 2021

I got kind of a weird error message recently that probably isn't too concerning, but maybe someone more familiar with the implementation of the _ desugaring might have more insight.

The following extremely weird program:

fn main() {
    let y = Some(42);
    if y < Some(_) {}
}

Errors with:

error[E0658]: destructuring assignments are unstable
 --> src/main.rs:3:17
  |
3 |     if y < Some(_) {}
  |                 ^
  |
  = note: see issue #71126 <https://github.com/rust-lang/rust/issues/71126> for more information
  = help: add `#![feature(destructuring_assignment)]` to the crate attributes to enable

error: in expressions, `_` can only be used on the left-hand side of an assignment
 --> src/main.rs:3:17
  |
3 |     if y < Some(_) {}
  |                 ^ `_` not allowed here

error: aborting due to 2 previous errors

I'm wondering whether it's considered a problem that the compiler appears to consider Some(_) to be part of a destructuring assignment. Obviously even if this feature were stabilized this program would still error out, but I found it surprising.

@fanzier
Copy link
Contributor

fanzier commented Feb 17, 2021

@bstrie Thanks for reporting this. The error message is indeed a bit misleading.

I'm wondering whether it's considered a problem that the compiler appears to consider Some(_) to be part of a destructuring assignment.

In fact, the compiler does not consider this _ to be part of a destructuring assignment. However, the feature destructuring assignments allows _ to occur in more places than before (namely the left-hand side of an assignment). This necessitated a parser change: we allow _ syntactically anywhere in an expression and only check later on that it only occurs on the left-hand side of an assignment. That parser change is feature-gated on destructuring assignments. If the _ indeed occurs on the LHS of an assignment, the error message is correct because there it is only allowed if the destructuring assignments feature is enabled. Otherwise, the error message is misleading because _ is illegal in all other expressions and that has nothing to do with destructuring assignments.

I didn't think this was a big problem because as soon as destructuring assignments are stabilized, you'll just get the second error message (_ can only be used on the left-hand side of an assignment), which is correct. I'm not sure what the best fix for this would be. Do you think it is important to fix while the feature is unstable?

@scottmcm
Copy link
Member

scottmcm commented Feb 17, 2021

I think this is ok -- the older one wasn't particularly amazing:

error: expected expression, found reserved identifier `_`
 --> <source>:3:17
  |
3 |     if y < Some(_) {}
  |                 ^ expected expression

That said, it might be nice to tune the error based on whether there's a = nearby -- it could be nice to talk about patterns and destructuring assignments as places for _s, for example.

@bstrie
Copy link
Contributor

bstrie commented Feb 17, 2021

Do you think it is important to fix while the feature is unstable?

No, I was just surprised and wanted to make sure this wasn't indicative of some deeper bug. :) Ship it!

@doivosevic
Copy link

Hey everyone. I'm relatively new so I might be missing something, but I can't see what is the roadmap for this feature. Is this expected to release in the next version of Rust?

@bstrie
Copy link
Contributor

bstrie commented Apr 19, 2021

The next step is for someone on the lang team to propose a final comment period, after which the feature can be stabilized. I'll bring it up with the lang team on Zulip.

@joshtriplett
Copy link
Member

@bstrie This needs a stabilization report.

@curldivergence
Copy link

Hello @bstrie, are there any news on the stabilization, by any chance? :)

@rfcbot
Copy link

rfcbot commented Oct 12, 2021

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

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!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 12, 2021
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Oct 19, 2021
@rfcbot
Copy link

rfcbot commented Oct 19, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Oct 29, 2021
@rfcbot
Copy link

rfcbot commented Oct 29, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Oct 29, 2021
@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

I'm excited about this :)

@varkor
Copy link
Member Author

varkor commented Nov 3, 2021

Now that the final comment period has ended, anyone can open up a stabilisation PR. The process is very straightforward, and essentially involves removing the destructuring_assignment feature gate wherever it currently appears in the compiler. I'm also not quite sure what the state of documentation is: it may be that pull requests (or at the very least issues) need to be opened on the repositories for the Rust book and the reference.

@jhpratt
Copy link
Member

jhpratt commented Nov 3, 2021

I'll take a shot at a stabilization PR.

@jhpratt
Copy link
Member

jhpratt commented Nov 3, 2021

PR is up: #90521

@leonardo-m
Copy link

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Nov 18, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 14, 2021
…ignment, r=jackh726,pnkfelix

Stabilize `destructuring_assignment`

Closes rust-lang#71126

- [Stabilization report](rust-lang#71126 (comment))
- [Completed FCP](rust-lang#71126 (comment))

`@rustbot` label +F-destructuring-assignment +T-lang
Also needs +relnotes but I don't have permission to add that tag.
@bors bors closed this as completed in d258e92 Dec 15, 2021
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Dec 17, 2021
…r=jackh726,pnkfelix

Stabilize `destructuring_assignment`

Closes #71126

- [Stabilization report](rust-lang/rust#71126 (comment))
- [Completed FCP](rust-lang/rust#71126 (comment))

`@rustbot` label +F-destructuring-assignment +T-lang
Also needs +relnotes but I don't have permission to add that tag.
magnunm added a commit to magnunm/rule110 that referenced this issue Mar 14, 2022
The usage of the `destrucuring_assignment` feature was added just for
fun, it only saves two lines of code. The feature is soon stable, see
rust-lang/rust#71126.
likebreath added a commit to likebreath/cloud-hypervisor that referenced this issue Apr 28, 2022
The 'destructuring_assignment' to tuples [1] was not stable with rust
toolchina 1.56.

[1] rust-lang/rust#71126

Signed-off-by: Bo Chen <chen.bo@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-destructuring_assignment `#![feature(destructuring_assignment)]` finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.