Skip to content

Commit bc4f9b8

Browse files
matthewjaspermatthewjasper
committed
Clearer later use messages for calls
Give a special message when the later use is from a call. Use the span of the callee instead of the whole expression. For conflicting borrow messages say that the later use is of the first borrow.
1 parent cc09cb5 commit bc4f9b8

File tree

102 files changed

+607
-636
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

102 files changed

+607
-636
lines changed

‎src/librustc_mir/borrow_check/error_reporting.rs‎

Lines changed: 47 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ use rustc::hir;
1515
use rustc::hir::def_id::DefId;
1616
use rustc::middle::region::ScopeTree;
1717
use rustc::mir::{
18-
self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, FakeReadCause, Field, Local,
19-
LocalDecl, LocalKind, Location, Operand, Place, PlaceProjection, ProjectionElem, Rvalue,
20-
Statement, StatementKind, TerminatorKind, VarBindingForm,
18+
self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, Field, Local,
19+
LocalDecl, LocalKind, Location, Operand, Place, PlaceProjection, ProjectionElem,
20+
Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm,
2121
};
2222
use rustc::ty;
2323
use rustc::util::ppaux::with_highlight_region_for_bound_region;
@@ -262,7 +262,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
262262
move_spans.var_span_label(&mut err, "move occurs due to use in closure");
263263

264264
self.explain_why_borrow_contains_point(context, borrow, None)
265-
.emit(self.infcx.tcx, &mut err);
265+
.emit(self.infcx.tcx, &mut err, String::new());
266266
err.buffer(&mut self.errors_buffer);
267267
}
268268

@@ -299,7 +299,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
299299
});
300300

301301
self.explain_why_borrow_contains_point(context, borrow, None)
302-
.emit(self.infcx.tcx, &mut err);
302+
.emit(self.infcx.tcx, &mut err, String::new());
303303
err.buffer(&mut self.errors_buffer);
304304
}
305305

@@ -319,6 +319,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
319319
let desc_place = self.describe_place(place).unwrap_or("_".to_owned());
320320
let tcx = self.infcx.tcx;
321321

