Skip to content

Commit

Permalink
Improve diagnostic labels and add note.
Browse files Browse the repository at this point in the history
This commit improves diagnostic labels to mention which field a borrow
overlaps with and adds a note explaining that the fields overlap.
  • Loading branch information
davidtwco committed Jan 4, 2019
1 parent 388dffe commit c2b477c
Show file tree
Hide file tree
Showing 13 changed files with 86 additions and 43 deletions.
8 changes: 2 additions & 6 deletions src/librustc_borrowck/borrowck/check_loans.rs
Expand Up @@ -557,12 +557,8 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
if new_loan.loan_path.has_fork(&old_loan.loan_path) && common.is_some() {
let nl = self.bccx.loan_path_to_string(&common.unwrap());
let ol = nl.clone();
let new_loan_msg = format!(" (via `{}`)",
self.bccx.loan_path_to_string(
&new_loan.loan_path));
let old_loan_msg = format!(" (via `{}`)",
self.bccx.loan_path_to_string(
&old_loan.loan_path));
let new_loan_msg = self.bccx.loan_path_to_string(&new_loan.loan_path);
let old_loan_msg = self.bccx.loan_path_to_string(&old_loan.loan_path);
(nl, ol, new_loan_msg, old_loan_msg)
} else {
(self.bccx.loan_path_to_string(&new_loan.loan_path),
Expand Down
26 changes: 17 additions & 9 deletions src/librustc_mir/borrow_check/error_reporting.rs
Expand Up @@ -329,12 +329,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
"closure"
};

let (desc_place, msg_place, msg_borrow) = self.describe_place_for_conflicting_borrow(
place, &issued_borrow.borrowed_place,
);
let via = |msg: String| if msg.is_empty() { msg } else { format!(" (via `{}`)", msg) };
let msg_place = via(msg_place);
let msg_borrow = via(msg_borrow);
let (desc_place, msg_place, msg_borrow, union_type_name) =
self.describe_place_for_conflicting_borrow(place, &issued_borrow.borrowed_place);

let explanation = self.explain_why_borrow_contains_point(context, issued_borrow, None);
let second_borrow_desc = if explanation.is_explained() {
Expand Down Expand Up @@ -516,6 +512,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
);
}

if union_type_name != "" {
err.note(&format!(
"`{}` is a field of the union `{}`, so it overlaps the field `{}`",
msg_place, union_type_name, msg_borrow,
));
}

explanation
.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, first_borrow_desc);

Expand Down Expand Up @@ -549,7 +552,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
&self,
first_borrowed_place: &Place<'tcx>,
second_borrowed_place: &Place<'tcx>,
) -> (String, String, String) {
) -> (String, String, String, String) {
// Define a small closure that we can use to check if the type of a place
// is a union.
let is_union = |place: &Place<'tcx>| -> bool {
Expand Down Expand Up @@ -600,7 +603,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
.unwrap_or_else(|| "_".to_owned());
let desc_second = self.describe_place(second_borrowed_place)
.unwrap_or_else(|| "_".to_owned());
return Some((desc_base, desc_first, desc_second));

// Also compute the name of the union type, eg. `Foo` so we
// can add a helpful note with it.
let ty = base.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx);

return Some((desc_base, desc_first, desc_second, ty.to_string()));
},
_ => current = base,
}
Expand All @@ -612,7 +620,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
// only return the description of the first place.
let desc_place = self.describe_place(first_borrowed_place)
.unwrap_or_else(|| "_".to_owned());
(desc_place, "".to_string(), "".to_string())
(desc_place, "".to_string(), "".to_string(), "".to_string())
})
}

