Auto merge of rust-lang#112945 - compiler-errors:tighten-span-of-adju…

…stment-error, r=oli-obk

(re-)tighten sourceinfo span of adjustments in MIR

Diagnostics rely on the spans of MIR statements being (approximately) correct in order to give suggestions relative to that span (i.e. `shrink_to_hi` and `shrink_to_lo`).

I discovered that we're *intentionally* lowering THIR exprs with their parent expr's span if they come from adjustments that are due to a parent expression. While I understand why that may be desirable to demonstrate the relationship of an adjustment and the expression that requires it, it leads to

1. very verbose borrowck output
2. incorrect spans for suggestions

Some diagnostics get around that by giving suggestions relative to other spans we've collected during MIR lowering, such as the span of the method's identifier (e.g. `name` in `.name()`), but this doesn't work too well when things come from desugaring.

I assume it also has lead to numerous tweaks and complications to diagnostics code down the road, which this PR doesn't necessarily aim to fix but may open the gates to fixing later... The last three commits are simplifications due to the fact that we can assume that the move span actually points to what is being moved (and a test).

This regressed in rust-lang#89110, which was debated somewhat in rust-lang#90286. cc `@Aaron1011` who originally made this change.

r? diagnostics

Fixes rust-lang#113547
Fixes rust-lang#111016

compiler/rustc_borrowck/src/diagnostics/mod.rs

@@ -1121,7 +1121,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                             err.eager_subdiagnostic(
                                 &self.infcx.tcx.sess.parse_sess.span_diagnostic,
                                 CaptureReasonSuggest::FreshReborrow {
-                                    span: fn_call_span.shrink_to_lo(),
+                                    span: move_span.shrink_to_hi(),
                                 });
                         }
                         if let Some(clone_trait) = tcx.lang_items().clone_trait()
@@ -1135,10 +1135,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                             && self.infcx.predicate_must_hold_modulo_regions(&o)
                         {
                             err.span_suggestion_verbose(
-                                fn_call_span.shrink_to_lo(),
+                                move_span.shrink_to_hi(),
                                 "you can `clone` the value and consume it, but this might not be \
                                  your desired behavior",
-                                "clone().".to_string(),
+                                ".clone()".to_string(),
                                 Applicability::MaybeIncorrect,
                             );
                         }

compiler/rustc_borrowck/src/session_diagnostics.rs

@@ -398,7 +398,7 @@ pub(crate) enum CaptureReasonSuggest<'tcx> {
     #[suggestion(
         borrowck_suggest_create_freash_reborrow,
         applicability = "maybe-incorrect",
