try to suggest eliding redundant binding modifiers

Author: dianne

Diff for: compiler/rustc_hir_typeck/src/pat.rs

@@ -2786,7 +2786,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         let mut typeck_results = self.typeck_results.borrow_mut();
         let mut table = typeck_results.rust_2024_migration_desugared_pats_mut();
-        let info = table.entry(pat_id).or_default();
+        // FIXME(ref_pat_eat_one_layer_2024): The migration diagnostic doesn't know how to track the
+        // default binding mode in the presence of Rule 3 or Rule 5. As a consequence, the labels it
+        // gives for default binding modes are wrong, as well as suggestions based on the default
+        // binding mode. This keeps it from making those suggestions, as doing so could panic.
+        let info = table.entry(pat_id).or_insert_with(|| ty::Rust2024IncompatiblePatInfo {
+            primary_labels: Vec::new(),
+            bad_modifiers: false,
+            bad_ref_pats: false,
+            suggest_eliding_modes: !self.tcx.features().ref_pat_eat_one_layer_2024()
+                && !self.tcx.features().ref_pat_eat_one_layer_2024_structural(),
+        });
 
         // Only provide a detailed label if the problematic subpattern isn't from an expansion.
         // In the case that it's from a macro, we'll add a more detailed note in the emitter.
@@ -2797,11 +2807,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // so, we may want to inspect the span's source callee or macro backtrace.
             "occurs within macro expansion".to_owned()
         } else {
-            let pat_kind = if matches!(subpat.kind, PatKind::Binding(_, _, _, _)) {
+            let pat_kind = if let PatKind::Binding(user_bind_annot, _, _, _) = subpat.kind {
                 info.bad_modifiers |= true;
+                // If the user-provided binding modifier doesn't match the default binding mode, we'll
+                // need to suggest reference patterns, which can affect other bindings.
+                // For simplicity, we opt to suggest making the pattern fully explicit.
+                info.suggest_eliding_modes &=
+                    user_bind_annot == BindingMode(ByRef::Yes(def_br_mutbl), Mutability::Not);
                 "binding modifier"
             } else {
                 info.bad_ref_pats |= true;
+                // For simplicity, we don't try to suggest eliding reference patterns. Thus, we'll
+                // suggest adding them instead, which can affect the types assigned to bindings.
+                // As such, we opt to suggest making the pattern fully explicit.
+                info.suggest_eliding_modes = false;
                 "reference pattern"
             };
             let dbm_str = match def_br_mutbl {

Diff for: compiler/rustc_middle/src/ty/typeck_results.rs

@@ -814,12 +814,14 @@ impl<'tcx> std::fmt::Display for UserTypeKind<'tcx> {
 
 /// Information on a pattern incompatible with Rust 2024, for use by the error/migration diagnostic
 /// emitted during THIR construction.
-#[derive(TyEncodable, TyDecodable, Debug, HashStable, Default)]
+#[derive(TyEncodable, TyDecodable, Debug, HashStable)]
 pub struct Rust2024IncompatiblePatInfo {
     /// Labeled spans for `&`s, `&mut`s, and binding modifiers incompatible with Rust 2024.
     pub primary_labels: Vec<(Span, String)>,
     /// Whether any binding modifiers occur under a non-`move` default binding mode.
     pub bad_modifiers: bool,
     /// Whether any `&` or `&mut` patterns occur under a non-`move` default binding mode.
     pub bad_ref_pats: bool,
+    /// If `true`, we can give a simpler suggestion solely by eliding explicit binding modifiers.
+    pub suggest_eliding_modes: bool,
 }

Diff for: compiler/rustc_mir_build/src/errors.rs

@@ -1107,6 +1107,9 @@ pub(crate) struct Rust2024IncompatiblePat {
 }
 
 pub(crate) struct Rust2024IncompatiblePatSugg {
+    /// If true, our suggestion is to elide explicit binding modifiers.
+    /// If false, our suggestion is to make the pattern fully explicit.
+    pub(crate) suggest_eliding_modes: bool,
     pub(crate) suggestion: Vec<(Span, String)>,
     pub(crate) ref_pattern_count: usize,
     pub(crate) binding_mode_count: usize,
@@ -1144,16 +1147,18 @@ impl Subdiagnostic for Rust2024IncompatiblePatSugg {
             } else {
                 Applicability::MaybeIncorrect
             };
-        let plural_derefs = pluralize!(self.ref_pattern_count);
-        let and_modes = if self.binding_mode_count > 0 {
-            format!(" and variable binding mode{}", pluralize!(self.binding_mode_count))
+        let msg = if self.suggest_eliding_modes {
+            let plural_modes = pluralize!(self.binding_mode_count);
+            format!("remove the unnecessary binding modifier{plural_modes}")
         } else {
-            String::new()
+            let plural_derefs = pluralize!(self.ref_pattern_count);
+            let and_modes = if self.binding_mode_count > 0 {
+                format!(" and variable binding mode{}", pluralize!(self.binding_mode_count))
+            } else {
+                String::new()
+            };
+            format!("make the implied reference pattern{plural_derefs}{and_modes} explicit")
         };
-        diag.multipart_suggestion_verbose(
-            format!("make the implied reference pattern{plural_derefs}{and_modes} explicit"),
-            self.suggestion,
-            applicability,
-        );
+        diag.multipart_suggestion_verbose(msg, self.suggestion, applicability);
     }
 }

Diff for: compiler/rustc_mir_build/src/thir/pattern/mod.rs

@@ -49,13 +49,16 @@ pub(super) fn pat_from_hir<'a, 'tcx>(
         tcx,
         typing_env,
         typeck_results,
-        rust_2024_migration_suggestion: migration_info.and(Some(Rust2024IncompatiblePatSugg {
-            suggestion: Vec::new(),
-            ref_pattern_count: 0,
-            binding_mode_count: 0,
-            default_mode_span: None,
-            default_mode_labels: Default::default(),
-        })),
+        rust_2024_migration_suggestion: migration_info.and_then(|info| {
+            Some(Rust2024IncompatiblePatSugg {
+                suggest_eliding_modes: info.suggest_eliding_modes,
+                suggestion: Vec::new(),
+                ref_pattern_count: 0,
+                binding_mode_count: 0,
+                default_mode_span: None,
+                default_mode_labels: Default::default(),
+            })
+        }),
     };
     let result = pcx.lower_pattern(pat);
     debug!("pat_from_hir({:?}) = {:?}", pat, result);
@@ -107,27 +110,22 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
         if let Some(s) = &mut self.rust_2024_migration_suggestion
             && !adjustments.is_empty()
         {
-            let mut min_mutbl = Mutability::Mut;
-            let suggestion_str: String = adjustments
-                .iter()
-                .map(|ref_ty| {
-                    let &ty::Ref(_, _, mutbl) = ref_ty.kind() else {
-                        span_bug!(pat.span, "pattern implicitly dereferences a non-ref type");
-                    };
-
-                    match mutbl {
-                        Mutability::Not => {
-                            min_mutbl = Mutability::Not;
-                            "&"
-                        }
-                        Mutability::Mut => "&mut ",
-                    }
-                })
-                .collect();
-            s.suggestion.push((pat.span.shrink_to_lo(), suggestion_str));
-            s.ref_pattern_count += adjustments.len();
+            let implicit_deref_mutbls = adjustments.iter().map(|ref_ty| {
+                let &ty::Ref(_, _, mutbl) = ref_ty.kind() else {
+                    span_bug!(pat.span, "pattern implicitly dereferences a non-ref type");
+                };
+                mutbl
+            });
+
+            if !s.suggest_eliding_modes {
+                let suggestion_str: String =
+                    implicit_deref_mutbls.clone().map(|mutbl| mutbl.ref_prefix_str()).collect();
+                s.suggestion.push((pat.span.shrink_to_lo(), suggestion_str));
+                s.ref_pattern_count += adjustments.len();
+            }
 
             // Remember if this changed the default binding mode, in case we want to label it.
+            let min_mutbl = implicit_deref_mutbls.min().unwrap();
             if s.default_mode_span.is_none_or(|(_, old_mutbl)| min_mutbl < old_mutbl) {
                 opt_old_mode_span = Some(s.default_mode_span);
                 s.default_mode_span = Some((pat.span, min_mutbl));
@@ -413,8 +411,14 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
                     {
                         // If this overrides a by-ref default binding mode, label the binding mode.
                         s.default_mode_labels.insert(default_mode_span, default_ref_mutbl);
+                        // If our suggestion is to elide redundnt modes, this will be one of them.
+                        if s.suggest_eliding_modes {
+                            s.suggestion.push((pat.span.with_hi(ident.span.lo()), String::new()));
+                            s.binding_mode_count += 1;
+                        }
                     }
-                    if explicit_ba.0 == ByRef::No
+                    if !s.suggest_eliding_modes
+                        && explicit_ba.0 == ByRef::No
                         && let ByRef::Yes(mutbl) = mode.0
                     {
                         let sugg_str = match mutbl {

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

@@ -32,7 +32,7 @@ fn main() {
     //~| WARN: this changes meaning in Rust 2024
     assert_type_eq(x, 0u8);
 
-    let &Foo(ref x) = &Foo(0);
+    let Foo(x) = &Foo(0);
     //~^ ERROR: binding modifiers may only be written when the default binding mode is `move` in Rust 2024
     //~| WARN: this changes meaning in Rust 2024
     assert_type_eq(x, &0u8);

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

@@ -52,10 +52,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
    |
 LL |     let Foo(ref x) = &Foo(0);
    |         ^^^^^^^^^^ this matches on type `&_`
-help: make the implied reference pattern explicit
+help: remove the unnecessary binding modifier
+   |
+LL -     let Foo(ref x) = &Foo(0);
+LL +     let Foo(x) = &Foo(0);
    |
-LL |     let &Foo(ref x) = &Foo(0);
-   |         +
 
 error: binding modifiers may only be written when the default binding mode is `move` in Rust 2024
   --> $DIR/migration_lint.rs:40:13

Diff for: tests/ui/pattern/rfc-3627-match-ergonomics-2024/min_match_ergonomics_fail.stderr

@@ -179,10 +179,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
    |
 LL | test_pat_on_type![(ref x,): &(T,)];
    |                   ^^^^^^^^ this matches on type `&_`
-help: make the implied reference pattern explicit
+help: remove the unnecessary binding modifier
+   |
+LL - test_pat_on_type![(ref x,): &(T,)];
+LL + test_pat_on_type![(x,): &(T,)];
    |
-LL | test_pat_on_type![&(ref x,): &(T,)];
-   |                   +
 
 error: binding modifiers may only be written when the default binding mode is `move`
   --> $DIR/min_match_ergonomics_fail.rs:34:20
@@ -196,10 +197,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
    |
 LL | test_pat_on_type![(ref mut x,): &mut (T,)];
    |                   ^^^^^^^^^^^^ this matches on type `&mut _`
-help: make the implied reference pattern explicit
+help: remove the unnecessary binding modifier
+   |
+LL - test_pat_on_type![(ref mut x,): &mut (T,)];
+LL + test_pat_on_type![(x,): &mut (T,)];
    |
-LL | test_pat_on_type![&mut (ref mut x,): &mut (T,)];
-   |                   ++++
 
 error: reference patterns may only be written when the default binding mode is `move`
   --> $DIR/min_match_ergonomics_fail.rs:43:10