Expand Down
43 changes: 32 additions & 11 deletions src/librustc_mir/util/borrowck_errors.rs
Expand Up @@ -138,13 +138,15 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
old_load_end_span: Option<Span>,
o: Origin,
) -> DiagnosticBuilder<'cx> {
let via = |msg: &str|
if msg.is_empty() { msg.to_string() } else { format!(" (via `{}`)", msg) };
let mut err = struct_span_err!(
self,
new_loan_span,
E0499,
"cannot borrow `{}`{} as mutable more than once at a time{OGN}",
desc,
opt_via,
via(opt_via),
OGN = o
);
if old_loan_span == new_loan_span {
Expand All @@ -164,11 +166,11 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
} else {
err.span_label(
old_loan_span,
format!("first mutable borrow occurs here{}", old_opt_via),
format!("first mutable borrow occurs here{}", via(old_opt_via)),
);
err.span_label(
new_loan_span,
format!("second mutable borrow occurs here{}", opt_via),
format!("second mutable borrow occurs here{}", via(opt_via)),
);
if let Some(old_load_end_span) = old_load_end_span {
err.span_label(old_load_end_span, "first borrow ends here");
Expand Down Expand Up @@ -292,27 +294,46 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
old_load_end_span: Option<Span>,
o: Origin,
) -> DiagnosticBuilder<'cx> {
let via = |msg: &str|
if msg.is_empty() { msg.to_string() } else { format!(" (via `{}`)", msg) };
let mut err = struct_span_err!(
self,
span,
E0502,
"cannot borrow `{}`{} as {} because {} is also borrowed as {}{}{OGN}",
"cannot borrow `{}`{} as {} because {} is also borrowed \
as {}{}{OGN}",
desc_new,
msg_new,
via(msg_new),
kind_new,
noun_old,
kind_old,
msg_old,
via(msg_old),
OGN = o
);
err.span_label(span, format!("{} borrow occurs here{}", kind_new, msg_new));
err.span_label(
old_span,
format!("{} borrow occurs here{}", kind_old, msg_old),
);

if msg_new == "" {
// If `msg_new` is empty, then this isn't a borrow of a union field.
err.span_label(span, format!("{} borrow occurs here", kind_new));
err.span_label(old_span, format!("{} borrow occurs here", kind_old));
} else {
// If `msg_new` isn't empty, then this a borrow of a union field.
err.span_label(
span,
format!(
"{} borrow of `{}` -- which overlaps with `{}` -- occurs here",
kind_new, msg_new, msg_old,
)
);
err.span_label(
old_span,
format!("{} borrow occurs here{}", kind_old, via(msg_old)),
);
}

if let Some(old_load_end_span) = old_load_end_span {
err.span_label(old_load_end_span, format!("{} borrow ends here", kind_old));
}

self.cancel_if_wrong_origin(err, o)
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/borrowck/borrowck-borrow-from-owned-ptr.stderr
Expand Up @@ -138,7 +138,7 @@ error[E0502]: cannot borrow `foo` (via `foo.bar2`) as immutable because `foo` is
LL | let bar1 = &mut foo.bar1;
| -------- mutable borrow occurs here (via `foo.bar1`)
LL | let _foo1 = &foo.bar2; //~ ERROR cannot borrow
| ^^^^^^^^ immutable borrow occurs here (via `foo.bar2`)
| ^^^^^^^^ immutable borrow of `foo.bar2` -- which overlaps with `foo.bar1` -- occurs here
LL | *bar1;
LL | }
| - mutable borrow ends here
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/borrowck/borrowck-box-insensitivity.ast.stderr
Expand Up @@ -61,7 +61,7 @@ error[E0502]: cannot borrow `a` (via `a.y`) as immutable because `a` is also bor
LL | let _x = &mut a.x;
| --- mutable borrow occurs here (via `a.x`)
LL | let _y = &a.y; //[ast]~ ERROR cannot borrow
| ^^^ immutable borrow occurs here (via `a.y`)
| ^^^ immutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here
...
LL | }
| - mutable borrow ends here
Expand All @@ -72,7 +72,7 @@ error[E0502]: cannot borrow `a` (via `a.y`) as mutable because `a` is also borro
LL | let _x = &a.x;
| --- immutable borrow occurs here (via `a.x`)
LL | let _y = &mut a.y; //[ast]~ ERROR cannot borrow
| ^^^ mutable borrow occurs here (via `a.y`)
| ^^^ mutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here
...
LL | }
| - immutable borrow ends here
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/borrowck/borrowck-box-insensitivity.rs
Expand Up @@ -83,14 +83,14 @@ fn borrow_after_mut_borrow() {
let mut a: Box<_> = box A { x: box 0, y: 1 };
let _x = &mut a.x;
let _y = &a.y; //[ast]~ ERROR cannot borrow
//[ast]~^ immutable borrow occurs here (via `a.y`)
//[ast]~^ immutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here
use_mut(_x);
}
fn mut_borrow_after_borrow() {
let mut a: Box<_> = box A { x: box 0, y: 1 };
let _x = &a.x;
let _y = &mut a.y; //[ast]~ ERROR cannot borrow
//[ast]~^ mutable borrow occurs here (via `a.y`)
//[ast]~^ mutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here
use_imm(_x);
}
fn copy_after_move_nested() {
Expand Down
10 changes: 8 additions & 2 deletions src/test/ui/borrowck/borrowck-union-borrow.nll.stderr
Expand Up @@ -24,9 +24,11 @@ error[E0502]: cannot borrow `u` (via `u.b`) as mutable because it is also borrow
LL | let ra = &u.a;
| ---- immutable borrow occurs here (via `u.a`)
LL | let rmb = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as mutable because `u` is also borrowed as immutable (via `u.a`)
| ^^^^^^^^ mutable borrow occurs here (via `u.b`)
| ^^^^^^^^ mutable borrow of `u.b` -- which overlaps with `u.a` -- occurs here
LL | drop(ra);
| -- immutable borrow later used here
|
= note: `u.b` is a field of the union `U`, so it overlaps the field `u.a`

error[E0506]: cannot assign to `u.b` because it is borrowed
--> $DIR/borrowck-union-borrow.rs:51:13
Expand Down Expand Up @@ -84,9 +86,11 @@ error[E0502]: cannot borrow `u` (via `u.b`) as immutable because it is also borr
LL | let rma = &mut u.a;
| -------- mutable borrow occurs here (via `u.a`)
LL | let rb = &u.b; //~ ERROR cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`)
| ^^^^ immutable borrow occurs here (via `u.b`)
| ^^^^ immutable borrow of `u.b` -- which overlaps with `u.a` -- occurs here
LL | drop(rma);
| --- mutable borrow later used here
|
= note: `u.b` is a field of the union `U`, so it overlaps the field `u.a`

error[E0503]: cannot use `u.b` because it was mutably borrowed
--> $DIR/borrowck-union-borrow.rs:83:21
Expand All @@ -108,6 +112,8 @@ LL | let rmb2 = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as
| ^^^^^^^^ second mutable borrow occurs here (via `u.b`)
LL | drop(rma);
| --- first borrow later used here
|
= note: `u.b` is a field of the union `U`, so it overlaps the field `u.a`

error[E0506]: cannot assign to `u.b` because it is borrowed
--> $DIR/borrowck-union-borrow.rs:94:13
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/borrowck/borrowck-union-borrow.stderr
Expand Up @@ -23,7 +23,7 @@ error[E0502]: cannot borrow `u` (via `u.b`) as mutable because `u` is also borro
LL | let ra = &u.a;
| --- immutable borrow occurs here (via `u.a`)
LL | let rmb = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as mutable because `u` is also borrowed as immutable (via `u.a`)
| ^^^ mutable borrow occurs here (via `u.b`)
| ^^^ mutable borrow of `u.b` -- which overlaps with `u.a` -- occurs here
LL | drop(ra);
LL | }
| - immutable borrow ends here
Expand Down Expand Up @@ -80,7 +80,7 @@ error[E0502]: cannot borrow `u` (via `u.b`) as immutable because `u` is also bor
LL | let rma = &mut u.a;
| --- mutable borrow occurs here (via `u.a`)
LL | let rb = &u.b; //~ ERROR cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`)
| ^^^ immutable borrow occurs here (via `u.b`)
| ^^^ immutable borrow of `u.b` -- which overlaps with `u.a` -- occurs here
LL | drop(rma);
LL | }
| - mutable borrow ends here
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-17263.ast.stderr
Expand Up @@ -13,7 +13,7 @@ error[E0502]: cannot borrow `foo` (via `foo.b`) as immutable because `foo` is al
--> $DIR/issue-17263.rs:21:32
|
LL | let (c, d) = (&mut foo.a, &foo.b);
| ----- ^^^^^ immutable borrow occurs here (via `foo.b`)
| ----- ^^^^^ immutable borrow of `foo.b` -- which overlaps with `foo.a` -- occurs here
| |
| mutable borrow occurs here (via `foo.a`)
...
Expand Down
4 changes: 3 additions & 1 deletion src/test/ui/issues/issue-45157.stderr
Expand Up @@ -5,10 +5,12 @@ LL | let mref = &mut u.s.a;
| ---------- mutable borrow occurs here (via `u.s.a`)
...
LL | let nref = &u.z.c;
| ^^^^^^ immutable borrow occurs here (via `u.z.c`)
| ^^^^^^ immutable borrow of `u.z.c` -- which overlaps with `u.s.a` -- occurs here
LL | //~^ ERROR cannot borrow `u` (via `u.z.c`) as immutable because it is also borrowed as mutable (via `u.s.a`) [E0502]
LL | println!("{} {}", mref, nref)
| ---- mutable borrow later used here
|
= note: `u.z.c` is a field of the union `U`, so it overlaps the field `u.s.a`

