experimentally label the spans for default binding modes

dianne · dianne · commit 6ff2d19d1ca7 · 2025-02-10T23:32:54.000-08:00
(cherry picked from commit 203d310)
diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs
@@ -699,7 +699,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         // Determine the binding mode...
         let bm = match user_bind_annot {
-            BindingMode(ByRef::No, Mutability::Mut) if let ByRef::Yes(def_br_mutbl) = def_br => {
+            BindingMode(ByRef::No, Mutability::Mut) if matches!(def_br, ByRef::Yes(_)) => {
                 if pat.span.at_least_rust_2024()
                     && (self.tcx.features().ref_pat_eat_one_layer_2024()
                         || self.tcx.features().ref_pat_eat_one_layer_2024_structural())
@@ -721,20 +721,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         pat_info.top_info.hir_id,
                         pat,
                         ident.span,
-                        def_br_mutbl,
                     );
                     BindingMode(ByRef::No, Mutability::Mut)
                 }
             }
             BindingMode(ByRef::No, mutbl) => BindingMode(def_br, mutbl),
             BindingMode(ByRef::Yes(_), _) => {
-                if let ByRef::Yes(def_br_mutbl) = def_br {
+                if matches!(def_br, ByRef::Yes(_)) {
                     // `ref`/`ref mut` overrides the binding mode on edition <= 2021
                     self.add_rust_2024_migration_desugared_pat(
                         pat_info.top_info.hir_id,
                         pat,
                         ident.span,
-                        def_br_mutbl,
                     );
                 }
                 user_bind_annot
@@ -2263,13 +2261,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             }
         } else {
             // Reset binding mode on old editions
-            if let ByRef::Yes(inh_mut) = pat_info.binding_mode {
+            if pat_info.binding_mode != ByRef::No {
                 pat_info.binding_mode = ByRef::No;
                 self.add_rust_2024_migration_desugared_pat(
                     pat_info.top_info.hir_id,
                     pat,
                     inner.span,
-                    inh_mut,
                 )
             }
         }
@@ -2637,7 +2634,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         pat_id: HirId,
         subpat: &'tcx Pat<'tcx>,
         cutoff_span: Span,
-        def_br_mutbl: Mutability,
     ) {
         // Try to trim the span we're labeling to just the `&` or binding mode that's an issue.
         // If the subpattern's span is is from an expansion, the emitted label will not be trimmed.
@@ -2653,8 +2649,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let mut table = typeck_results.rust_2024_migration_desugared_pats_mut();
         let info = table.entry(pat_id).or_default();
 
-        info.primary_spans.push(trimmed_span);
-
         // 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.
         let from_expansion = subpat.span.from_expansion();
@@ -2672,15 +2666,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 "this reference pattern"
             }
         };
-        info.span_labels.push((trimmed_span, primary_label.to_owned()));
-
-        if !from_expansion {
-            // Add a secondary label covering the whole pattern noting the default binding mode
-            let def_br_desc = match def_br_mutbl {
-                Mutability::Not => "default binding mode is `ref`",
-                Mutability::Mut => "default binding mode is `ref mut`",
-            };
-            info.span_labels.push((subpat.span, def_br_desc.to_owned()));
-        }
+        info.primary_labels.push((trimmed_span, primary_label.to_owned()));
     }
 }
