Skip to content

Commit

Permalink
Auto merge of #48592 - spastorino:borrowed_value_error, r=nikomatsakis
Browse files Browse the repository at this point in the history 
[NLL] Avoid borrowed value must be valid for lifetime '_#2r..." in errors

Closes #48428

- [x] If NLL is enabled, [do not invoke `note_and_explain_region`](#48428 (comment))
- [x] Modify `-Zdump-nll-cause` to not print [the overwhelming debug output here](https://github.com/rust-lang/rust/blob/master/src/librustc_mir/borrow_check/nll/region_infer/mod.rs#L1288-L1299). This way we should I believe at least get nice-ish output for [our original example](#48428 (comment)).
- [x] Extend `explain_why_borrow_contains_point` to also work for "universal lifetimes" like the `'a` in [the example at the end of this comment](#48428 (comment)).
- [ ] Figure out how to enable causal information all the time (but that is #46590).
  • Loading branch information
@bors
bors committed Mar 4, 2018
2 parents 1e1bfc7 + 99c42dc commit 259e4a6
Show file tree
Hide file tree
Showing 18 changed files with 787 additions and 563 deletions.
833 changes: 465 additions & 368 deletions src/librustc/infer/error_reporting/mod.rs
View file

Large diffs are not rendered by default.

196 changes: 125 additions & 71 deletions src/librustc_mir/borrow_check/error_reporting.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc::ty::{self, RegionKind};
use rustc_data_structures::indexed_vec::Idx;
use rustc_data_structures::sync::Lrc;

use super::{MirBorrowckCtxt, Context};
use super::{Context, MirBorrowckCtxt};
use super::{InitializationRequiringAction, PrefixSet};
use dataflow::{ActiveBorrows, BorrowData, FlowAtLocation, MovingOutStatements};
use dataflow::move_paths::MovePathIndex;
Expand Down Expand Up @@ -96,7 +96,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
} else {
true
}
},
}
_ => true,
};

Expand All @@ -106,9 +106,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
None => "value".to_owned(),
};

err.note(&format!("move occurs because {} has type `{}`, \
which does not implement the `Copy` trait",
note_msg, ty));
err.note(&format!(
"move occurs because {} has type `{}`, \
which does not implement the `Copy` trait",
note_msg, ty
));
}
}

Expand Down Expand Up @@ -154,7 +156,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
span,
&self.describe_place(place).unwrap_or("_".to_owned()),
self.retrieve_borrow_span(borrow),
&self.describe_place(&borrow.borrowed_place).unwrap_or("_".to_owned()),
&self.describe_place(&borrow.borrowed_place)
.unwrap_or("_".to_owned()),
Origin::Mir,
);

Expand All @@ -175,8 +178,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
use rustc::hir::ExprClosure;
use rustc::mir::AggregateKind;