error: aborting due to previous error

Expand Down
8 changes: 6 additions & 2 deletions src/test/ui/nll/issue-57100.stderr
Expand Up @@ -5,10 +5,12 @@ LL | let mref = &mut r.r2_union.f3_union.s1_leaf.l1_u8;
| -------------------------------------- mutable borrow occurs here (via `r.r2_union.f3_union.s1_leaf.l1_u8`)
...
LL | let nref = &r.r2_union.f3_union.s2_leaf.l1_u8;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here (via `r.r2_union.f3_union.s2_leaf.l1_u8`)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow of `r.r2_union.f3_union.s2_leaf.l1_u8` -- which overlaps with `r.r2_union.f3_union.s1_leaf.l1_u8` -- occurs here
...
LL | println!("{} {}", mref, nref)
| ---- mutable borrow later used here
|
= note: `r.r2_union.f3_union.s2_leaf.l1_u8` is a field of the union `Second`, so it overlaps the field `r.r2_union.f3_union.s1_leaf.l1_u8`

error[E0502]: cannot borrow `r.r2_union` (via `r.r2_union.f1_leaf.l1_u8`) as immutable because it is also borrowed as mutable (via `r.r2_union.f2_leaf.l1_u8`)
--> $DIR/issue-57100.rs:62:20
Expand All @@ -17,10 +19,12 @@ LL | let mref = &mut r.r2_union.f2_leaf.l1_u8;
| ----------------------------- mutable borrow occurs here (via `r.r2_union.f2_leaf.l1_u8`)
...
LL | let nref = &r.r2_union.f1_leaf.l1_u8;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here (via `r.r2_union.f1_leaf.l1_u8`)
| ^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow of `r.r2_union.f1_leaf.l1_u8` -- which overlaps with `r.r2_union.f2_leaf.l1_u8` -- occurs here
...
LL | println!("{} {}", mref, nref)
| ---- mutable borrow later used here
|
= note: `r.r2_union.f1_leaf.l1_u8` is a field of the union `First`, so it overlaps the field `r.r2_union.f2_leaf.l1_u8`