322+
let first_borrow_desc;
323+
322324
// FIXME: supply non-"" `opt_via` when appropriate
323325
let mut err = match (
324326
gen_borrow_kind,
@@ -328,8 +330,23 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
328330
"immutable",
329331
"mutable",
330332
) {
331-
(BorrowKind::Shared, lft, _, BorrowKind::Mut { .. }, _, rgt)
332-
| (BorrowKind::Mut { .. }, _, lft, BorrowKind::Shared, rgt, _) => {
333+
(BorrowKind::Shared, lft, _, BorrowKind::Mut { .. }, _, rgt) => {
334+
first_borrow_desc = "mutable ";
335+
tcx.cannot_reborrow_already_borrowed(
336+
span,
337+
&desc_place,
338+
"",
339+
lft,
340+
issued_span,
341+
"it",
342+
rgt,
343+
"",
344+
None,
345+
Origin::Mir,
346+
)
347+
}
348+
(BorrowKind::Mut { .. }, _, lft, BorrowKind::Shared, rgt, _) => {
349+
first_borrow_desc = "immutable ";
333350
tcx.cannot_reborrow_already_borrowed(
334351
span,
335352
&desc_place,
@@ -345,6 +362,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
345362
}
346363

347364
(BorrowKind::Mut { .. }, _, _, BorrowKind::Mut { .. }, _, _) => {
365+
first_borrow_desc = "first ";
348366
tcx.cannot_mutably_borrow_multiply(
349367
span,
350368
&desc_place,
@@ -357,6 +375,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
357375
}
358376

359377
(BorrowKind::Unique, _, _, BorrowKind::Unique, _, _) => {
378+
first_borrow_desc = "first ";
360379
tcx.cannot_uniquely_borrow_by_two_closures(
361380
span,
362381
&desc_place,
@@ -384,18 +403,22 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
384403
return;
385404
}
386405

387-
(BorrowKind::Unique, _, _, _, _, _) => tcx.cannot_uniquely_borrow_by_one_closure(
388-
span,
389-
&desc_place,
390-
"",
391-
issued_span,
392-
"it",
393-
"",
394-
None,
395-
Origin::Mir,
396-
),
406+
(BorrowKind::Unique, _, _, _, _, _) => {
407+
first_borrow_desc = "first ";
408+
tcx.cannot_uniquely_borrow_by_one_closure(
409+
span,
410+
&desc_place,
411+
"",
412+
issued_span,
413+
"it",
414+
"",
415+
None,
416+
Origin::Mir,
417+
)
418+
},
397419

398420
(BorrowKind::Shared, lft, _, BorrowKind::Unique, _, _) => {
421+
first_borrow_desc = "first ";
399422
tcx.cannot_reborrow_already_uniquely_borrowed(
400423
span,
401424
&desc_place,
@@ -409,6 +432,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
409432
}
410433

411434
(BorrowKind::Mut { .. }, _, lft, BorrowKind::Unique, _, _) => {
435+
first_borrow_desc = "first ";
412436
tcx.cannot_reborrow_already_uniquely_borrowed(
413437
span,
414438
&desc_place,
@@ -459,7 +483,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
459483
}
460484

461485
self.explain_why_borrow_contains_point(context, issued_borrow, None)
462-
.emit(self.infcx.tcx, &mut err);
486+
.emit(self.infcx.tcx, &mut err, first_borrow_desc.to_string());
463487

464488
err.buffer(&mut self.errors_buffer);
465489
}
@@ -614,7 +638,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
614638

615639
if let BorrowExplanation::MustBeValidFor(..) = explanation {
616640
} else {
617-
explanation.emit(self.infcx.tcx, &mut err);
641+
explanation.emit(self.infcx.tcx, &mut err, String::new());
618642
}
619643
} else {
620644
err.span_label(borrow_span, "borrowed value does not live long enough");
@@ -625,7 +649,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
625649

626650
borrow_spans.args_span_label(&mut err, "value captured here");
627651

628-
explanation.emit(self.infcx.tcx, &mut err);
652+
explanation.emit(self.infcx.tcx, &mut err, String::new());
629653
}
630654

631655
err
@@ -685,7 +709,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
685709
_ => {}
686710
}
687711

688-
explanation.emit(self.infcx.tcx, &mut err);
712+
explanation.emit(self.infcx.tcx, &mut err, String::new());
689713

690714
err.buffer(&mut self.errors_buffer);
691715
}
@@ -752,7 +776,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
752776
}
753777
_ => {}
754778
}
755-
explanation.emit(self.infcx.tcx, &mut err);
779+
explanation.emit(self.infcx.tcx, &mut err, String::new());
756780

757781
borrow_spans.args_span_label(&mut err, "value captured here");
758782

@@ -889,7 +913,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
889913
loan_spans.var_span_label(&mut err, "borrow occurs due to use in closure");
890914

891915
self.explain_why_borrow_contains_point(context, loan, None)
892-
.emit(self.infcx.tcx, &mut err);
916+
.emit(self.infcx.tcx, &mut err, String::new());
893917

894918
err.buffer(&mut self.errors_buffer);
895919
}
@@ -1262,21 +1286,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
12621286
}
12631287
}
12641288

