Pattern Migration 2024: suggest nicer patterns

compiler/rustc_hir_typeck/src/pat.rs

@@ -835,20 +835,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     self.add_rust_2024_migration_desugared_pat(
                         pat_info.top_info.hir_id,
                         pat,
-                        ident.span,
+                        't',
                         def_br_mutbl,
                     );
                     BindingMode(ByRef::No, Mutability::Mut)
                 }
             }
             BindingMode(ByRef::No, mutbl) => BindingMode(def_br, mutbl),
-            BindingMode(ByRef::Yes(_), _) => {
+            BindingMode(ByRef::Yes(user_br_mutbl), _) => {
                 if let ByRef::Yes(def_br_mutbl) = def_br {
                     // `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,
+                        if user_br_mutbl.is_mut() { 't' } else { 'f' },
                         def_br_mutbl,
                     );
                 }
@@ -2387,7 +2387,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     self.add_rust_2024_migration_desugared_pat(
                         pat_info.top_info.hir_id,
                         pat,
-                        inner.span,
+                        if pat_mutbl.is_mut() { 't' } else { '&' },
                         inh_mut,
                     )
                 }
@@ -2779,55 +2779,38 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         &self,
         pat_id: HirId,
         subpat: &'tcx Pat<'tcx>,
-        cutoff_span: Span,
+        final_char: char,
         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.
-        let source_map = self.tcx.sess.source_map();
-        let cutoff_span = source_map
-            .span_extend_prev_while(cutoff_span, |c| c.is_whitespace() || c == '(')
-            .unwrap_or(cutoff_span);
-        // Ensure we use the syntax context and thus edition of `subpat.span`; this will be a hard
-        // error if the subpattern is of edition >= 2024.
-        let trimmed_span = subpat.span.until(cutoff_span).with_ctxt(subpat.span.ctxt());
+        // Importantly, the edition of the trimmed span should be the same as `subpat.span`; this
+        // will be a hard error if the subpattern is of edition >= 2024.
+        let from_expansion = subpat.span.from_expansion();
+        let trimmed_span = if from_expansion {
+            subpat.span
+        } else {
+            self.tcx.sess.source_map().span_through_char(subpat.span, final_char)
+        };
 
         let mut typeck_results = self.typeck_results.borrow_mut();
         let mut table = typeck_results.rust_2024_migration_desugared_pats_mut();
         // 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(),
-        });
+        // binding mode.
+        let info = table.entry(pat_id).or_default();
 
-        let pat_kind = if let PatKind::Binding(user_bind_annot, _, _, _) = subpat.kind {
+        let pat_kind = if matches!(subpat.kind, PatKind::Binding(..)) {
             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"
         };
         // 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();
         let primary_label = if from_expansion {
-            // We can't suggest eliding modifiers within expansions.
-            info.suggest_eliding_modes = false;
             // NB: This wording assumes the only expansions that can produce problematic reference
             // patterns and bindings are macros. If a desugaring or AST pass is added that can do
             // so, we may want to inspect the span's source callee or macro backtrace.

compiler/rustc_middle/src/ty/typeck_results.rs

@@ -812,14 +812,12 @@ 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)]
+#[derive(TyEncodable, TyDecodable, Debug, Default, 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,
 }

compiler/rustc_mir_build/src/errors.rs

@@ -1097,34 +1097,41 @@ pub(crate) enum MiscPatternSuggestion {
 
 #[derive(LintDiagnostic)]
 #[diag(mir_build_rust_2024_incompatible_pat)]
-pub(crate) struct Rust2024IncompatiblePat {
+pub(crate) struct Rust2024IncompatiblePat<'m> {
     #[subdiagnostic]
-    pub(crate) sugg: Rust2024IncompatiblePatSugg,
+    pub(crate) sugg: Rust2024IncompatiblePatSugg<'m>,
     pub(crate) bad_modifiers: bool,
     pub(crate) bad_ref_pats: bool,
     pub(crate) is_hard_error: bool,
 }
 
-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) struct Rust2024IncompatiblePatSugg<'m> {
     pub(crate) suggestion: Vec<(Span, String)>,
+    /// If `Some(..)`, we provide a suggestion about either adding or removing syntax.
+    /// If `None`, we suggest both additions and removals; use a generic wording for simplicity.
+    pub(crate) kind: Option<Rust2024IncompatiblePatSuggKind>,
     pub(crate) ref_pattern_count: usize,
     pub(crate) binding_mode_count: usize,
     /// Labels for where incompatibility-causing by-ref default binding modes were introduced.
-    pub(crate) default_mode_labels: FxIndexMap<Span, ty::Mutability>,
+    pub(crate) default_mode_labels: &'m FxIndexMap<Span, ty::Mutability>,
 }
 
-impl Subdiagnostic for Rust2024IncompatiblePatSugg {
+pub(crate) enum Rust2024IncompatiblePatSuggKind {
+    Subtractive,
+    Additive,
+}
+
+impl<'m> Subdiagnostic for Rust2024IncompatiblePatSugg<'m> {
     fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
         self,
         diag: &mut Diag<'_, G>,
         _f: &F,
     ) {
+        use Rust2024IncompatiblePatSuggKind::*;
+
         // Format and emit explanatory notes about default binding modes. Reversing the spans' order
         // means if we have nested spans, the innermost ones will be visited first.
-        for (span, def_br_mutbl) in self.default_mode_labels.into_iter().rev() {
+        for (&span, &def_br_mutbl) in self.default_mode_labels.iter().rev() {
             // Don't point to a macro call site.
             if !span.from_expansion() {
                 let note_msg = "matching on a reference type with a non-reference pattern changes the default binding mode";
@@ -1143,17 +1150,33 @@ impl Subdiagnostic for Rust2024IncompatiblePatSugg {
             } else {
                 Applicability::MaybeIncorrect
             };
-        let msg = if self.suggest_eliding_modes {
-            let plural_modes = pluralize!(self.binding_mode_count);
-            format!("remove the unnecessary binding modifier{plural_modes}")
-        } else {
-            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 let Some(kind) = self.kind {
+            let derefs = if self.ref_pattern_count > 0 {
+                format!("reference pattern{}", pluralize!(self.ref_pattern_count))
             } else {
                 String::new()
             };
-            format!("make the implied reference pattern{plural_derefs}{and_modes} explicit")
+            let modes = if self.binding_mode_count > 0 {
+                match kind {
+                    Subtractive => {
+                        format!("binding modifier{}", pluralize!(self.binding_mode_count))
+                    }
+                    Additive => {
+                        format!("variable binding mode{}", pluralize!(self.binding_mode_count))
+                    }
+                }
+            } else {
+                String::new()
+            };
+            let and = if !derefs.is_empty() && !modes.is_empty() { " and " } else { "" };
+            match kind {
+                Subtractive => format!("remove the unnecessary {derefs}{and}{modes}"),
+                Additive => {
+                    format!("make the implied {derefs}{and}{modes} explicit")
+                }
+            }
+        } else {
+            "rewrite the pattern".to_owned()
         };
         // FIXME(dianne): for peace of mind, don't risk emitting a 0-part suggestion (that panics!)
         if !self.suggestion.is_empty() {