error: aborting due to 2 previous errors

Expand Down
12 changes: 9 additions & 3 deletions src/test/ui/union/union-borrow-move-parent-sibling.nll.stderr
Expand Up @@ -4,9 +4,11 @@ error[E0502]: cannot borrow `u` (via `u.y`) as immutable because it is also borr
LL | let a = &mut u.x.0;
| ---------- mutable borrow occurs here (via `u.x.0`)
LL | let b = &u.y; //~ ERROR cannot borrow `u.y`
| ^^^^ immutable borrow occurs here (via `u.y`)
| ^^^^ immutable borrow of `u.y` -- which overlaps with `u.x.0` -- occurs here
LL | use_borrow(a);
| - mutable borrow later used here
|
= note: `u.y` is a field of the union `U`, so it overlaps the field `u.x.0`

error[E0382]: use of moved value: `u`
--> $DIR/union-borrow-move-parent-sibling.rs:22:13
Expand All @@ -24,9 +26,11 @@ error[E0502]: cannot borrow `u` (via `u.y`) as immutable because it is also borr
LL | let a = &mut (u.x.0).0;
| -------------- mutable borrow occurs here (via `u.x.0.0`)
LL | let b = &u.y; //~ ERROR cannot borrow `u.y`
| ^^^^ immutable borrow occurs here (via `u.y`)
| ^^^^ immutable borrow of `u.y` -- which overlaps with `u.x.0.0` -- occurs here
LL | use_borrow(a);
| - mutable borrow later used here
|
= note: `u.y` is a field of the union `U`, so it overlaps the field `u.x.0.0`

error[E0382]: use of moved value: `u`
--> $DIR/union-borrow-move-parent-sibling.rs:35:13
Expand All @@ -44,9 +48,11 @@ error[E0502]: cannot borrow `u` (via `u.x`) as immutable because it is also borr
LL | let a = &mut *u.y;
| --------- mutable borrow occurs here (via `*u.y`)
LL | let b = &u.x; //~ ERROR cannot borrow `u` (via `u.x`)
| ^^^^ immutable borrow occurs here (via `u.x`)
| ^^^^ immutable borrow of `u.x` -- which overlaps with `*u.y` -- occurs here
LL | use_borrow(a);
| - mutable borrow later used here
|
= note: `u.x` is a field of the union `U`, so it overlaps the field `*u.y`

error[E0382]: use of moved value: `u`
--> $DIR/union-borrow-move-parent-sibling.rs:48:13
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/union/union-borrow-move-parent-sibling.stderr
Expand Up @@ -46,7 +46,7 @@ error[E0502]: cannot borrow `u` (via `u.x`) as immutable because `u` is also bor
LL | let a = &mut *u.y;
| ---- mutable borrow occurs here (via `*u.y`)
LL | let b = &u.x; //~ ERROR cannot borrow `u` (via `u.x`)
| ^^^ immutable borrow occurs here (via `u.x`)
| ^^^ immutable borrow of `u.x` -- which overlaps with `*u.y` -- occurs here
LL | use_borrow(a);
LL | }
| - mutable borrow ends here
Expand Down

0 comments on commit c2b477c

Please sign in to comment.