pattern migration beautification, part 1: beautify in simple cases

This makes it so that if we can suggest removing all of the explicit
reference patterns and binding modes from a pattern to migrate it, we do
that. Otherwise, we use the old behavior of making everything explicit.
In a later commit, this will be extended (and partially rewritten) to
handle cases where we can omit some reference patterns to avoid
specifying some binding modes, but not all.

Author: dianne

compiler/rustc_mir_build/src/errors.rs

@@ -1104,10 +1104,16 @@ pub(crate) struct Rust2024IncompatiblePat {
 
 pub(crate) struct Rust2024IncompatiblePatSugg {
     pub(crate) suggestion: Vec<(Span, String)>,
+    pub(crate) kind: Rust2024IncompatiblePatSuggKind,
     pub(crate) ref_pattern_count: usize,
     pub(crate) binding_mode_count: usize,
 }
 
+pub(crate) enum Rust2024IncompatiblePatSuggKind {
+    Subtractive,
+    Additive,
+}
+
 impl Subdiagnostic for Rust2024IncompatiblePatSugg {
     fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
         self,
@@ -1120,14 +1126,23 @@ 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 derefs = if self.ref_pattern_count > 0 {
+            format!("reference pattern{}", pluralize!(self.ref_pattern_count))
+        } else {
+            String::new()
+        };
+        let modes = if self.binding_mode_count > 0 {
+            format!("variable binding mode{}", pluralize!(self.binding_mode_count))
         } else {
             String::new()
         };
+        let and = if !derefs.is_empty() && !modes.is_empty() { " and " } else { "" };
+        let (old_visibility, new_visibility) = match self.kind {
+            Rust2024IncompatiblePatSuggKind::Subtractive => ("", "implicit"),
+            Rust2024IncompatiblePatSuggKind::Additive => ("implied ", "explicit"),
+        };
         diag.multipart_suggestion_verbose(
-            format!("make the implied reference pattern{plural_derefs}{and_modes} explicit"),
+            format!("make the {old_visibility}{derefs}{and}{modes} {new_visibility}"),
             self.suggestion,
             applicability,
         );

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

@@ -2,47 +2,64 @@
 
 use rustc_ast::BindingMode;
 use rustc_errors::MultiSpan;
-use rustc_hir::{ByRef, HirId, Mutability};
+use rustc_hir::{self as hir, ByRef, HirId, Mutability};
 use rustc_lint as lint;
 use rustc_middle::span_bug;
-use rustc_middle::ty::{self, Ty, TyCtxt};
+use rustc_middle::ty::{self, Ty, TyCtxt, TypeckResults};
 use rustc_span::{Ident, Span};
 
-use crate::errors::{Rust2024IncompatiblePat, Rust2024IncompatiblePatSugg};
+use crate::errors::*;
 use crate::fluent_generated as fluent;
 
 /// For patterns flagged for migration during HIR typeck, this handles constructing and emitting
 /// a diagnostic suggestion.
 pub(super) struct PatMigration<'a> {
-    suggestion: Vec<(Span, String)>,
-    ref_pattern_count: usize,
-    binding_mode_count: usize,
+    /// `&` and `&mut` patterns we may need to suggest removing.
+    explicit_derefs: Vec<Span>,
+    /// Variable binding modes we may need to suggest making implicit.
+    explicit_modes: Vec<Span>,
+    /// Implicit dereferences we may need to suggest adding `&` or `&mut` patterns for, together
+    /// with the HIR id of the pattern where they occur, for formatting.
+    implicit_derefs: Vec<(Span, HirId)>,
+    /// Implicit by-reference binding modes we may need to suggest making explicit.
+    implicit_modes: Vec<(Span, Mutability)>,
+    /// How many references deep is the current pattern? For determining default binding mode.
+    current_ref_depth: usize,
+    /// How many references deep is the binding mode able to be `ref mut`?
+    current_max_ref_mut_depth: usize,
+    /// Whether we can suggest making derefs and binding modes implicit rather than explicit.
+    can_suggest_removing: bool,
     /// Labeled spans for subpatterns invalid in Rust 2024.
     labels: &'a [(Span, String)],
 }
 
 impl<'a> PatMigration<'a> {
     pub(super) fn new(labels: &'a Vec<(Span, String)>) -> Self {
         PatMigration {
-            suggestion: Vec::new(),
-            ref_pattern_count: 0,
-            binding_mode_count: 0,
+            explicit_derefs: Vec::new(),
+            explicit_modes: Vec::new(),
+            implicit_derefs: Vec::new(),
+            implicit_modes: Vec::new(),
+            current_ref_depth: 0,
+            current_max_ref_mut_depth: 0,
+            can_suggest_removing: true,
             labels: labels.as_slice(),
         }
     }
 
     /// On Rust 2024, this emits a hard error. On earlier Editions, this emits the
     /// future-incompatibility lint `rust_2024_incompatible_pat`.
-    pub(super) fn emit<'tcx>(self, tcx: TyCtxt<'tcx>, pat_id: HirId) {
+    pub(super) fn emit<'tcx>(
+        self,
+        tcx: TyCtxt<'tcx>,
+        typeck_results: &'a TypeckResults<'tcx>,
+        pat_id: HirId,
+    ) {
         let mut spans = MultiSpan::from_spans(self.labels.iter().map(|(span, _)| *span).collect());
         for (span, label) in self.labels {
             spans.push_span_label(*span, label.clone());
         }
-        let sugg = Rust2024IncompatiblePatSugg {
-            suggestion: self.suggestion,
-            ref_pattern_count: self.ref_pattern_count,
-            binding_mode_count: self.binding_mode_count,
-        };
+        let sugg = self.build_suggestion(typeck_results);
         // If a relevant span is from at least edition 2024, this is a hard error.
         let is_hard_error = spans.primary_spans().iter().any(|span| span.at_least_rust_2024());
         if is_hard_error {
@@ -64,38 +81,159 @@ impl<'a> PatMigration<'a> {
         }
     }
 
-    pub(super) fn visit_implicit_derefs<'tcx>(&mut self, pat_span: Span, adjustments: &[Ty<'tcx>]) {
-        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");
+    fn build_suggestion(
+        &self,
+        typeck_results: &'a TypeckResults<'_>,
+    ) -> Rust2024IncompatiblePatSugg {
+        if self.can_suggest_removing {
+            // We can suggest a simple pattern by removing all explicit derefs and binding modes.
+            let suggestion = self
+                .explicit_modes
+                .iter()
+                .chain(&self.explicit_derefs)
+                .map(|&removed_sp| (removed_sp, String::new()))
+                .collect();
+            Rust2024IncompatiblePatSugg {
+                suggestion,
+                kind: Rust2024IncompatiblePatSuggKind::Subtractive,
+                ref_pattern_count: self.explicit_derefs.len(),
+                binding_mode_count: self.explicit_modes.len(),
+            }
+        } else {
+            // We can't suggest a simple pattern, so fully elaborate the pattern's match ergonomics.
+            let modes = self.implicit_modes.iter().map(|&(sp, mutbl)| {
+                let sugg_str = match mutbl {
+                    Mutability::Not => "ref ",
+                    Mutability::Mut => "ref mut ",
                 };
+                (sp, sugg_str.to_owned())
+            });
+            let mut ref_pattern_count = 0;
+            let derefs = self.implicit_derefs.iter().map(|&(sp, hir_id)| {
+                let adjustments = typeck_results.pat_adjustments().get(hir_id).unwrap();
+                let ref_pat_str = adjustments
+                    .iter()
+                    .map(|ref_ty| {
+                        let &ty::Ref(_, _, mutbl) = ref_ty.kind() else {
+                            span_bug!(sp, "pattern implicitly dereferences a non-ref type");
+                        };
+
+                        mutbl.ref_prefix_str()
+                    })
+                    .collect();
+                ref_pattern_count += adjustments.len();
+                (sp.shrink_to_lo(), ref_pat_str)
+            });
+            Rust2024IncompatiblePatSugg {
+                suggestion: modes.chain(derefs).collect(),
+                kind: Rust2024IncompatiblePatSuggKind::Additive,
+                ref_pattern_count,
+                binding_mode_count: self.implicit_modes.len(),
+            }
+        }
+    }
+
+    /// The default binding mode at the current pattern, if all reference patterns were removed.
+    fn default_mode(&self) -> ByRef {
+        if self.current_ref_depth == 0 {
+            ByRef::No
+        } else {
+            // If all `&` and `&mut` patterns are removed, the default binding mode's reference
+            // mutability is mutable if and only if there are only `&mut` reference types.
+            // See `FnCtxt::peel_off_references` in `rustc_hir_typeck::pat` for more information.
+            let mutbl = if self.current_max_ref_mut_depth == self.current_ref_depth {
+                Mutability::Mut
+            } else {
+                Mutability::Not
+            };
+            ByRef::Yes(mutbl)
+        }
+    }
+
+    /// Tracks when we're lowering a `&` or `&mut` pattern and adjusts the suggestion if necessary.
+    /// This should be followed by a call to [`PatMigration::leave_ref`] when we leave the pattern.
+    pub(super) fn visit_explicit_deref<'tcx>(
+        &mut self,
+        pat_span: Span,
+        mutbl: Mutability,
+        subpat: &'tcx hir::Pat<'tcx>,
+    ) {
+        self.explicit_derefs.push(pat_span.with_hi(subpat.span.lo()));
+
+        // If the immediate subpattern is a binding, removing this reference pattern would change
+        // its type, so we opt not to remove any, for simplicity.
+        // 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.
+        if matches!(subpat.kind, hir::PatKind::Binding(_, _, _, _)) {
+            self.can_suggest_removing = false;
+        }
 
-                mutbl.ref_prefix_str()
+        // Keep track of the reference depth for determining the default binding mode.
+        if self.current_max_ref_mut_depth == self.current_ref_depth && mutbl.is_mut() {
+            self.current_max_ref_mut_depth += 1;
+        }
+        self.current_ref_depth += 1;
+    }
+
+    /// Tracks when we're lowering a pattern that implicitly dereferences the scrutinee.
+    /// This should be followed by a call to [`PatMigration::leave_ref`] when we leave the pattern.
+    pub(super) fn visit_implicit_derefs<'tcx>(
+        &mut self,
+        pat: &'tcx hir::Pat<'tcx>,
+        adjustments: &[Ty<'tcx>],
+    ) {
+        self.implicit_derefs.push((pat.span, pat.hir_id));
+
+        // Keep track of the reference depth for determining the default binding mode.
+        if self.current_max_ref_mut_depth == self.current_ref_depth
+            && adjustments.iter().all(|ref_ty| {
+                let &ty::Ref(_, _, mutbl) = ref_ty.kind() else {
+                    span_bug!(pat.span, "pattern implicitly dereferences a non-ref type");
+                };
+                mutbl.is_mut()
             })
-            .collect();
-        self.suggestion.push((pat_span.shrink_to_lo(), suggestion_str));
-        self.ref_pattern_count += adjustments.len();
+        {
+            self.current_max_ref_mut_depth += 1;
+        }
+        self.current_ref_depth += 1;
+    }
+
+    /// Tracks when we leave a reference (either implicitly or explicitly derefed) while lowering.
+    /// This should follow a call to [`PatMigration::visit_explicit_deref`] or
+    /// [`PatMigration::visit_implicit_derefs`].
+    pub(super) fn leave_ref(&mut self) {
+        if self.current_max_ref_mut_depth == self.current_ref_depth {
+            self.current_max_ref_mut_depth -= 1;
+        }
+        self.current_ref_depth -= 1;
     }
 
