Skip to content

[NLL] Avoid borrowed value must be valid for lifetime '_#2r..." in errors #48592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 4, 2018
Merged

[NLL] Avoid borrowed value must be valid for lifetime '_#2r..." in errors #48592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a17a2e3
Do not report _#nr lifetimes names in errors
spastorino Feb 27, 2018
7580826
Avoid printing overwhelming info on -Znll-dump-cause
spastorino Feb 27, 2018
834e392
Refactor note_and_explain_region to call note_and_explain_free_region
spastorino Feb 28, 2018
ff7dca5
Make explain borrow work for Universal lifetimes
spastorino Mar 1, 2018
99c42dc
rustfmt over error_reporting, explain_borrow and region_infer mods
spastorino Mar 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
833 changes: 465 additions & 368 deletions src/librustc/infer/error_reporting/mod.rs
View file
Open in desktop

Large diffs are not rendered by default.

196 changes: 125 additions & 71 deletions src/librustc_mir/borrow_check/error_reporting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use rustc_data_structures::indexed_vec::Idx;

use std::rc::Rc;

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 @@ -97,7 +97,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
} else {
true
}
},
}
_ => true,
};

Expand All @@ -107,9 +107,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 @@ -155,7 +157,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 @@ -176,8 +179,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 @@ -203,8 +212,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 @@ -233,7 +242,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 @@ -256,8 +265,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 @@ -356,25 +365,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 @@ -386,9 +402,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 @@ -397,14 +413,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 @@ -415,12 +431,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 @@ -430,13 +446,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 @@ -451,11 +467,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 @@ -473,11 +494,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 @@ -497,14 +521,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 @@ -517,16 +558,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 @@ -535,7 +591,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 @@ -707,9 +763,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 @@ -766,13 +822,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
Open in desktop
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