-        code = "as_mut().",
+        code = ".as_mut()",
         style = "verbose"
     )]
     FreshReborrow {

compiler/rustc_mir_build/src/thir/cx/expr.rs

@@ -41,24 +41,14 @@ impl<'tcx> Cx<'tcx> {
 
         let mut expr = self.make_mirror_unadjusted(hir_expr);
 
-        let adjustment_span = match self.adjustment_span {
-            Some((hir_id, span)) if hir_id == hir_expr.hir_id => Some(span),
-            _ => None,
-        };
-
         trace!(?expr.ty);
 
         // Now apply adjustments, if any.
         if self.apply_adjustments {
             for adjustment in self.typeck_results.expr_adjustments(hir_expr) {
                 trace!(?expr, ?adjustment);
                 let span = expr.span;
-                expr = self.apply_adjustment(
-                    hir_expr,
-                    expr,
-                    adjustment,
-                    adjustment_span.unwrap_or(span),
-                );
+                expr = self.apply_adjustment(hir_expr, expr, adjustment, span);
             }
         }
 
@@ -274,7 +264,6 @@ impl<'tcx> Cx<'tcx> {
     fn make_mirror_unadjusted(&mut self, expr: &'tcx hir::Expr<'tcx>) -> Expr<'tcx> {
         let tcx = self.tcx;
         let expr_ty = self.typeck_results().expr_ty(expr);
-        let expr_span = expr.span;
         let temp_lifetime =
             self.rvalue_scopes.temporary_scope(self.region_scope_tree, expr.hir_id.local_id);
 
@@ -283,17 +272,11 @@ impl<'tcx> Cx<'tcx> {
             hir::ExprKind::MethodCall(segment, receiver, ref args, fn_span) => {
                 // Rewrite a.b(c) into UFCS form like Trait::b(a, c)
                 let expr = self.method_callee(expr, segment.ident.span, None);
-                // When we apply adjustments to the receiver, use the span of
-                // the overall method call for better diagnostics. args[0]
-                // is guaranteed to exist, since a method call always has a receiver.
-                let old_adjustment_span =
-                    self.adjustment_span.replace((receiver.hir_id, expr_span));
                 info!("Using method span: {:?}", expr.span);
                 let args = std::iter::once(receiver)
                     .chain(args.iter())
                     .map(|expr| self.mirror_expr(expr))
                     .collect();
-                self.adjustment_span = old_adjustment_span;
                 ExprKind::Call {
                     ty: expr.ty,
                     fun: self.thir.exprs.push(expr),

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

@@ -16,7 +16,6 @@ use rustc_hir::Node;
 use rustc_middle::middle::region;
 use rustc_middle::thir::*;
 use rustc_middle::ty::{self, RvalueScopes, Ty, TyCtxt};
-use rustc_span::Span;
 
 pub(crate) fn thir_body(
     tcx: TyCtxt<'_>,
@@ -62,14 +61,6 @@ struct Cx<'tcx> {
     typeck_results: &'tcx ty::TypeckResults<'tcx>,
     rvalue_scopes: &'tcx RvalueScopes,
 
-    /// When applying adjustments to the expression
-    /// with the given `HirId`, use the given `Span`,
-    /// instead of the usual span. This is used to
-    /// assign the span of an overall method call
-    /// (e.g. `my_val.foo()`) to the adjustment expressions
-    /// for the receiver.
-    adjustment_span: Option<(HirId, Span)>,
-
     /// False to indicate that adjustments should not be applied. Only used for `custom_mir`
     apply_adjustments: bool,
 
@@ -110,7 +101,6 @@ impl<'tcx> Cx<'tcx> {
             typeck_results,
             rvalue_scopes: &typeck_results.rvalue_scopes,
             body_owner: def.to_def_id(),
-            adjustment_span: None,
             apply_adjustments: hir
                 .attrs(hir_id)
                 .iter()

src/tools/miri/tests/fail/box-cell-alias.stderr

@@ -2,7 +2,7 @@ error: Undefined Behavior: trying to retag from <TAG> for SharedReadWrite permis
   --> $DIR/box-cell-alias.rs:LL:CC
    |
 LL |     unsafe { (*ptr).set(20) };
-   |              ^^^^^^^^^^^^^^
+   |              ^^^^^^
    |              |
    |              trying to retag from <TAG> for SharedReadWrite permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
    |              this error occurs as part of retag at ALLOC[0x0..0x1]

src/tools/miri/tests/fail/stacked_borrows/illegal_read7.stderr

@@ -2,7 +2,7 @@ error: Undefined Behavior: trying to retag from <TAG> for SharedReadWrite permis
   --> $DIR/illegal_read7.rs:LL:CC
    |
 LL |         let _val = *x.get_mut();
-   |                     ^^^^^^^^^^^
+   |                     ^
    |                     |
    |                     trying to retag from <TAG> for SharedReadWrite permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
    |                     this error occurs as part of two-phase retag at ALLOC[0x0..0x4]

src/tools/miri/tests/fail/stacked_borrows/interior_mut1.stderr

@@ -2,7 +2,7 @@ error: Undefined Behavior: trying to retag from <TAG> for SharedReadWrite permis
   --> $DIR/interior_mut1.rs:LL:CC
    |
 LL |         let _val = *inner_shr.get();
-   |                     ^^^^^^^^^^^^^^^
+   |                     ^^^^^^^^^
    |                     |
    |                     trying to retag from <TAG> for SharedReadWrite permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
    |                     this error occurs as part of retag at ALLOC[0x0..0x4]

src/tools/miri/tests/fail/stacked_borrows/interior_mut2.stderr

@@ -2,7 +2,7 @@ error: Undefined Behavior: trying to retag from <TAG> for SharedReadWrite permis
   --> $DIR/interior_mut2.rs:LL:CC
    |
 LL |         let _val = *inner_shr.get();
-   |                     ^^^^^^^^^^^^^^^
+   |                     ^^^^^^^^^
    |                     |
    |                     trying to retag from <TAG> for SharedReadWrite permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
    |                     this error occurs as part of retag at ALLOC[0x0..0x4]

src/tools/miri/tests/fail/stacked_borrows/shared_rw_borrows_are_weak1.stderr

@@ -2,7 +2,7 @@ error: Undefined Behavior: trying to retag from <TAG> for SharedReadWrite permis
   --> $DIR/shared_rw_borrows_are_weak1.rs:LL:CC
    |
 LL |         y.get_mut();
-   |         ^^^^^^^^^^^
+   |         ^
    |         |
    |         trying to retag from <TAG> for SharedReadWrite permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
    |         this error occurs as part of two-phase retag at ALLOC[0x0..0x4]

src/tools/miri/tests/fail/tree_borrows/fnentry_invalidation.stderr

@@ -21,7 +21,7 @@ help: the accessed tag <TAG> later transitioned to Frozen due to a reborrow (act
   --> $DIR/fnentry_invalidation.rs:LL:CC
    |
 LL |     x.do_bad();
-   |     ^^^^^^^^^^
+   |     ^
    = help: this transition corresponds to a loss of write permissions
    = note: BACKTRACE (of the first span):
    = note: inside `main` at $DIR/fnentry_invalidation.rs:LL:CC

src/tools/miri/tests/fail/tree_borrows/write-during-2phase.stderr

@@ -9,15 +9,8 @@ LL |     fn add(&mut self, n: u64) -> u64 {
 help: the accessed tag <TAG> was created here, in the initial state Reserved
   --> $DIR/write-during-2phase.rs:LL:CC
    |
-LL |       let _res = f.add(unsafe {
-   |  ________________^
-LL | |         let n = f.0;
-LL | |         // This is the access at fault, but it's not immediately apparent because
-LL | |         // the reference that got invalidated is not under a Protector.
-LL | |         *inner = 42;
-LL | |         n
-LL | |     });
-   | |______^
+LL |     let _res = f.add(unsafe {
+   |                ^
 help: the accessed tag <TAG> later transitioned to Disabled due to a foreign write access at offsets [0x0..0x8]
   --> $DIR/write-during-2phase.rs:LL:CC
    |

tests/ui/array-slice-vec/vec-mut-iter-borrow.stderr

@@ -7,7 +7,7 @@ LL |     for x in &mut xs {
    |              first mutable borrow occurs here
    |              first borrow later used here
 LL |         xs.push(1)
-   |         ^^^^^^^^^^ second mutable borrow occurs here
+   |         ^^ second mutable borrow occurs here
 
 error: aborting due to previous error
 

tests/ui/async-await/clone-suggestion.stderr

@@ -13,7 +13,7 @@ note: `into_future` takes ownership of the receiver `self`, which moves `f`
 help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
 LL |     f.clone().await;
-   |       ++++++++
+   |      ++++++++
 
 error: aborting due to previous error
 

tests/ui/async-await/issue-61452.stderr

@@ -2,7 +2,7 @@ error[E0596]: cannot borrow `x` as mutable, as it is not declared as mutable
   --> $DIR/issue-61452.rs:4:5
    |
 LL |     x.take();
-   |     ^^^^^^^^ cannot borrow as mutable
+   |     ^ cannot borrow as mutable
    |
 help: consider changing this to be mutable
    |

tests/ui/async-await/issues/issue-61187.stderr

@@ -2,7 +2,7 @@ error[E0596]: cannot borrow `data` as mutable, as it is not declared as mutable
   --> $DIR/issue-61187.rs:6:5
    |
 LL |     data.reverse();
-   |     ^^^^^^^^^^^^^^ cannot borrow as mutable
+   |     ^^^^ cannot borrow as mutable
    |
 help: consider changing this to be mutable
    |

tests/ui/binop/binop-move-semantics.stderr

@@ -27,7 +27,7 @@ LL |     x
    |     - value moved here
 LL |     +
 LL |     x.clone();
-   |     ^^^^^^^^^ value borrowed here after move
+   |     ^ value borrowed here after move
    |
 help: consider cloning the value if the performance cost is acceptable
    |

tests/ui/borrowck/borrow-tuple-fields.stderr

@@ -9,7 +9,7 @@ LL |     let y = x;
    |             ^ move out of `x` occurs here
 LL |
 LL |     r.use_ref();
-   |     ----------- borrow later used here
+   |     - borrow later used here
 
 error[E0502]: cannot borrow `x.0` as mutable because it is also borrowed as immutable
   --> $DIR/borrow-tuple-fields.rs:18:13
@@ -19,7 +19,7 @@ LL |     let a = &x.0;
 LL |     let b = &mut x.0;
    |             ^^^^^^^^ mutable borrow occurs here
 LL |     a.use_ref();
-   |     ----------- immutable borrow later used here
+   |     - immutable borrow later used here
 
 error[E0499]: cannot borrow `x.0` as mutable more than once at a time
   --> $DIR/borrow-tuple-fields.rs:23:13
@@ -29,7 +29,7 @@ LL |     let a = &mut x.0;
 LL |     let b = &mut x.0;
    |             ^^^^^^^^ second mutable borrow occurs here
 LL |     a.use_ref();
-   |     ----------- first borrow later used here
+   |     - first borrow later used here
 
 error[E0505]: cannot move out of `x` because it is borrowed
   --> $DIR/borrow-tuple-fields.rs:28:13
@@ -41,7 +41,7 @@ LL |     let r = &x.0;
 LL |     let y = x;
    |             ^ move out of `x` occurs here
 LL |     r.use_ref();
-   |     ----------- borrow later used here
+   |     - borrow later used here
 
 error[E0502]: cannot borrow `x.0` as mutable because it is also borrowed as immutable
   --> $DIR/borrow-tuple-fields.rs:33:13
@@ -51,7 +51,7 @@ LL |     let a = &x.0;
 LL |     let b = &mut x.0;
    |             ^^^^^^^^ mutable borrow occurs here
 LL |     a.use_ref();
-   |     ----------- immutable borrow later used here
+   |     - immutable borrow later used here
 
 error[E0499]: cannot borrow `x.0` as mutable more than once at a time
   --> $DIR/borrow-tuple-fields.rs:38:13
@@ -61,7 +61,7 @@ LL |     let a = &mut x.0;
 LL |     let b = &mut x.0;
    |             ^^^^^^^^ second mutable borrow occurs here
 LL |     a.use_mut();
-   |     ----------- first borrow later used here
+   |     - first borrow later used here
 
 error: aborting due to 6 previous errors
 

tests/ui/borrowck/borrowck-argument.stderr

@@ -2,7 +2,7 @@ error[E0596]: cannot borrow `arg` as mutable, as it is not declared as mutable
   --> $DIR/borrowck-argument.rs:10:5
    |
 LL |     arg.mutate();
-   |     ^^^^^^^^^^^^ cannot borrow as mutable
+   |     ^^^ cannot borrow as mutable
    |
 help: consider changing this to be mutable
    |
@@ -13,7 +13,7 @@ error[E0596]: cannot borrow `arg` as mutable, as it is not declared as mutable
   --> $DIR/borrowck-argument.rs:15:9
    |
 LL |         arg.mutate();
-   |         ^^^^^^^^^^^^ cannot borrow as mutable
+   |         ^^^ cannot borrow as mutable
    |
 help: consider changing this to be mutable
    |
@@ -24,7 +24,7 @@ error[E0596]: cannot borrow `arg` as mutable, as it is not declared as mutable
   --> $DIR/borrowck-argument.rs:21:9
    |
 LL |         arg.mutate();
-   |         ^^^^^^^^^^^^ cannot borrow as mutable
+   |         ^^^ cannot borrow as mutable
    |
 help: consider changing this to be mutable
    |
@@ -35,7 +35,7 @@ error[E0596]: cannot borrow `arg` as mutable, as it is not declared as mutable
   --> $DIR/borrowck-argument.rs:32:17
    |
 LL |     (|arg: S| { arg.mutate() })(s);
-   |                 ^^^^^^^^^^^^ cannot borrow as mutable
+   |                 ^^^ cannot borrow as mutable
    |
 help: consider changing this to be mutable
    |

tests/ui/borrowck/borrowck-auto-mut-ref-to-immut-var.stderr

@@ -2,7 +2,7 @@ error[E0596]: cannot borrow `x` as mutable, as it is not declared as mutable
   --> $DIR/borrowck-auto-mut-ref-to-immut-var.rs:15:5
    |
 LL |     x.printme();
-   |     ^^^^^^^^^^^ cannot borrow as mutable
+   |     ^ cannot borrow as mutable
    |
 help: consider changing this to be mutable
    |

tests/ui/borrowck/borrowck-borrow-immut-deref-of-box-as-mut.stderr

@@ -2,7 +2,7 @@ error[E0596]: cannot borrow `*a` as mutable, as `a` is not declared as mutable
   --> $DIR/borrowck-borrow-immut-deref-of-box-as-mut.rs:12:5
    |
 LL |     a.foo();
-   |     ^^^^^^^ cannot borrow as mutable
+   |     ^ cannot borrow as mutable
    |
 help: consider changing this to be mutable
    |

tests/ui/borrowck/borrowck-borrow-mut-object-twice.stderr

@@ -2,11 +2,11 @@ error[E0499]: cannot borrow `*x` as mutable more than once at a time
   --> $DIR/borrowck-borrow-mut-object-twice.rs:13:5
    |
 LL |     let y = x.f1();
-   |             ------ first mutable borrow occurs here
+   |             - first mutable borrow occurs here
 LL |     x.f2();
-   |     ^^^^^^ second mutable borrow occurs here
+   |     ^ second mutable borrow occurs here
 LL |     y.use_ref();
-   |     ----------- first borrow later used here
+   |     - first borrow later used here
 
 error: aborting due to previous error