Match ergonomics 2024: implement mutable by-reference bindings

[Jules-Bertholet]

Implements the mutable by-reference bindings portion of match ergonomics 2024 (#123076), with the mut ref/mut ref mut syntax, under feature gate mut_ref.

r? @Nadrieril

@rustbot label A-patterns A-edition-2024

[rustbot]

Some changes occurred in match checking

cc @Nadrieril

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Some changes occurred in match lowering

cc @Nadrieril

[Nadrieril]

From a process perspective, we don't have an accepted RFC for this so I think we need a lang team liaison. Either that or write a precise proposal that can be approved (the original proposal was positively received but didn't get proposer "we agree to do this" kind of approval)

[Nadrieril]

First of all, thanks for doing this work!

This PR is doing a bunch of different things at once; for future PRs I would have found it much easier to review a series of commits each doing a single change, e.g. the first commit would have changed how ByRef/BindingAnnotation work, a second one would have removed BindingMode etc and the last one would have added the new mut ref mut/mut ref possibility.

No need to change that now though, I've read through it. I like what you did with BindingAnnotation; this looks good. r=me once the feature gate is marked "incomplete" and we get a liaison.

[Nadrieril]

it would be nice to extend this help message to suggest mut ref as well when the feature is active. I'lll let you decide whether to include in this PR or do it separately

[Jules-Bertholet]

I'll do it separately, I'd rather the BindingAnnotation/ByRef changes land first so later PRs can make use of them

[bors]

☔ The latest upstream changes (presumably #123121) made this pull request unmergeable. Please resolve the merge conflicts.

[Jules-Bertholet]

This should be ready to go, now that we have a liaison (#123076 (comment)).

@rustbot label -S-waiting-on-team S-waiting-on-review

[Nadrieril]

Let's goooo

@bors r+

[bors]

📌 Commit e931595 has been approved by Nadrieril

It is now in the queue for this repository.

[bors]

⌛ Testing commit e931595 with merge 170598b...

[bors]

💔 Test failed - checks-actions

[rust-log-analyzer]

The job dist-aarch64-msvc failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

-- Configuring done (29.2s)
-- Generating done (2.5s)
-- Build files have been written to: C:/a/rust/rust/build/x86_64-pc-windows-msvc/llvm/build
running: "cmake" "--build" "." "--target" "install" "--config" "Release" "--" "-j" "8"
ninja: error: build.ninja:3780: multiple outputs aren't (yet?) supported by depslog; bring this up on the mailing list if it affects you
thread 'main' panicked at C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\cmake-0.1.48\src\lib.rs:975:5:

command did not execute successfully, got: exit code: 1

[Jules-Bertholet]

https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/ninja.20build.20failing needs retry

[Nadrieril]

@bors retry

[bors]

⌛ Testing commit e931595 with merge 45796d1...

[bors]

☀️ Test successful - checks-actions
Approved by: Nadrieril
Pushing 45796d1 to master...

[rust-timer]

Finished benchmarking commit (45796d1): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 668.769s -> 668.043s (-0.11%)
Artifact size: 315.81 MiB -> 315.85 MiB (0.01%)

[zachs18]

I think this doesn't feature-gate mut ref patterns for shorthand fields in struct patterns. Was that intentional?

Specifically, this code fails before this PR but compiles after (in all editions), with no unstable features enabled (playground).

#![allow(unused)]

struct Foo {
    x: i32,
}

fn main() {
    let Foo { mut ref x } = Foo { x: 4 };
    // let Foo { x: mut ref x } = Foo { x: 4 }; // this errors correctly
}

before:

error: expected identifier, found keyword `ref`
 --> src/main.rs:6:19
  |
8 |     let Foo { mut ref x } = Foo { x: 4 };
  |                   ^^^ expected identifier, found keyword

error: expected `,`
 --> src/main.rs:6:23
  |
8 |     let Foo { mut ref x } = Foo { x: 4 };
  |         ---           ^
  |         |
  |         while parsing the fields for this pattern

error: could not compile `bisect-when-mut-ref-patterns-were-added` (bin "bisect-when-mut-ref-patterns-were-added") due to 2 previous errors

after: (no errors)

I think this is missing a gate in parse_pat_field, like it has in parse_pat_ident_mut

diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs
index 59e0cd92c4c..fd524462e76 100644
--- a/compiler/rustc_parse/src/parser/pat.rs
+++ b/compiler/rustc_parse/src/parser/pat.rs
@@ -1404,6 +1404,10 @@ fn parse_pat_field(&mut self, lo: Span, attrs: AttrVec) -> PResult<'a, PatField>
             let fieldpat = self.mk_pat_ident(boxed_span.to(hi), ann, fieldname);
             let subpat =
                 if is_box { self.mk_pat(lo.to(hi), PatKind::Box(fieldpat)) } else { fieldpat };
+            if matches!(subpat.kind, PatKind::Ident(BindingAnnotation(ByRef::Yes(_), Mutability::Mut), ..))
+            {
+                self.psess.gated_spans.gate(sym::mut_ref, subpat.span);
+            }
             (subpat, fieldname, true)
         };

[Nadrieril]

Damn that's definitely not intentional! If you don't mind opening an issue or even a PR given that you already know how to fix it!