Skip to content

Commit

Permalink
Auto merge of #112945 - compiler-errors:tighten-span-of-adjustment-er…
Browse files Browse the repository at this point in the history
…ror, 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 #89110, which was debated somewhat in #90286. cc `@Aaron1011` who originally made this change.

r? diagnostics

Fixes #113547
Fixes #111016
  • Loading branch information
bors committed Jul 12, 2023
2 parents 136dab6 + a74db1a commit da1d099
Show file tree
Hide file tree
Showing 172 changed files with 426 additions and 452 deletions.
6 changes: 3 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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,
);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
19 changes: 1 addition & 18 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);

Expand All @@ -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),
Expand Down
10 changes: 0 additions & 10 deletions compiler/rustc_mir_build/src/thir/cx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<'_>,
Expand Down Expand Up @@ -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,

Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/fail/box-cell-alias.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/array-slice-vec/vec-mut-iter-borrow.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/async-await/clone-suggestion.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/async-await/issue-61452.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/async-await/issues/issue-61187.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/binop/binop-move-semantics.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
12 changes: 6 additions & 6 deletions tests/ui/borrowck/borrow-tuple-fields.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
8 changes: 4 additions & 4 deletions tests/ui/borrowck/borrowck-argument.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/borrowck/borrowck-borrow-mut-object-twice.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading

0 comments on commit da1d099

Please sign in to comment.