+    /// Keeps track of bindings and adjusts the suggestion if necessary.
     pub(super) fn visit_binding(
         &mut self,
         pat_span: Span,
         mode: BindingMode,
         explicit_ba: BindingMode,
         ident: Ident,
     ) {
+        // Note any binding modes we may need to make explicit.
         if explicit_ba.0 == ByRef::No
             && let ByRef::Yes(mutbl) = mode.0
         {
-            let sugg_str = match mutbl {
-                Mutability::Not => "ref ",
-                Mutability::Mut => "ref mut ",
-            };
-            self.suggestion
-                .push((pat_span.with_lo(ident.span.lo()).shrink_to_lo(), sugg_str.to_owned()));
-            self.binding_mode_count += 1;
+            self.implicit_modes.push((ident.span.shrink_to_lo(), mutbl));
+        }
+
+        if self.can_suggest_removing {
+            if mode == BindingMode(self.default_mode(), Mutability::Not) {
+                // Note any binding modes we may need to make implicit.
+                if matches!(explicit_ba.0, ByRef::Yes(_)) {
+                    self.explicit_modes.push(pat_span.with_hi(ident.span.lo()))
+                }
+            } else {
+                // If removing reference patterns would change the mode of this binding, we opt not
+                // to remove any, for simplicity.
+                self.can_suggest_removing = false;
+            }
         }
     }
 }

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