1265-
/// Returns the `FakeReadCause` at this location if it is a `FakeRead` statement.
1266-
pub(super) fn retrieve_fake_read_cause_for_location(
1267-
&self,
1268-
location: &Location,
1269-
) -> Option<FakeReadCause> {
1270-
let stmt = self.mir.basic_blocks()[location.block]
1271-
.statements
1272-
.get(location.statement_index)?;
1273-
if let StatementKind::FakeRead(cause, _) = stmt.kind {
1274-
Some(cause)
1275-
} else {
1276-
None
1277-
}
1278-
}
1279-
12801289
fn classify_drop_access_kind(&self, place: &Place<'tcx>) -> StorageDeadOrDrop<'tcx> {
12811290
let tcx = self.infcx.tcx;
12821291
match place {

‎src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs‎

Lines changed: 77 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,53 +9,69 @@
99
// except according to those terms.
1010

1111
use borrow_check::borrow_set::BorrowData;
12+
use borrow_check::error_reporting::UseSpans;
1213
use borrow_check::nll::region_infer::Cause;
1314
use borrow_check::{Context, MirBorrowckCtxt, WriteKind};
1415
use rustc::ty::{Region, TyCtxt};
15-
use rustc::mir::{FakeReadCause, Location, Place, TerminatorKind};
16+
use rustc::mir::{FakeReadCause, Location, Operand, Place, StatementKind, TerminatorKind};
1617
use rustc_errors::DiagnosticBuilder;
1718
use syntax_pos::Span;
1819
use syntax_pos::symbol::Symbol;
1920

2021
mod find_use;
2122

2223
pub(in borrow_check) enum BorrowExplanation<'tcx> {
23-
UsedLater(bool, Option<FakeReadCause>, Span),
24-
UsedLaterInLoop(bool, Span),
24+
UsedLater(LaterUseKind, Span),
25+
UsedLaterInLoop(LaterUseKind, Span),
2526
UsedLaterWhenDropped(Span, Symbol, bool),
2627
MustBeValidFor(Region<'tcx>),
2728
Unexplained,
2829
}
2930

31+
#[derive(Clone, Copy)]
32+
pub(in borrow_check) enum LaterUseKind {
33+
ClosureCapture,
34+
Call,
35+
FakeLetRead,
36+
Other,
37+
}
38+
3039
impl<'tcx> BorrowExplanation<'tcx> {
3140
pub(in borrow_check) fn emit<'cx, 'gcx>(
3241
&self,
3342
tcx: TyCtxt<'cx, 'gcx, 'tcx>,
34-
err: &mut DiagnosticBuilder<'_>
43+
err: &mut DiagnosticBuilder<'_>,
44+
borrow_desc: String,
3545
) {
3646
match *self {
37-
BorrowExplanation::UsedLater(is_in_closure, fake_read_cause, var_or_use_span) => {
38-
let message = if is_in_closure {
39-
"borrow later captured here by closure"
40-
} else if let Some(FakeReadCause::ForLet) = fake_read_cause {
41-
"borrow later stored here"
42-
} else {
43-
"borrow later used here"
47+
BorrowExplanation::UsedLater(later_use_kind, var_or_use_span) => {
48+
let message = borrow_desc + match later_use_kind {
49+
LaterUseKind::ClosureCapture => "borrow later captured here by closure",
50+
LaterUseKind::Call => "borrow later used by call",
51+
LaterUseKind::FakeLetRead => "borrow later stored here",
52+
LaterUseKind::Other => "borrow later used here",
4453
};
4554
err.span_label(var_or_use_span, message);
4655
},
47-
BorrowExplanation::UsedLaterInLoop(is_in_closure, var_or_use_span) => {
48-
let message = if is_in_closure {
49-
"borrow captured here by closure, in later iteration of loop"
50-
} else {
51-
"borrow used here, in later iteration of loop"
56+
BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span) => {
57+
let message = borrow_desc + match later_use_kind {
58+
LaterUseKind::ClosureCapture => {
59+
"borrow captured here by closure, in later iteration of loop"
60+
},
61+
LaterUseKind::Call => "borrow used by call, in later iteration of loop",
62+
LaterUseKind::FakeLetRead => "borrow later stored here",
63+
LaterUseKind::Other => "borrow used here, in later iteration of loop",
5264
};
5365
err.span_label(var_or_use_span, message);
5466
},
5567
BorrowExplanation::UsedLaterWhenDropped(span, local_name, should_note_order) => {
5668
err.span_label(
5769
span,
58-
format!("borrow later used here, when `{}` is dropped", local_name),
70+
format!(
71+
"{}borrow later used here, when `{}` is dropped",
72+
borrow_desc,
73+
local_name,
74+
),
5975
);
6076

6177
if should_note_order {
@@ -68,7 +84,7 @@ impl<'tcx> BorrowExplanation<'tcx> {
6884
BorrowExplanation::MustBeValidFor(region) => {
6985
tcx.note_and_explain_free_region(
7086
err,
71-
"borrowed value must be valid for ",
87+
&(borrow_desc + "borrowed value must be valid for "),
7288
region,
7389
"...",
7490
);
@@ -127,16 +143,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
127143
.or_else(|| self.borrow_spans(span, location));
128144

129145
if self.is_borrow_location_in_loop(context.loc) {
130-
BorrowExplanation::UsedLaterInLoop(spans.for_closure(), spans.var_or_use())
146+
let later_use = self.later_use_kind(spans, location);
147+
BorrowExplanation::UsedLaterInLoop(later_use.0, later_use.1)
131148
} else {
132149
// Check if the location represents a `FakeRead`, and adapt the error
133150
// message to the `FakeReadCause` it is from: in particular,
134151
// the ones inserted in optimized `let var = <expr>` patterns.
135-
BorrowExplanation::UsedLater(
136-
spans.for_closure(),
137-
self.retrieve_fake_read_cause_for_location(&location),
138-
spans.var_or_use()
139-
)
152+
let later_use = self.later_use_kind(spans, location);
153+
BorrowExplanation::UsedLater(later_use.0, later_use.1)
140154
}
141155
}
142156

@@ -246,4 +260,43 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
246260

247261
false
248262
}
263+
264+
fn later_use_kind(&self, use_spans: UseSpans, location: Location) -> (LaterUseKind, Span) {
265+
use self::LaterUseKind::*;
266+
267+
let block = &self.mir.basic_blocks()[location.block];
268+
match use_spans {
269+
UseSpans::ClosureUse { var_span, .. } => (LaterUseKind::ClosureCapture, var_span),
270+
UseSpans::OtherUse(span) => {
271+
(if let Some(stmt) = block.statements.get(location.statement_index) {
272+
match stmt.kind {
273+
StatementKind::FakeRead(FakeReadCause::ForLet, _) => FakeLetRead,
274+
_ => Other,
275+
}
276+
} else {
277+
assert_eq!(location.statement_index, block.statements.len());
278+
match block.terminator().kind {
279+
TerminatorKind::Call { ref func, from_hir_call: true, .. } => {
280+
// Just point to the function, to reduce the chance
281+
// of overlapping spans.
282+
let function_span = match func {
283+
Operand::Constant(c) => c.span,
284+
Operand::Copy(Place::Local(l)) | Operand::Move(Place::Local(l)) => {
285+
let local_decl = &self.mir.local_decls[*l];
286+
if local_decl.name.is_none() {
287+
local_decl.source_info.span
288+
} else {
289+
span
290+
}
291+
},
292+
_ => span,
293+
};
294+
return (Call, function_span);
295+
},
296+
_ => Other,
297+
}
298+
}, span)
299+
}
300+
}
301+
}
249302
}

‎src/test/ui/E0501.ast.nll.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ LL | outside_closure_1(a); //[ast]~ ERROR cannot borrow `*a` as mutable beca
1010
| ^ borrow occurs here
1111
...
1212
LL | drop(bar);
13-
| --- borrow later used here
13+
| --- first borrow later used here
1414

1515
error[E0501]: cannot borrow `*a` as immutable because previous closure requires unique access
1616
--> $DIR/E0501.rs:31:23
@@ -24,7 +24,7 @@ LL | outside_closure_2(a); //[ast]~ ERROR cannot borrow `*a` as immutable be
2424
| ^ borrow occurs here
2525
...
2626
LL | drop(bar);
27-
| --- borrow later used here
27+
| --- first borrow later used here
2828

2929
error: aborting due to 2 previous errors
3030

‎src/test/ui/E0501.mir.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ LL | outside_closure_1(a); //[ast]~ ERROR cannot borrow `*a` as mutable beca
1010
| ^ borrow occurs here
1111
...
1212
LL | drop(bar);
13-
| --- borrow later used here
13+
| --- first borrow later used here
1414

1515
error[E0501]: cannot borrow `*a` as immutable because previous closure requires unique access
1616
--> $DIR/E0501.rs:31:23
@@ -24,7 +24,7 @@ LL | outside_closure_2(a); //[ast]~ ERROR cannot borrow `*a` as immutable be
2424
| ^ borrow occurs here
2525
...
2626
LL | drop(bar);
27-
| --- borrow later used here
27+
| --- first borrow later used here
2828

2929
error: aborting due to 2 previous errors
3030

0 commit comments

Comments
 (0)