Rollup merge of rust-lang#137269 - dianne:fix-ref-pat-label-span, r=davidtwco

Pattern Migration 2024: properly label `&` patterns whose subpatterns are from macro expansions

See the failing test output in the first commit for an example of what this going wrong looks like. The error/lint diagnostic tries to point to just the `&` or `&mut` of reference patterns when labeling the causes, to make the output clearer (rust-lang#134394). The trimming there wasn't quite right though: it used the interior of the reference pattern as a cutoff and extended backwards to find where to trim the pattern's span, but this breaks if the `&` and the interior are from different sources. This PR instead trims by starting at the start of the pattern and ending at the final character of the `&` (or `&mut`, `ref`, `ref mut`, or `mut`, depending on what the error/lint is labeling); that way, there's no opportunity for failure from mixing sources.

I'm not 100% happy with this approach, but I'm also not sure what the best practices are as far as hacky `SourceMap` munching goes, so please let me know if something else would be preferred.

Since `SourceMap::span_through_char` can't change the syntax context of the span, I've also removed a call to `Span::with_ctxt` (we care about the edition of the span in question since this is a hard error in Rust 2024). If we want to be extra safe in case that changes, I can re-add it or track error hardness separately in the `rust_2024_migration_desugared_pats` table.

Author: matthiaskrgr

compiler/rustc_hir_typeck/src/pat.rs

@@ -835,20 +835,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     self.add_rust_2024_migration_desugared_pat(
                         pat_info.top_info.hir_id,
                         pat,
-                        ident.span,
+                        't', // last char of `mut`
                         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,
+                        match user_br_mutbl {
+                            Mutability::Not => 'f', // last char of `ref`
+                            Mutability::Mut => 't', // last char of `ref mut`
+                        },
                         def_br_mutbl,
                     );
                 }
@@ -2387,7 +2390,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     self.add_rust_2024_migration_desugared_pat(
                         pat_info.top_info.hir_id,
                         pat,
-                        inner.span,
+                        match pat_mutbl {
+                            Mutability::Not => '&', // last char of `&`
+                            Mutability::Mut => 't', // last char of `&mut`
+                        },
                         inh_mut,
                     )
                 }
@@ -2779,18 +2785,20 @@ 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());
+        let from_expansion = subpat.span.from_expansion();
+        let trimmed_span = if from_expansion {
+            // If the subpattern is from an expansion, highlight the whole macro call instead.
+            subpat.span
+        } else {
+            let trimmed = self.tcx.sess.source_map().span_through_char(subpat.span, final_char);
+            // The edition of the trimmed span should be the same as `subpat.span`; this will be a
+            // a hard error if the subpattern is of edition >= 2024. We set it manually to be sure:
+            trimmed.with_ctxt(subpat.span.ctxt())
+        };
 
         let mut typeck_results = self.typeck_results.borrow_mut();
         let mut table = typeck_results.rust_2024_migration_desugared_pats_mut();
@@ -2824,7 +2832,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         };
         // 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;

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

@@ -244,4 +244,10 @@ fn main() {
     let &[migration_lint_macros::bind_ref!(a)] = &[0];
     //~^ ERROR: binding modifiers may only be written when the default binding mode is `move`
     assert_type_eq(a, &0u32);
+
+    // Test that we use the correct span when labeling a `&` whose subpattern is from an expansion.
+    let &[&migration_lint_macros::bind_ref!(a)] = &[&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);
 }

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

@@ -244,4 +244,10 @@ fn main() {
     let [migration_lint_macros::bind_ref!(a)] = &[0];
     //~^ ERROR: binding modifiers may only be written when the default binding mode is `move`
     assert_type_eq(a, &0u32);
+
+    // Test that we use the correct span when labeling a `&` whose subpattern is from an expansion.
+    let [&migration_lint_macros::bind_ref!(a)] = &[&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);
 }

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

@@ -580,5 +580,23 @@ help: make the implied reference pattern explicit
 LL |     let &[migration_lint_macros::bind_ref!(a)] = &[0];
    |         +
 
-error: aborting due to 30 previous errors
+error: reference patterns may only be written when the default binding mode is `move` in Rust 2024
+  --> $DIR/migration_lint.rs:249:10
+   |
+LL |     let [&migration_lint_macros::bind_ref!(a)] = &[&0];
+   |          ^ reference pattern not allowed under `ref` default binding mode
+   |
+   = warning: this changes meaning in Rust 2024
+   = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
+note: matching on a reference type with a non-reference pattern changes the default binding mode
+  --> $DIR/migration_lint.rs:249:9
+   |
+LL |     let [&migration_lint_macros::bind_ref!(a)] = &[&0];
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_`
+help: make the implied reference pattern explicit
+   |
+LL |     let &[&migration_lint_macros::bind_ref!(a)] = &[&0];
+   |         +
+
+error: aborting due to 31 previous errors