Skip to content

Commit c2b477c

Browse files
davidtwcodavidtwco
committed
Improve diagnostic labels and add note.
This commit improves diagnostic labels to mention which field a borrow overlaps with and adds a note explaining that the fields overlap.
1 parent 388dffe commit c2b477c

13 files changed

+86
-43
lines changed

‎src/librustc_borrowck/borrowck/check_loans.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -557,12 +557,8 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
557557
if new_loan.loan_path.has_fork(&old_loan.loan_path) && common.is_some() {
558558
let nl = self.bccx.loan_path_to_string(&common.unwrap());
559559
let ol = nl.clone();
560-
let new_loan_msg = format!(" (via `{}`)",
561-
self.bccx.loan_path_to_string(
562-
&new_loan.loan_path));
563-
let old_loan_msg = format!(" (via `{}`)",
564-
self.bccx.loan_path_to_string(
565-
&old_loan.loan_path));
560+
let new_loan_msg = self.bccx.loan_path_to_string(&new_loan.loan_path);
561+
let old_loan_msg = self.bccx.loan_path_to_string(&old_loan.loan_path);
566562
(nl, ol, new_loan_msg, old_loan_msg)
567563
} else {
568564
(self.bccx.loan_path_to_string(&new_loan.loan_path),

‎src/librustc_mir/borrow_check/error_reporting.rs‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -329,12 +329,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
329329
"closure"
330330
};
331331

332-
let (desc_place, msg_place, msg_borrow) = self.describe_place_for_conflicting_borrow(
333-
place, &issued_borrow.borrowed_place,
334-
);
335-
let via = |msg: String| if msg.is_empty() { msg } else { format!(" (via `{}`)", msg) };
336-
let msg_place = via(msg_place);
337-
let msg_borrow = via(msg_borrow);
332+
let (desc_place, msg_place, msg_borrow, union_type_name) =
333+
self.describe_place_for_conflicting_borrow(place, &issued_borrow.borrowed_place);
338334

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

515+
if union_type_name != "" {
516+
err.note(&format!(
517+
"`{}` is a field of the union `{}`, so it overlaps the field `{}`",
518+
msg_place, union_type_name, msg_borrow,
519+
));
520+
}
521+
519522
explanation
520523
.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, first_borrow_desc);
521524

@@ -549,7 +552,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
549552
&self,
550553
first_borrowed_place: &Place<'tcx>,
551554
second_borrowed_place: &Place<'tcx>,
552-
) -> (String, String, String) {
555+
) -> (String, String, String, String) {
553556
// Define a small closure that we can use to check if the type of a place
554557
// is a union.
555558
let is_union = |place: &Place<'tcx>| -> bool {
@@ -600,7 +603,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
600603
.unwrap_or_else(|| "_".to_owned());
601604
let desc_second = self.describe_place(second_borrowed_place)
602605
.unwrap_or_else(|| "_".to_owned());
603-
return Some((desc_base, desc_first, desc_second));
606+
607+
// Also compute the name of the union type, eg. `Foo` so we
608+
// can add a helpful note with it.
609+
let ty = base.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx);
610+
611+
return Some((desc_base, desc_first, desc_second, ty.to_string()));
604612
},
605613
_ => current = base,
606614
}
@@ -612,7 +620,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
612620
// only return the description of the first place.
613621
let desc_place = self.describe_place(first_borrowed_place)
614622
.unwrap_or_else(|| "_".to_owned());
615-
(desc_place, "".to_string(), "".to_string())
623+
(desc_place, "".to_string(), "".to_string(), "".to_string())
616624
})
617625
}
618626

