-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Implement destructuring assignment for tuples #78748
Conversation
61e78fd
to
d301378
Compare
Thanks! |
Co-authored-by: varkor <github@varkor.com>
bacd41f
to
3a7a997
Compare
Just squashed and force pushed. Thanks for the quick review, @petrochenkov! I don't think I have the rights to r+, so I'll wait for someone to do that. |
@bors r+ |
📌 Commit 3a7a997 has been approved by |
I've seen that about 100% of the destructuring assignments in my codebase are for tuples (usually 2-tuples, but sometimes longer). So I think it's OK to prioritize the merging of this pull, and work on the rest of the destructuring assignment cases later. |
@leonardo-m: the rest of the cases are already implemented and just have to be split out from the original pull request. |
Rollup of 12 pull requests Successful merges: - rust-lang#77640 (Refactor IntErrorKind to avoid "underflow" terminology) - rust-lang#78026 (Define `fs::hard_link` to not follow symlinks.) - rust-lang#78114 (Recognize `private_intra_doc_links` as a lint) - rust-lang#78228 (Promote aarch64-unknown-linux-gnu to Tier 1) - rust-lang#78345 (Fix handling of item names for HIR) - rust-lang#78437 (BTreeMap: stop mistaking node for an orderly place) - rust-lang#78476 (fix some incorrect aliasing in the BTree) - rust-lang#78674 (inliner: Use substs_for_mir_body) - rust-lang#78748 (Implement destructuring assignment for tuples) - rust-lang#78868 (Fix tab focus on restyled switches) - rust-lang#78878 (Avoid overlapping cfg attributes when both macOS and aarch64) - rust-lang#78882 (Nicer hunk headers for rust files) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
I've finally tried this feature in my codebase, and I've seen in some situations it's not a drop-in replacement for the swap() function: #![feature(destructuring_assignment)]
#![allow(unused_imports, unused_assignments)]
use std::mem::swap;
fn foo1() {
let a = &mut [0_u32; 10];
let b = &mut [0_u32; 10];
swap(a, b); // OK
(a, b) = (b, a); // cannot assign twice to immutable variable error
}
fn foo2(va: &mut Vec<u32>, vb: &mut Vec<u32>) {
swap(vb, va); // OK
(vb, va) = (va, vb); // Lifetime mismatch error
}
struct Bar {
vb: Vec<u32>,
}
impl Bar {
fn spam(&mut self) {
let mut va = vec![0_u32; 10];
swap(&mut self.vb, &mut va); // OK
(self.vb, va) = (va, self.vb); // Error cannot move out of `self.vb` which is behind a mutable reference
}
}
fn main() {} (Also a possible bug report: it shows only the first error here). |
@leonardo-m I think the line should read
This error makes sense to me because we really are moving out of it (very briefly) before reassigning something else. It seems that |
You're very kind, thank you. |
Do you know why my original code doesn't show all three errors, but only the first? |
@leonardo-m If I run your code in the playground, I get two errors, not one. Both of them are about mismatched lifetimes (meaning that type-checking fails). The other errors arising after adding |
…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``
test: add `()=()=()=...` to weird-exprs.rs Idea from rust-lang#71156 (comment) 😄 Builds on nightly since rust-lang#78748 has been merged.
…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``
This is the first step towards implementing destructuring assignment (RFC: rust-lang/rfcs#2909, tracking issue: #71126). This PR is the first part of #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:
... becomes ...
Thanks to @varkor who helped with the implementation, particularly around default binding modes.
r? @petrochenkov