@@ -55,13 +55,22 @@ pub(super) fn pat_from_hir<'a, 'tcx>(
     let result = pcx.lower_pattern(pat);
     debug!("pat_from_hir({:?}) = {:?}", pat, result);
     if let Some(m) = pcx.rust_2024_migration {
-        m.emit(tcx, pat.hir_id);
+        m.emit(tcx, typeck_results, pat.hir_id);
     }
     result
 }
 
 impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
     fn lower_pattern(&mut self, pat: &'tcx hir::Pat<'tcx>) -> Box<Pat<'tcx>> {
+        let adjustments: &[Ty<'tcx>] =
+            self.typeck_results.pat_adjustments().get(pat.hir_id).map_or(&[], |v| &**v);
+
+        if let Some(m) = &mut self.rust_2024_migration
+            && !adjustments.is_empty()
+        {
+            m.visit_implicit_derefs(pat, adjustments);
+        }
+
         // When implicit dereferences have been inserted in this pattern, the unadjusted lowered
         // pattern has the type that results *after* dereferencing. For example, in this code:
         //
@@ -90,8 +99,6 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
             _ => self.lower_pattern_unadjusted(pat),
         };
 
-        let adjustments: &[Ty<'tcx>] =
-            self.typeck_results.pat_adjustments().get(pat.hir_id).map_or(&[], |v| &**v);
         let adjusted_pat = adjustments.iter().rev().fold(unadjusted_pat, |thir_pat, ref_ty| {
             debug!("{:?}: wrapping pattern with type {:?}", thir_pat, ref_ty);
             Box::new(Pat {
@@ -104,7 +111,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
         if let Some(m) = &mut self.rust_2024_migration
             && !adjustments.is_empty()
         {
-            m.visit_implicit_derefs(pat.span, adjustments);
+            m.leave_ref();
         }
 
         adjusted_pat
@@ -295,7 +302,15 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
                 let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not };
                 PatKind::DerefPattern { subpattern: self.lower_pattern(subpattern), mutability }
             }
-            hir::PatKind::Ref(subpattern, _) | hir::PatKind::Box(subpattern) => {
+            hir::PatKind::Ref(subpattern, mutbl) => {
+                if let Some(m) = &mut self.rust_2024_migration {
+                    m.visit_explicit_deref(pat.span, mutbl, subpattern)
+                }
+                let subpattern = self.lower_pattern(subpattern);
+                self.rust_2024_migration.as_mut().map(|m| m.leave_ref());
+                PatKind::Deref { subpattern }
+            }
+            hir::PatKind::Box(subpattern) => {
                 PatKind::Deref { subpattern: self.lower_pattern(subpattern) }
             }
 

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

@@ -35,10 +35,11 @@ LL |     let [ref x] = &[0];
    |          ^^^ cannot override to bind by-reference when that is the implicit default
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
-help: make the implied reference pattern explicit
+help: make the variable binding mode implicit
+   |
+LL -     let [ref x] = &[0];
+LL +     let [x] = &[0];
    |
-LL |     let &[ref x] = &[0];
-   |         +
 
 error: this pattern relies on behavior which may change in edition 2024
   --> $DIR/ref-binding-on-inh-ref-errors.rs:88:10
@@ -59,10 +60,11 @@ LL |     let [ref mut x] = &mut [0];
    |          ^^^^^^^ cannot override to bind by-reference when that is the implicit default
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
-help: make the implied reference pattern explicit
+help: make the variable binding mode implicit
+   |
+LL -     let [ref mut x] = &mut [0];
+LL +     let [x] = &mut [0];
    |
-LL |     let &mut [ref mut x] = &mut [0];
-   |         ++++
 
 error: aborting due to 6 previous errors