‎src/librustc_mir/util/borrowck_errors.rs‎

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,15 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
138138
old_load_end_span: Option<Span>,
139139
o: Origin,
140140
) -> DiagnosticBuilder<'cx> {
141+
let via = |msg: &str|
142+
if msg.is_empty() { msg.to_string() } else { format!(" (via `{}`)", msg) };
141143
let mut err = struct_span_err!(
142144
self,
143145
new_loan_span,
144146
E0499,
145147
"cannot borrow `{}`{} as mutable more than once at a time{OGN}",
146148
desc,
147-
opt_via,
149+
via(opt_via),
148150
OGN = o
149151
);
150152
if old_loan_span == new_loan_span {
@@ -164,11 +166,11 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
164166
} else {
165167
err.span_label(
166168
old_loan_span,
167-
format!("first mutable borrow occurs here{}", old_opt_via),
169+
format!("first mutable borrow occurs here{}", via(old_opt_via)),
168170
);
169171
err.span_label(
170172
new_loan_span,
171-
format!("second mutable borrow occurs here{}", opt_via),
173+
format!("second mutable borrow occurs here{}", via(opt_via)),
172174
);
173175
if let Some(old_load_end_span) = old_load_end_span {
174176
err.span_label(old_load_end_span, "first borrow ends here");
@@ -292,27 +294,46 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
292294
old_load_end_span: Option<Span>,
293295
o: Origin,
294296
) -> DiagnosticBuilder<'cx> {
297+
let via = |msg: &str|
298+
if msg.is_empty() { msg.to_string() } else { format!(" (via `{}`)", msg) };
295299
let mut err = struct_span_err!(
296300
self,
297301
span,
298302
E0502,
299-
"cannot borrow `{}`{} as {} because {} is also borrowed as {}{}{OGN}",
303+
"cannot borrow `{}`{} as {} because {} is also borrowed \
304+
as {}{}{OGN}",
300305
desc_new,
301-
msg_new,
306+
via(msg_new),
302307
kind_new,
303308
noun_old,
304309
kind_old,
305-
msg_old,
310+
via(msg_old),
306311
OGN = o
307312
);
308-
err.span_label(span, format!("{} borrow occurs here{}", kind_new, msg_new));
309-
err.span_label(
310-
old_span,
311-
format!("{} borrow occurs here{}", kind_old, msg_old),
312-
);
313+
314+
if msg_new == "" {
315+
// If `msg_new` is empty, then this isn't a borrow of a union field.
316+
err.span_label(span, format!("{} borrow occurs here", kind_new));
317+
err.span_label(old_span, format!("{} borrow occurs here", kind_old));
318+
} else {
319+
// If `msg_new` isn't empty, then this a borrow of a union field.
320+
err.span_label(
321+
span,
322+
format!(
323+
"{} borrow of `{}` -- which overlaps with `{}` -- occurs here",
324+
kind_new, msg_new, msg_old,
325+
)
326+
);
327+
err.span_label(
328+
old_span,
329+
format!("{} borrow occurs here{}", kind_old, via(msg_old)),
330+
);
331+
}
332+
313333
if let Some(old_load_end_span) = old_load_end_span {
314334
err.span_label(old_load_end_span, format!("{} borrow ends here", kind_old));
315335
}
336+
316337
self.cancel_if_wrong_origin(err, o)
317338
}
318339

‎src/test/ui/borrowck/borrowck-borrow-from-owned-ptr.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ error[E0502]: cannot borrow `foo` (via `foo.bar2`) as immutable because `foo` is
138138
LL | let bar1 = &mut foo.bar1;
139139
| -------- mutable borrow occurs here (via `foo.bar1`)
140140
LL | let _foo1 = &foo.bar2; //~ ERROR cannot borrow
141-
| ^^^^^^^^ immutable borrow occurs here (via `foo.bar2`)
141+
| ^^^^^^^^ immutable borrow of `foo.bar2` -- which overlaps with `foo.bar1` -- occurs here
142142
LL | *bar1;
143143
LL | }
144144
| - mutable borrow ends here

‎src/test/ui/borrowck/borrowck-box-insensitivity.ast.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ error[E0502]: cannot borrow `a` (via `a.y`) as immutable because `a` is also bor
6161
LL | let _x = &mut a.x;
6262
| --- mutable borrow occurs here (via `a.x`)
6363
LL | let _y = &a.y; //[ast]~ ERROR cannot borrow
64-
| ^^^ immutable borrow occurs here (via `a.y`)
64+
| ^^^ immutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here
6565
...
6666
LL | }
6767
| - mutable borrow ends here
@@ -72,7 +72,7 @@ error[E0502]: cannot borrow `a` (via `a.y`) as mutable because `a` is also borro
7272
LL | let _x = &a.x;
7373
| --- immutable borrow occurs here (via `a.x`)
7474
LL | let _y = &mut a.y; //[ast]~ ERROR cannot borrow
75-
| ^^^ mutable borrow occurs here (via `a.y`)
75+
| ^^^ mutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here
7676
...
7777
LL | }
7878
| - immutable borrow ends here