diff --git a/compiler/rustc_middle/src/ty/typeck_results.rs b/compiler/rustc_middle/src/ty/typeck_results.rs
@@ -816,10 +816,8 @@ impl<'tcx> std::fmt::Display for UserTypeKind<'tcx> {
 /// emitted during THIR construction.
 #[derive(TyEncodable, TyDecodable, Debug, HashStable, Default)]
 pub struct Rust2024IncompatiblePatInfo {
-    /// Spans for `&`s, `&mut`s, and binding modifiers incompatible with Rust 2024.
-    pub primary_spans: Vec<Span>,
-    /// Labels for the primary spans and their patterns, to provide additional context.
-    pub span_labels: Vec<(Span, String)>,
+    /// 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.
diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs
@@ -1,3 +1,4 @@
+use rustc_data_structures::fx::FxIndexMap;
 use rustc_errors::codes::*;
 use rustc_errors::{
     Applicability, Diag, DiagArgValue, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level,
@@ -1100,6 +1101,11 @@ pub(crate) struct Rust2024IncompatiblePatSugg {
     pub(crate) suggestion: Vec<(Span, String)>,
     pub(crate) ref_pattern_count: usize,
     pub(crate) binding_mode_count: usize,
+    /// Internal state: the ref-mutability of the default binding mode at the subpattern being
+    /// lowered, with the span where it was introduced. `None` for a by-value default mode.
+    pub(crate) default_mode_span: Option<(Span, ty::Mutability)>,
+    /// Labels for where incompatibility-causing by-ref default binding modes were introduced.
+    pub(crate) default_mode_labels: FxIndexMap<Span, ty::Mutability>,
 }
 
 impl Subdiagnostic for Rust2024IncompatiblePatSugg {
diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs
@@ -53,16 +53,29 @@ pub(super) fn pat_from_hir<'a, 'tcx>(
             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);
     if let Some(info) = migration_info {
         let sugg = pcx.rust_2024_migration_suggestion.expect("suggestion should be present");
-        let mut spans = MultiSpan::from_spans(info.primary_spans.clone());
-        for (span, label) in &info.span_labels {
+        let mut spans =
+            MultiSpan::from_spans(info.primary_labels.iter().map(|(span, _)| *span).collect());
+        for (span, label) in &info.primary_labels {
             spans.push_span_label(*span, label.clone());
         }
+        for (span, label_mutbl) in &sugg.default_mode_labels {
+            // Don't point to a macro call site.
+            if !span.from_expansion() {
+                let label = match label_mutbl {
+                    Mutability::Not => "default binding mode is `ref`",
+                    Mutability::Mut => "default binding mode is `ref mut`",
+                };
+                spans.push_span_label(*span, label.to_owned())
+            }
+        }
         // 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 {
@@ -96,6 +109,40 @@ pub(super) fn pat_from_hir<'a, 'tcx>(
 
 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);
+
+        let mut opt_old_mode_span = None;
+        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();
+
+            // Remember if this changed the default binding mode, in case we want to label it.
+            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));
+            }
+        };
+
         // 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:
         //
@@ -124,8 +171,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 {
@@ -136,24 +181,10 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
         });
 
         if let Some(s) = &mut self.rust_2024_migration_suggestion
-            && !adjustments.is_empty()
+            && let Some(old_mode_span) = opt_old_mode_span
         {
-            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 {
-                        ty::Mutability::Not => "&",
-                        ty::Mutability::Mut => "&mut ",
-                    }
-                })
-                .collect();
-            s.suggestion.push((pat.span.shrink_to_lo(), suggestion_str));
-            s.ref_pattern_count += adjustments.len();
-        };
+            s.default_mode_span = old_mode_span;
+        }
 
         adjusted_pat
     }
@@ -353,7 +384,22 @@ 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, _) => {
+                // Track the default binding mode for the Rust 2024 migration suggestion.
+                let old_mode_span = self.rust_2024_migration_suggestion.as_mut().and_then(|s| {
+                    if let Some((default_mode_span, default_ref_mutbl)) = s.default_mode_span {
+                        // If this eats a by-ref default binding mode, label the binding mode.
+                        s.default_mode_labels.insert(default_mode_span, default_ref_mutbl);
+                    }
+                    s.default_mode_span.take()
+                });
+                let subpattern = self.lower_pattern(subpattern);
+                if let Some(s) = &mut self.rust_2024_migration_suggestion {
+                    s.default_mode_span = old_mode_span;
+                }
+                PatKind::Deref { subpattern }
+            }
+            hir::PatKind::Box(subpattern) => {
                 PatKind::Deref { subpattern: self.lower_pattern(subpattern) }
             }
 
@@ -380,19 +426,26 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
                     .get(pat.hir_id)
                     .expect("missing binding mode");
 
-                if let Some(s) = &mut self.rust_2024_migration_suggestion
-                    && explicit_ba.0 == ByRef::No
-                    && let ByRef::Yes(mutbl) = mode.0
-                {
-                    let sugg_str = match mutbl {
-                        Mutability::Not => "ref ",
-                        Mutability::Mut => "ref mut ",
-                    };
-                    s.suggestion.push((
-                        pat.span.with_lo(ident.span.lo()).shrink_to_lo(),
-                        sugg_str.to_owned(),
-                    ));
-                    s.binding_mode_count += 1;
+                if let Some(s) = &mut self.rust_2024_migration_suggestion {
+                    if explicit_ba != hir::BindingMode::NONE
+                        && let Some((default_mode_span, default_ref_mutbl)) = s.default_mode_span
+                    {
+                        // 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 explicit_ba.0 == ByRef::No
+                        && let ByRef::Yes(mutbl) = mode.0
+                    {
+                        let sugg_str = match mutbl {
+                            Mutability::Not => "ref ",
+                            Mutability::Mut => "ref mut ",
+                        };
+                        s.suggestion.push((
+                            pat.span.with_lo(ident.span.lo()).shrink_to_lo(),
+                            sugg_str.to_owned(),
+                        ));
+                        s.binding_mode_count += 1;
+                    }
                 }
 
                 // A ref x pattern is the same node used for x, and as such it has
diff --git a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.stderr b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.stderr
diff --git a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/min_match_ergonomics_fail.stderr b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/min_match_ergonomics_fail.stderr