let local = match self.mir[location.block].statements.get(location.statement_index) {
Some(&Statement { kind: StatementKind::Assign(Place::Local(local), _), .. }) => local,
let local = match self.mir[location.block]
.statements
.get(location.statement_index)
{
Some(&Statement {
kind: StatementKind::Assign(Place::Local(local), _),
..
}) => local,
_ => return None,
};

Expand All @@ -202,8 +211,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
.with_freevars(node_id, |freevars| {
for (v, place) in freevars.iter().zip(places) {
match *place {
Operand::Copy(Place::Local(l)) |
Operand::Move(Place::Local(l)) if local == l =>
Operand::Copy(Place::Local(l))
| Operand::Move(Place::Local(l)) if local == l =>
{
debug!(
"find_closure_span: found captured local {:?}",
Expand Down Expand Up @@ -232,7 +241,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
context: Context,
(place, span): (&Place<'tcx>, Span),
gen_borrow_kind: BorrowKind,
issued_borrow: &BorrowData,
issued_borrow: &BorrowData<'tcx>,
end_issued_loan_span: Option<Span>,
) {
let issued_span = self.retrieve_borrow_span(issued_borrow);
Expand All @@ -255,8 +264,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
"immutable",
"mutable",
) {
(BorrowKind::Shared, lft, _, BorrowKind::Mut { .. }, _, rgt) |
(BorrowKind::Mut { .. }, _, lft, BorrowKind::Shared, rgt, _) => self.tcx
(BorrowKind::Shared, lft, _, BorrowKind::Mut { .. }, _, rgt)
| (BorrowKind::Mut { .. }, _, lft, BorrowKind::Shared, rgt, _) => self.tcx
.cannot_reborrow_already_borrowed(
span,
&desc_place,
Expand Down Expand Up @@ -355,25 +364,32 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
context: Context,
borrow: &BorrowData<'tcx>,
drop_span: Span,
borrows: &ActiveBorrows<'cx, 'gcx, 'tcx>
borrows: &ActiveBorrows<'cx, 'gcx, 'tcx>,
) {
let end_span = borrows.opt_region_end_span(&borrow.region);
let scope_tree = borrows.0.scope_tree();
let root_place = self.prefixes(&borrow.borrowed_place, PrefixSet::All).last().unwrap();
let root_place = self.prefixes(&borrow.borrowed_place, PrefixSet::All)
.last()
.unwrap();

let borrow_span = self.mir.source_info(borrow.location).span;
let proper_span = match *root_place {
Place::Local(local) => self.mir.local_decls[local].source_info.span,
_ => drop_span,
};

if self.access_place_error_reported.contains(&(root_place.clone(), borrow_span)) {
debug!("suppressing access_place error when borrow doesn't live long enough for {:?}",
borrow_span);
if self.access_place_error_reported
.contains(&(root_place.clone(), borrow_span))
{
debug!(
"suppressing access_place error when borrow doesn't live long enough for {:?}",
borrow_span
);
return;
}

self.access_place_error_reported.insert((root_place.clone(), borrow_span));
self.access_place_error_reported
.insert((root_place.clone(), borrow_span));

match (borrow.region, &self.describe_place(&borrow.borrowed_place)) {
(RegionKind::ReScope(_), Some(name)) => {
Expand All @@ -385,9 +401,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
drop_span,
borrow_span,
proper_span,
end_span
end_span,
);
},
}
(RegionKind::ReScope(_), None) => {
self.report_scoped_temporary_value_does_not_live_long_enough(
context,
Expand All @@ -396,14 +412,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
drop_span,
borrow_span,
proper_span,
end_span
end_span,
);
},
(RegionKind::ReEarlyBound(_), Some(name)) |
(RegionKind::ReFree(_), Some(name)) |
(RegionKind::ReStatic, Some(name)) |
(RegionKind::ReEmpty, Some(name)) |
(RegionKind::ReVar(_), Some(name)) => {
}
(RegionKind::ReEarlyBound(_), Some(name))
| (RegionKind::ReFree(_), Some(name))
| (RegionKind::ReStatic, Some(name))
| (RegionKind::ReEmpty, Some(name))
| (RegionKind::ReVar(_), Some(name)) => {
self.report_unscoped_local_value_does_not_live_long_enough(
context,
name,
Expand All @@ -414,12 +430,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
proper_span,
end_span,
);
},
(RegionKind::ReEarlyBound(_), None) |
(RegionKind::ReFree(_), None) |
(RegionKind::ReStatic, None) |
(RegionKind::ReEmpty, None) |
(RegionKind::ReVar(_), None) => {
}
(RegionKind::ReEarlyBound(_), None)
| (RegionKind::ReFree(_), None)
| (RegionKind::ReStatic, None)
| (RegionKind::ReEmpty, None)
| (RegionKind::ReVar(_), None) => {
self.report_unscoped_temporary_value_does_not_live_long_enough(
context,
&scope_tree,
Expand All @@ -429,13 +445,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
proper_span,
end_span,
);
},
(RegionKind::ReLateBound(_, _), _) |
(RegionKind::ReSkolemized(_, _), _) |
(RegionKind::ReClosureBound(_), _) |
(RegionKind::ReErased, _) => {
}
(RegionKind::ReLateBound(_, _), _)
| (RegionKind::ReSkolemized(_, _), _)
| (RegionKind::ReClosureBound(_), _)
| (RegionKind::ReErased, _) => {
span_bug!(drop_span, "region does not make sense in this context");
},
}
}
}

Expand All @@ -450,11 +466,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
_proper_span: Span,
end_span: Option<Span>,
) {
let mut err = self.tcx.path_does_not_live_long_enough(borrow_span,
&format!("`{}`", name),
Origin::Mir);
let mut err = self.tcx.path_does_not_live_long_enough(
borrow_span,
&format!("`{}`", name),
Origin::Mir,
);
err.span_label(borrow_span, "borrowed value does not live long enough");
err.span_label(drop_span, format!("`{}` dropped here while still borrowed", name));
err.span_label(
drop_span,
format!("`{}` dropped here while still borrowed", name),
);
if let Some(end) = end_span {
err.span_label(end, "borrowed value needs to live until here");
}
Expand All @@ -472,11 +493,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
proper_span: Span,
end_span: Option<Span>,
) {
let mut err = self.tcx.path_does_not_live_long_enough(proper_span,
"borrowed value",
Origin::Mir);
let mut err =
self.tcx
.path_does_not_live_long_enough(proper_span, "borrowed value", Origin::Mir);
err.span_label(proper_span, "temporary value does not live long enough");
err.span_label(drop_span, "temporary value dropped here while still borrowed");
err.span_label(
drop_span,
"temporary value dropped here while still borrowed",
);
err.note("consider using a `let` binding to increase its lifetime");
if let Some(end) = end_span {
err.span_label(end, "temporary value needs to live until here");
Expand All @@ -496,14 +520,31 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
_proper_span: Span,
_end_span: Option<Span>,
) {
let mut err = self.tcx.path_does_not_live_long_enough(borrow_span,
&format!("`{}`", name),
Origin::Mir);
debug!(
"report_unscoped_local_value_does_not_live_long_enough(\
{:?}, {:?}, {:?}, {:?}, {:?}, {:?}\
)",
context, name, scope_tree, borrow, drop_span, borrow_span
);

let mut err = self.tcx.path_does_not_live_long_enough(
borrow_span,
&format!("`{}`", name),
Origin::Mir,
);
err.span_label(borrow_span, "borrowed value does not live long enough");
err.span_label(drop_span, "borrowed value only lives until here");
self.tcx.note_and_explain_region(scope_tree, &mut err,
"borrowed value must be valid for ",
borrow.region, "...");

if !self.tcx.sess.nll() {
self.tcx.note_and_explain_region(
scope_tree,
&mut err,
"borrowed value must be valid for ",
borrow.region,
"...",
);
}

self.explain_why_borrow_contains_point(context, borrow, &mut err);
err.emit();
}
Expand All @@ -516,16 +557,31 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
drop_span: Span,
_borrow_span: Span,
proper_span: Span,
_end_span: Option<Span>
_end_span: Option<Span>,
) {
let mut err = self.tcx.path_does_not_live_long_enough(proper_span,
"borrowed value",
Origin::Mir);
debug!(
"report_unscoped_temporary_value_does_not_live_long_enough(\
{:?}, {:?}, {:?}, {:?}, {:?}\
)",
context, scope_tree, borrow, drop_span, proper_span
);

let mut err =
self.tcx
.path_does_not_live_long_enough(proper_span, "borrowed value", Origin::Mir);
err.span_label(proper_span, "temporary value does not live long enough");
err.span_label(drop_span, "temporary value only lives until here");
self.tcx.note_and_explain_region(scope_tree, &mut err,
"borrowed value must be valid for ",
borrow.region, "...");

if !self.tcx.sess.nll() {
self.tcx.note_and_explain_region(
scope_tree,
&mut err,
"borrowed value must be valid for ",
borrow.region,
"...",
);
}

self.explain_why_borrow_contains_point(context, borrow, &mut err);
err.emit();
}
Expand All @@ -534,7 +590,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
&mut self,
context: Context,
(place, span): (&Place<'tcx>, Span),
loan: &BorrowData,
loan: &BorrowData<'tcx>,
) {
let mut err = self.tcx.cannot_assign_to_borrowed(
span,
Expand Down Expand Up @@ -706,9 +762,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
ProjectionElem::Field(_, field_type) => {
self.describe_field_from_ty(&field_type, field)
}
ProjectionElem::Index(..) |
ProjectionElem::ConstantIndex { .. } |
ProjectionElem::Subslice { .. } => {
ProjectionElem::Index(..)
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. } => {
format!("{}", self.describe_field(&proj.base, field))
}
},
Expand Down Expand Up @@ -765,13 +821,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Place::Local(local) => {
let local = &self.mir.local_decls[*local];
Some(local.ty)
},
}
Place::Static(ref st) => Some(st.ty),
Place::Projection(ref proj) => {
match proj.elem {
ProjectionElem::Field(_, ty) => Some(ty),
_ => None,
}
Place::Projection(ref proj) => match proj.elem {
ProjectionElem::Field(_, ty) => Some(ty),
_ => None,
},
}
}
Expand Down
15 changes: 12 additions & 3 deletions src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
pub(in borrow_check) fn explain_why_borrow_contains_point(
&self,
context: Context,
borrow: &BorrowData<'_>,
borrow: &BorrowData<'tcx>,
err: &mut DiagnosticBuilder<'_>,
) {
if let Some(regioncx) = &self.nonlexical_regioncx {
Expand Down Expand Up @@ -70,9 +70,18 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
}

_ => {
cause.label_diagnostic(mir, err);
Cause::UniversalRegion(region_vid) => {
if let Some(region) = regioncx.to_error_region(region_vid) {
self.tcx.note_and_explain_free_region(
err,
"borrowed value must be valid for ",
region,
"...",
);
}
}

_ => {}
}
}
}
Expand Down
Loading

0 comments on commit 259e4a6

Please sign in to comment.