‎src/test/ui/borrowck/borrowck-box-insensitivity.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,14 @@ fn borrow_after_mut_borrow() {
8383
let mut a: Box<_> = box A { x: box 0, y: 1 };
8484
let _x = &mut a.x;
8585
let _y = &a.y; //[ast]~ ERROR cannot borrow
86-
//[ast]~^ immutable borrow occurs here (via `a.y`)
86+
//[ast]~^ immutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here
8787
use_mut(_x);
8888
}
8989
fn mut_borrow_after_borrow() {
9090
let mut a: Box<_> = box A { x: box 0, y: 1 };
9191
let _x = &a.x;
9292
let _y = &mut a.y; //[ast]~ ERROR cannot borrow
93-
//[ast]~^ mutable borrow occurs here (via `a.y`)
93+
//[ast]~^ mutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here
9494
use_imm(_x);
9595
}
9696
fn copy_after_move_nested() {

‎src/test/ui/borrowck/borrowck-union-borrow.nll.stderr‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@ error[E0502]: cannot borrow `u` (via `u.b`) as mutable because it is also borrow
2424
LL | let ra = &u.a;
2525
| ---- immutable borrow occurs here (via `u.a`)
2626
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`)
27-
| ^^^^^^^^ mutable borrow occurs here (via `u.b`)
27+
| ^^^^^^^^ mutable borrow of `u.b` -- which overlaps with `u.a` -- occurs here
2828
LL | drop(ra);
2929
| -- immutable borrow later used here
30+
|
31+
= note: `u.b` is a field of the union `U`, so it overlaps the field `u.a`
3032

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

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

112118
error[E0506]: cannot assign to `u.b` because it is borrowed
113119
--> $DIR/borrowck-union-borrow.rs:94:13

‎src/test/ui/borrowck/borrowck-union-borrow.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ error[E0502]: cannot borrow `u` (via `u.b`) as mutable because `u` is also borro
2323
LL | let ra = &u.a;
2424
| --- immutable borrow occurs here (via `u.a`)
2525
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`)
26-
| ^^^ mutable borrow occurs here (via `u.b`)
26+
| ^^^ mutable borrow of `u.b` -- which overlaps with `u.a` -- occurs here
2727
LL | drop(ra);
2828
LL | }
2929
| - immutable borrow ends here
@@ -80,7 +80,7 @@ error[E0502]: cannot borrow `u` (via `u.b`) as immutable because `u` is also bor
8080
LL | let rma = &mut u.a;
8181
| --- mutable borrow occurs here (via `u.a`)
8282
LL | let rb = &u.b; //~ ERROR cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`)
83-
| ^^^ immutable borrow occurs here (via `u.b`)
83+
| ^^^ immutable borrow of `u.b` -- which overlaps with `u.a` -- occurs here
8484
LL | drop(rma);
8585
LL | }
8686
| - mutable borrow ends here

‎src/test/ui/issues/issue-17263.ast.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ error[E0502]: cannot borrow `foo` (via `foo.b`) as immutable because `foo` is al
1313
--> $DIR/issue-17263.rs:21:32
1414
|
1515
LL | let (c, d) = (&mut foo.a, &foo.b);
16-
| ----- ^^^^^ immutable borrow occurs here (via `foo.b`)
16+
| ----- ^^^^^ immutable borrow of `foo.b` -- which overlaps with `foo.a` -- occurs here
1717
| |
1818
| mutable borrow occurs here (via `foo.a`)
1919
...

‎src/test/ui/issues/issue-45157.stderr‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@ LL | let mref = &mut u.s.a;
55
| ---------- mutable borrow occurs here (via `u.s.a`)
66
...
77
LL | let nref = &u.z.c;
8-
| ^^^^^^ immutable borrow occurs here (via `u.z.c`)
8+
| ^^^^^^ immutable borrow of `u.z.c` -- which overlaps with `u.s.a` -- occurs here
99
LL | //~^ ERROR cannot borrow `u` (via `u.z.c`) as immutable because it is also borrowed as mutable (via `u.s.a`) [E0502]
1010
LL | println!("{} {}", mref, nref)
1111
| ---- mutable borrow later used here
12+
|
13+
= note: `u.z.c` is a field of the union `U`, so it overlaps the field `u.s.a`
1214

1315
error: aborting due to previous error
1416

0 commit comments

Comments
 (0)