simplify &ref x to x

Author: dianne

compiler/rustc_mir_build/src/thir/pattern/migration.rs

@@ -74,6 +74,9 @@ struct PatDeref<'a, 'tcx> {
     sugg_default_mode: ByRef,
     /// The span that introduced the current default binding mode, or `None` for the top-level pat.
     default_mode_origin: Option<Span>,
+    /// Whether this is an instance of `&ref x` which we may be able to simplify to `x`.
+    /// Stores the HIR id of the binding pattern `ref x`, to identify it later.
+    simplify_deref_ref: Option<HirId>,
     /// The next deref above this. Since we can't suggest using `&` or `&mut` on a by-ref default
     /// binding mode, a suggested deref's ancestors must also all be suggested.
     // FIXME(ref_pat_eat_one_layer_2024): By suggesting `&` and `&mut` patterns that can eat the
@@ -293,28 +296,45 @@ impl<'a, 'tcx> PatMigration<'a, 'tcx> {
         self.add_default_mode_label_if_needed();
         // This sets the default binding mode to by-value in the user's pattern, but we'll try to
         // suggest removing it.
-        self.push_deref(pat_span, mutbl, PatDerefKind::Explicit { inner_span: subpat.span });
+        let my_ix =
+            self.push_deref(pat_span, mutbl, PatDerefKind::Explicit { inner_span: subpat.span });
 
         // If this is inside a macro expansion, we won't be able to remove it.
         if pat_span.from_expansion() {
             self.add_derefs_to_suggestion(self.innermost_deref);
             return;
         }
 
-        // If the immediate subpattern is a binding, removing this reference pattern would change
-        // its type. To avoid that, we include it and all its parents in that case.
+        // If the subpattern is a binding, removing this reference pattern would change its type.
         // FIXME(ref_pat_eat_one_layer_2024): This assumes ref pats can't eat the binding mode
         // alone. Depending on the pattern typing rules in use, we can be more precise here.
-        // TODO: if the binding is by-`ref`, we can keep only the parent derefs and remove the `ref`
-        if matches!(subpat.kind, hir::PatKind::Binding(_, _, _, _)) {
-            self.add_derefs_to_suggestion(self.innermost_deref);
+        if let hir::PatKind::Binding(explicit_ba, _, _, _) = subpat.kind {
+            if explicit_ba == BindingMode(ByRef::Yes(mutbl), Mutability::Not) {
+                // If the binding has a `ref` modifier, we can elide both this `&` and the `ref`;
+                // i.e. we can simplify `&ref x` to `x`, as long as all parent derefs are explicit.
+                // NB: We don't rewrite `&ref x @ ...` to `x @ &...`, so we may end up needing to
+                // reinstate this `&` later if the binding's subpattern requires it.
+                // FIXME(ref_pat_eat_one_layer_2024): With RFC 3627's Rule 5, `&` patterns can match
+                // `&mut` types; we'll have to check the mutability of the type rather than the
+                // pattern to see whether we can elide it.
+                self.derefs[my_ix].simplify_deref_ref = Some(subpat.hir_id);
+                self.add_derefs_to_suggestion(self.derefs[my_ix].parent);
+            } else {
+                // Otherwise, we need to suggest including this `&` as well.
+                self.add_derefs_to_suggestion(self.innermost_deref);
+            }
         }
     }
 
     /// Adds a deref to our deref-forest, so that we can track the default binding mode and
     /// propagate binding mode changes when we suggest adding patterns.
     /// See [`PatMigration::propagate_default_mode_change`].
-    fn push_deref(&mut self, span: Span, mutbl: Mutability, kind: PatDerefKind<'a, 'tcx>) {
+    fn push_deref(
+        &mut self,
+        span: Span,
+        mutbl: Mutability,
+        kind: PatDerefKind<'a, 'tcx>,
+    ) -> PatDerefIdx {
         let parent = self.innermost_deref;
         // Get the new default binding mode in the pattern the user wrote.
         let real_default_mode = match kind {
@@ -345,6 +365,7 @@ impl<'a, 'tcx> PatMigration<'a, 'tcx> {
             sugg_default_mode,
             real_default_mode,
             default_mode_origin,
+            simplify_deref_ref: None,
             parent,
             next_sibling: parent.and_then(|p| self.derefs[p].first_child),
             first_child: None,
@@ -354,6 +375,7 @@ impl<'a, 'tcx> PatMigration<'a, 'tcx> {
             self.derefs[p].first_child = Some(my_ix);
         }
         self.innermost_deref = Some(my_ix);
+        my_ix
     }
 
     /// Restores the default binding mode after lowering a pattern that could change it.
@@ -371,7 +393,7 @@ impl<'a, 'tcx> PatMigration<'a, 'tcx> {
     /// Rust 2024) or if we need to suggest a binding modifier for them.
     pub(super) fn visit_binding(
         &mut self,
-        pat_span: Span,
+        pat: &hir::Pat<'_>,
         mode: BindingMode,
         explicit_ba: BindingMode,
         ident: Ident,
@@ -381,19 +403,26 @@ impl<'a, 'tcx> PatMigration<'a, 'tcx> {
             self.add_default_mode_label_if_needed();
         }
 
-        // If `mode` doesn't match the default, we'll need to specify its binding modifiers
-        // explicitly, which in turn necessitates a by-move default binding mode.
+        // As a special case, we may simplify `&ref x` to `x`; check our parent to see if we can.
+        // The default binding mode will always be by-move in this case.
+        let simplify_deref_ref = self.innermost_deref.is_some_and(|p| {
+            self.derefs[p].simplify_deref_ref.is_some_and(|binding_id| pat.hir_id == binding_id)
+        });
+
+        // Otherwise, if `mode` doesn't match the default, we'll need to specify its binding
+        // modifiers explicitly, which in turn necessitates a by-move default binding mode.
         // Additionally, if this is inside a macro expansion, we won't be able to change it. If a
         // binding modifier is missing inside the expansion, there's not much we can do, but we can
         // avoid suggestions to elide binding modifiers that are explicit within expansions.
-        let suggest = mode != BindingMode(self.sugg_default_mode(), Mutability::Not)
-            || pat_span.from_expansion() && explicit_ba != BindingMode::NONE;
+        let suggest = !simplify_deref_ref
+            && mode != BindingMode(self.sugg_default_mode(), Mutability::Not)
+            || pat.span.from_expansion() && explicit_ba != BindingMode::NONE;
 
         // Track the binding
         let span = if explicit_ba == BindingMode::NONE {
-            pat_span.shrink_to_lo()
+            pat.span.shrink_to_lo()
         } else {
-            pat_span.with_hi(ident.span.lo())
+            pat.span.with_hi(ident.span.lo())
         };
         // If we're not already suggesting an explicit binding modifier for this binding, we may
         // need to later, if adding reference patterns above it changes the default binding mode.
@@ -406,8 +435,6 @@ impl<'a, 'tcx> PatMigration<'a, 'tcx> {
         }
 
         // If there was a mismatch, add `&`s to make sure we're in a by-move default binding mode.
-        // TODO: to rewrite `&ref x` as `x`, we'll need to be able to accept a by-value default
-        // binding mode if we remove the `&` that was eating a reference from `x`'s type.
         if suggest {
             self.add_derefs_to_suggestion(self.innermost_deref);
         }
@@ -425,6 +452,7 @@ impl<'a, 'tcx> PatMigration<'a, 'tcx> {
             }
             deref.suggest = true;
             deref.sugg_default_mode = ByRef::No;
+            deref.simplify_deref_ref = None;
             opt_ix = deref.parent;
             let propagate_downstream_ref_mut = deref.mutbl.is_not();
             self.propagate_default_mode_change(ix, propagate_downstream_ref_mut);

compiler/rustc_mir_build/src/thir/pattern/mod.rs

@@ -344,8 +344,8 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
                     .get(pat.hir_id)
                     .expect("missing binding mode");
 
-                if let Some(s) = &mut self.rust_2024_migration {
-                    s.visit_binding(pat.span, mode, explicit_ba, ident);
+                if let Some(m) = &mut self.rust_2024_migration {
+                    m.visit_binding(pat, mode, explicit_ba, ident);
                 }
 
                 // A ref x pattern is the same node used for x, and as such it has

tests/ui/pattern/rfc-3627-match-ergonomics-2024/experimental/ref-binding-on-inh-ref-errors.structural2024.stderr

@@ -10,10 +10,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
    |
 LL |     let [&ref x] = &[&0];
    |         ^^^^^^^^ this matches on type `&_`
-help: make the implied reference pattern explicit
+help: rewrite the pattern
+   |
+LL -     let [&ref x] = &[&0];
+LL +     let &[x] = &[&0];
    |
-LL |     let &[&ref x] = &[&0];
-   |         +
 
 error: binding modifiers may only be written when the default binding mode is `move`
   --> $DIR/ref-binding-on-inh-ref-errors.rs:20:11
@@ -27,10 +28,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
    |
 LL |     let [&ref x] = &mut [&0];
    |         ^^^^^^^^ this matches on type `&mut _`
-help: make the implied reference pattern explicit
+help: rewrite the pattern
+   |
+LL -     let [&ref x] = &mut [&0];
+LL +     let &mut [x] = &mut [&0];
    |
-LL |     let &mut [&ref x] = &mut [&0];
-   |         ++++
 
 error: binding modifiers may only be written when the default binding mode is `move`
   --> $DIR/ref-binding-on-inh-ref-errors.rs:25:15
@@ -61,10 +63,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
    |
 LL |     let [&mut ref mut x] = &mut [&mut 0];
    |         ^^^^^^^^^^^^^^^^ this matches on type `&mut _`
-help: make the implied reference pattern explicit
+help: rewrite the pattern
+   |
+LL -     let [&mut ref mut x] = &mut [&mut 0];
+LL +     let &mut [x] = &mut [&mut 0];
    |
-LL |     let &mut [&mut ref mut x] = &mut [&mut 0];
-   |         ++++
 
 error: binding modifiers may only be written when the default binding mode is `move`
   --> $DIR/ref-binding-on-inh-ref-errors.rs:39:11
@@ -78,10 +81,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
    |
 LL |     let [&ref x] = &[&mut 0];
    |         ^^^^^^^^ this matches on type `&_`
-help: make the implied reference pattern explicit
+help: rewrite the pattern
+   |
+LL -     let [&ref x] = &[&mut 0];
+LL +     let &[x] = &[&mut 0];
    |
-LL |     let &[&ref x] = &[&mut 0];
-   |         +
 
 error: binding modifiers may only be written when the default binding mode is `move`
   --> $DIR/ref-binding-on-inh-ref-errors.rs:45:11
@@ -95,10 +99,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
    |
 LL |     let [&ref x] = &mut [&mut 0];
    |         ^^^^^^^^ this matches on type `&mut _`
-help: make the implied reference pattern explicit
+help: rewrite the pattern
+   |
+LL -     let [&ref x] = &mut [&mut 0];
+LL +     let &mut [x] = &mut [&mut 0];
    |
-LL |     let &mut [&ref x] = &mut [&mut 0];
-   |         ++++
 
 error: binding modifiers may only be written when the default binding mode is `move`
   --> $DIR/ref-binding-on-inh-ref-errors.rs:54:15

tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.fixed

@@ -175,7 +175,7 @@ fn main() {
     assert_type_eq(c, &0u32);
 
     // Test that we don't change bindings' types when removing reference patterns.
-    let &Foo(&ref a) = &Foo(&0);
+    let &Foo(a) = &Foo(&0);
     //~^ ERROR: reference patterns may only be written when the default binding mode is `move` in Rust 2024
     //~| WARN: this changes meaning in Rust 2024
     assert_type_eq(a, &0u32);
@@ -221,7 +221,7 @@ fn main() {
     assert_type_eq(b, 0u32);
 
     // Test that we respect bindings' subpatterns' types when rewriting `&ref x` to `x`.
-    let [&Foo(&ref a @ ref b), &Foo(&ref c @ d)] = [&Foo(&0); 2];
+    let [&Foo(a @ b), &Foo(&ref c @ d)] = [&Foo(&0); 2];
     //~^ ERROR: reference patterns may only be written when the default binding mode is `move` in Rust 2024
     //~| WARN: this changes meaning in Rust 2024
     assert_type_eq(a, &0u32);
@@ -230,7 +230,7 @@ fn main() {
     assert_type_eq(d, 0u32);
 
     // Test that we respect bindings' subpatterns' modes when rewriting `&ref x` to `x`.
-    let [&Foo(&ref a @ [ref b]), &Foo(&ref c @ [d])] = [&Foo(&[0]); 2];
+    let [&Foo(a @ [b]), &Foo(&ref c @ [d])] = [&Foo(&[0]); 2];
     //~^ ERROR: reference patterns may only be written when the default binding mode is `move` in Rust 2024
     //~| WARN: this changes meaning in Rust 2024
     assert_type_eq(a, &[0u32]);

tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.stderr

@@ -419,10 +419,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
    |
 LL |     let Foo(&ref a) = &Foo(&0);
    |         ^^^^^^^^^^^ this matches on type `&_`
-help: make the implied reference pattern explicit
+help: rewrite the pattern
+   |
+LL -     let Foo(&ref a) = &Foo(&0);
+LL +     let &Foo(a) = &Foo(&0);
    |
-LL |     let &Foo(&ref a) = &Foo(&0);
-   |         +
 
 error: reference patterns may only be written when the default binding mode is `move` in Rust 2024
   --> $DIR/migration_lint.rs:184:10
@@ -537,10 +538,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
    |
 LL |     let [Foo(&ref a @ ref b), Foo(&ref c @ d)] = [&Foo(&0); 2];
    |          ^^^^^^^^^^^^^^^^^^^ this matches on type `&_`
-help: make the implied reference patterns explicit
+help: rewrite the pattern
+   |
+LL -     let [Foo(&ref a @ ref b), Foo(&ref c @ d)] = [&Foo(&0); 2];
+LL +     let [&Foo(a @ b), &Foo(&ref c @ d)] = [&Foo(&0); 2];
    |
-LL |     let [&Foo(&ref a @ ref b), &Foo(&ref c @ d)] = [&Foo(&0); 2];
-   |          +                     +
 
 error: reference patterns may only be written when the default binding mode is `move` in Rust 2024
   --> $DIR/migration_lint.rs:233:14
@@ -562,10 +564,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
    |
 LL |     let [Foo(&ref a @ [ref b]), Foo(&ref c @ [d])] = [&Foo(&[0]); 2];
    |          ^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_`
-help: make the implied reference patterns explicit
+help: rewrite the pattern
+   |
+LL -     let [Foo(&ref a @ [ref b]), Foo(&ref c @ [d])] = [&Foo(&[0]); 2];
+LL +     let [&Foo(a @ [b]), &Foo(&ref c @ [d])] = [&Foo(&[0]); 2];
    |
-LL |     let [&Foo(&ref a @ [ref b]), &Foo(&ref c @ [d])] = [&Foo(&[0]); 2];
-   |          +                       +
 
 error: binding modifiers may only be written when the default binding mode is `move`
   --> $DIR/migration_lint.rs:242:10