Skip to content

Commit

Permalink
Auto merge of #54666 - matthewjasper:mir-function-spans, r=pnkfelix
Browse files Browse the repository at this point in the history
[NLL] Improve "borrow later used here" messages

* In the case of two conflicting borrows, the later used message says which borrow it's referring to
* If the later use is a function call (from the users point of view) say that the later use is for the call. Point just to the function.

r? @pnkfelix
Closes #48643
  • Loading branch information
bors committed Oct 4, 2018
2 parents 5de5281 + bc4f9b8 commit a57f1c9
Show file tree
Hide file tree
Showing 119 changed files with 665 additions and 653 deletions.
4 changes: 3 additions & 1 deletion src/librustc/ich/impls_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,13 @@ for mir::TerminatorKind<'gcx> {
mir::TerminatorKind::Call { ref func,
ref args,
ref destination,
cleanup } => {
cleanup,
from_hir_call, } => {
func.hash_stable(hcx, hasher);
args.hash_stable(hcx, hasher);
destination.hash_stable(hcx, hasher);
cleanup.hash_stable(hcx, hasher);
from_hir_call.hash_stable(hcx, hasher);
}
mir::TerminatorKind::Assert { ref cond,
expected,
Expand Down
5 changes: 5 additions & 0 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,9 @@ pub enum TerminatorKind<'tcx> {
destination: Option<(Place<'tcx>, BasicBlock)>,
/// Cleanups to be done if the call unwinds.
cleanup: Option<BasicBlock>,
/// Whether this is from a call in HIR, rather than from an overloaded
/// operator. True for overloaded function call.
from_hir_call: bool,
},

/// Jump to the target if the condition has the expected value,
Expand Down Expand Up @@ -2805,6 +2808,7 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
ref args,
ref destination,
cleanup,
from_hir_call,
} => {
let dest = destination
.as_ref()
Expand All @@ -2815,6 +2819,7 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
args: args.fold_with(folder),
destination: dest,
cleanup,
from_hir_call,
}
}
Assert {
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,8 @@ macro_rules! make_mir_visitor {
TerminatorKind::Call { ref $($mutability)* func,
ref $($mutability)* args,
ref $($mutability)* destination,
cleanup } => {
cleanup,
from_hir_call: _, } => {
self.visit_operand(func, source_location);
for arg in args {
self.visit_operand(arg, source_location);
Expand Down
8 changes: 7 additions & 1 deletion src/librustc_codegen_llvm/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,13 @@ impl FunctionCx<'a, 'll, 'tcx> {
bug!("undesugared DropAndReplace in codegen: {:?}", terminator);
}

mir::TerminatorKind::Call { ref func, ref args, ref destination, cleanup } => {
mir::TerminatorKind::Call {
ref func,
ref args,
ref destination,
cleanup,
from_hir_call: _
} => {
// Create the callee. This is a fn ptr or zero-sized and hence a kind of scalar.
let callee = self.codegen_operand(&bx, func);

Expand Down
85 changes: 47 additions & 38 deletions src/librustc_mir/borrow_check/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ use rustc::hir;
use rustc::hir::def_id::DefId;
use rustc::middle::region::ScopeTree;
use rustc::mir::{
self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, FakeReadCause, Field, Local,
LocalDecl, LocalKind, Location, Operand, Place, PlaceProjection, ProjectionElem, Rvalue,
Statement, StatementKind, TerminatorKind, VarBindingForm,
self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, Field, Local,
LocalDecl, LocalKind, Location, Operand, Place, PlaceProjection, ProjectionElem,
Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm,
};
use rustc::ty;
use rustc::util::ppaux::with_highlight_region_for_bound_region;
Expand Down Expand Up @@ -262,7 +262,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
move_spans.var_span_label(&mut err, "move occurs due to use in closure");

self.explain_why_borrow_contains_point(context, borrow, None)
.emit(self.infcx.tcx, &mut err);
.emit(self.infcx.tcx, &mut err, String::new());
err.buffer(&mut self.errors_buffer);
}

Expand Down Expand Up @@ -299,7 +299,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
});

self.explain_why_borrow_contains_point(context, borrow, None)
.emit(self.infcx.tcx, &mut err);
.emit(self.infcx.tcx, &mut err, String::new());
err.buffer(&mut self.errors_buffer);
}

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

let first_borrow_desc;

// FIXME: supply non-"" `opt_via` when appropriate
let mut err = match (
gen_borrow_kind,
Expand All @@ -328,8 +330,23 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
"immutable",
"mutable",
) {
(BorrowKind::Shared, lft, _, BorrowKind::Mut { .. }, _, rgt)
| (BorrowKind::Mut { .. }, _, lft, BorrowKind::Shared, rgt, _) => {
(BorrowKind::Shared, lft, _, BorrowKind::Mut { .. }, _, rgt) => {
first_borrow_desc = "mutable ";
tcx.cannot_reborrow_already_borrowed(
span,
&desc_place,
"",
lft,
issued_span,
"it",
rgt,
"",
None,
Origin::Mir,
)
}
(BorrowKind::Mut { .. }, _, lft, BorrowKind::Shared, rgt, _) => {
first_borrow_desc = "immutable ";
tcx.cannot_reborrow_already_borrowed(
span,
&desc_place,
Expand All @@ -345,6 +362,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}

(BorrowKind::Mut { .. }, _, _, BorrowKind::Mut { .. }, _, _) => {
first_borrow_desc = "first ";
tcx.cannot_mutably_borrow_multiply(
span,
&desc_place,
Expand All @@ -357,6 +375,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}

(BorrowKind::Unique, _, _, BorrowKind::Unique, _, _) => {
first_borrow_desc = "first ";
tcx.cannot_uniquely_borrow_by_two_closures(
span,
&desc_place,
Expand Down Expand Up @@ -384,18 +403,22 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
return;
}

(BorrowKind::Unique, _, _, _, _, _) => tcx.cannot_uniquely_borrow_by_one_closure(
span,
&desc_place,
"",
issued_span,
"it",
"",
None,
Origin::Mir,
),
(BorrowKind::Unique, _, _, _, _, _) => {
first_borrow_desc = "first ";
tcx.cannot_uniquely_borrow_by_one_closure(
span,
&desc_place,
"",
issued_span,
"it",
"",
None,
Origin::Mir,
)
},

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

(BorrowKind::Mut { .. }, _, lft, BorrowKind::Unique, _, _) => {
first_borrow_desc = "first ";
tcx.cannot_reborrow_already_uniquely_borrowed(
span,
&desc_place,
Expand Down Expand Up @@ -459,7 +483,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}

self.explain_why_borrow_contains_point(context, issued_borrow, None)
.emit(self.infcx.tcx, &mut err);
.emit(self.infcx.tcx, &mut err, first_borrow_desc.to_string());

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

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

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

explanation.emit(self.infcx.tcx, &mut err);
explanation.emit(self.infcx.tcx, &mut err, String::new());
}

err
Expand Down Expand Up @@ -685,7 +709,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
_ => {}
}

explanation.emit(self.infcx.tcx, &mut err);
explanation.emit(self.infcx.tcx, &mut err, String::new());

err.buffer(&mut self.errors_buffer);
}
Expand Down Expand Up @@ -752,7 +776,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
_ => {}
}
explanation.emit(self.infcx.tcx, &mut err);
explanation.emit(self.infcx.tcx, &mut err, String::new());

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

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

self.explain_why_borrow_contains_point(context, loan, None)
.emit(self.infcx.tcx, &mut err);
.emit(self.infcx.tcx, &mut err, String::new());

err.buffer(&mut self.errors_buffer);
}
Expand Down Expand Up @@ -1262,21 +1286,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
}

/// Returns the `FakeReadCause` at this location if it is a `FakeRead` statement.
pub(super) fn retrieve_fake_read_cause_for_location(
&self,
location: &Location,
) -> Option<FakeReadCause> {
let stmt = self.mir.basic_blocks()[location.block]
.statements
.get(location.statement_index)?;
if let StatementKind::FakeRead(cause, _) = stmt.kind {
Some(cause)
} else {
None
}
}

fn classify_drop_access_kind(&self, place: &Place<'tcx>) -> StorageDeadOrDrop<'tcx> {
let tcx = self.infcx.tcx;
match place {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,7 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx
ref args,
ref destination,
cleanup: _,
from_hir_call: _,
} => {
self.consume_operand(ContextKind::CallOperator.new(loc), (func, span), flow_state);
for arg in args {
Expand Down
Loading

0 comments on commit a57f1c9

Please sign in to comment.