Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Use new region infer errors for explaining borrows
This gives at least some explanation for why a borrow is expected to
last for a certain free region. Also:

* Reports E0373: "closure may outlive the current function" with NLL.
* Special cases the case of returning a reference to (or value
  referencing) a local variable or temporary (E0515).
* Special case assigning a reference to a local variable in a closure
  to a captured variable.
  • Loading branch information
matthewjasper committed Oct 21, 2018
1 parent 275432c commit 2a3969a
Show file tree
Hide file tree
Showing 131 changed files with 1,849 additions and 1,820 deletions.
298 changes: 283 additions & 15 deletions src/librustc_mir/borrow_check/error_reporting.rs

Large diffs are not rendered by default.

78 changes: 57 additions & 21 deletions src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
Expand Up @@ -10,27 +10,35 @@

use borrow_check::borrow_set::BorrowData;
use borrow_check::error_reporting::UseSpans;
use borrow_check::nll::region_infer::Cause;
use borrow_check::nll::ConstraintDescription;
use borrow_check::nll::region_infer::{Cause, RegionName};
use borrow_check::{Context, MirBorrowckCtxt, WriteKind};
use rustc::ty::{self, Region, TyCtxt};
use rustc::ty::{self, TyCtxt};
use rustc::mir::{
CastKind, FakeReadCause, Local, Location, Mir, Operand, Place, Projection, ProjectionElem,
Rvalue, Statement, StatementKind, TerminatorKind
CastKind, ConstraintCategory, FakeReadCause, Local, Location, Mir, Operand,
Place, Projection, ProjectionElem, Rvalue, Statement, StatementKind,
TerminatorKind
};
use rustc_errors::DiagnosticBuilder;
use syntax_pos::Span;

mod find_use;

pub(in borrow_check) enum BorrowExplanation<'tcx> {
pub(in borrow_check) enum BorrowExplanation {
UsedLater(LaterUseKind, Span),
UsedLaterInLoop(LaterUseKind, Span),
UsedLaterWhenDropped {
drop_loc: Location,
dropped_local: Local,
should_note_order: bool,
},
MustBeValidFor(Region<'tcx>),
MustBeValidFor {
category: ConstraintCategory,
from_closure: bool,
span: Span,
region_name: RegionName,
opt_place_desc: Option<String>,
},
Unexplained,
}

Expand All @@ -43,8 +51,8 @@ pub(in borrow_check) enum LaterUseKind {
Other,
}

impl<'tcx> BorrowExplanation<'tcx> {
pub(in borrow_check) fn add_explanation_to_diagnostic<'cx, 'gcx>(
impl BorrowExplanation {
pub(in borrow_check) fn add_explanation_to_diagnostic<'cx, 'gcx, 'tcx>(
&self,
tcx: TyCtxt<'cx, 'gcx, 'tcx>,
mir: &Mir<'tcx>,
Expand Down Expand Up @@ -142,15 +150,27 @@ impl<'tcx> BorrowExplanation<'tcx> {
}
}
}
}

BorrowExplanation::MustBeValidFor(region) => {
tcx.note_and_explain_free_region(
err,
&format!("{}{}", borrow_desc, "borrowed value must be valid for "),
region,
"...",
);
},
BorrowExplanation::MustBeValidFor {
category,
span,
ref region_name,
ref opt_place_desc,
from_closure: _,
} => {
region_name.highlight_region_name(err);

if let Some(desc) = opt_place_desc {
err.span_label(span, format!(
"{}requires that `{}` is borrowed for `{}`",
category.description(), desc, region_name,
));
} else {
err.span_label(span, format!(
"{}requires that {}borrow lasts for `{}`",
category.description(), borrow_desc, region_name,
));
};
},
_ => {},
}
Expand All @@ -176,7 +196,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
context: Context,
borrow: &BorrowData<'tcx>,
kind_place: Option<(WriteKind, &Place<'tcx>)>,
) -> BorrowExplanation<'tcx> {
) -> BorrowExplanation {
debug!(
"explain_why_borrow_contains_point(context={:?}, borrow={:?}, kind_place={:?})",
context, borrow, kind_place
Expand Down Expand Up @@ -241,11 +261,27 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
}

None => if let Some(region) = regioncx.to_error_region(region_sub) {
BorrowExplanation::MustBeValidFor(region)
None => if let Some(region) = regioncx.to_error_region_vid(borrow_region_vid) {
let (category, from_closure, span, region_name) = self
.nonlexical_regioncx
.free_region_constraint_info(
self.mir,
self.mir_def_id,
self.infcx,
borrow_region_vid,
region,
);
let opt_place_desc = self.describe_place(&borrow.borrowed_place);
BorrowExplanation::MustBeValidFor {
category,
from_closure,
span,
region_name,
opt_place_desc,
}
} else {
BorrowExplanation::Unexplained
},
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/librustc_mir/borrow_check/nll/mod.rs
Expand Up @@ -371,3 +371,7 @@ impl ToRegionVid for RegionVid {
self
}
}

crate trait ConstraintDescription {
fn description(&self) -> &'static str;
}
Expand Up @@ -8,9 +8,9 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use borrow_check::nll::ConstraintDescription;
use borrow_check::nll::constraints::{OutlivesConstraint};
use borrow_check::nll::region_infer::RegionInferenceContext;
use borrow_check::nll::region_infer::error_reporting::region_name::RegionNameSource;
use borrow_check::nll::type_check::Locations;
use borrow_check::nll::universal_regions::DefiningTy;
use util::borrowck_errors::{BorrowckErrors, Origin};
Expand All @@ -29,11 +29,7 @@ use syntax::errors::Applicability;
mod region_name;
mod var_name;

use self::region_name::RegionName;

trait ConstraintDescription {
fn description(&self) -> &'static str;
}
crate use self::region_name::{RegionName, RegionNameSource};

impl ConstraintDescription for ConstraintCategory {
fn description(&self) -> &'static str {
Expand Down Expand Up @@ -76,7 +72,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
mir: &Mir<'tcx>,
from_region: RegionVid,
target_test: impl Fn(RegionVid) -> bool,
) -> (ConstraintCategory, Span, RegionVid) {
) -> (ConstraintCategory, bool, Span) {
debug!("best_blame_constraint(from_region={:?})", from_region);

// Find all paths
Expand All @@ -96,13 +92,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
);

// Classify each of the constraints along the path.
let mut categorized_path: Vec<(ConstraintCategory, Span)> = path
let mut categorized_path: Vec<(ConstraintCategory, bool, Span)> = path
.iter()
.map(|constraint| {
if constraint.category == ConstraintCategory::ClosureBounds {
self.retrieve_closure_constraint_info(mir, &constraint)
} else {
(constraint.category, constraint.locations.span(mir))
(constraint.category, false, constraint.locations.span(mir))
}
})
.collect();
Expand Down Expand Up @@ -147,20 +143,17 @@ impl<'tcx> RegionInferenceContext<'tcx> {
}
});
if let Some(i) = best_choice {
let (category, span) = categorized_path[i];
return (category, span, target_region);
return categorized_path[i]
}

// If that search fails, that is.. unusual. Maybe everything
// is in the same SCC or something. In that case, find what
// appears to be the most interesting point to report to the
// user via an even more ad-hoc guess.
categorized_path.sort_by(|p0, p1| p0.0.cmp(&p1.0));
debug!("best_blame_constraint: sorted_path={:#?}", categorized_path);
debug!("`: sorted_path={:#?}", categorized_path);

let &(category, span) = categorized_path.first().unwrap();

(category, span, target_region)
*categorized_path.first().unwrap()
}

/// Walks the graph of constraints (where `'a: 'b` is considered
Expand Down Expand Up @@ -247,7 +240,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
) {
debug!("report_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr);

let (category, span, _) = self.best_blame_constraint(
let (category, _, span) = self.best_blame_constraint(
mir,
fr,
|r| r == outlived_fr
Expand Down Expand Up @@ -580,6 +573,24 @@ impl<'tcx> RegionInferenceContext<'tcx> {
}
}

crate fn free_region_constraint_info(
&self,
mir: &Mir<'tcx>,
mir_def_id: DefId,
infcx: &InferCtxt<'_, '_, 'tcx>,
borrow_region: RegionVid,
outlived_region: RegionVid,
) -> (ConstraintCategory, bool, Span, RegionName) {
let (category, from_closure, span) = self.best_blame_constraint(
mir,
borrow_region,
|r| r == outlived_region
);
let outlived_fr_name = self.give_region_a_name(
infcx, mir, mir_def_id, outlived_region, &mut 1);
(category, from_closure, span, outlived_fr_name)
}

// Finds some region R such that `fr1: R` and `R` is live at
// `elem`.
crate fn find_sub_region_live_at(&self, fr1: RegionVid, elem: Location) -> RegionVid {
Expand All @@ -598,24 +609,26 @@ impl<'tcx> RegionInferenceContext<'tcx> {
fr1: RegionVid,
fr2: RegionVid,
) -> (ConstraintCategory, Span) {
let (category, span, _) = self.best_blame_constraint(mir, fr1, |r| r == fr2);
let (category, _, span) = self.best_blame_constraint(mir, fr1, |r| r == fr2);
(category, span)
}

fn retrieve_closure_constraint_info(
&self,
mir: &Mir<'tcx>,
constraint: &OutlivesConstraint
) -> (ConstraintCategory, Span) {
) -> (ConstraintCategory, bool, Span) {
let loc = match constraint.locations {
Locations::All(span) => return (constraint.category, span),
Locations::All(span) => return (constraint.category, false, span),
Locations::Single(loc) => loc,
};

let opt_span_category = self
.closure_bounds_mapping[&loc]
.get(&(constraint.sup, constraint.sub));
*opt_span_category.unwrap_or(&(constraint.category, mir.source_info(loc).span))
opt_span_category
.map(|&(category, span)| (category, true, span))
.unwrap_or((constraint.category, false, mir.source_info(loc).span))
}

/// Returns `true` if a closure is inferred to be an `FnMut` closure.
Expand Down
11 changes: 9 additions & 2 deletions src/librustc_mir/borrow_check/nll/region_infer/mod.rs
Expand Up @@ -35,6 +35,7 @@ use std::rc::Rc;

mod dump_mir;
mod error_reporting;
crate use self::error_reporting::{RegionName, RegionNameSource};
mod graphviz;
pub mod values;
use self::values::{LivenessValues, RegionValueElements, RegionValues};
Expand Down Expand Up @@ -669,13 +670,19 @@ impl<'tcx> RegionInferenceContext<'tcx> {
/// to find a good name from that. Returns `None` if we can't find
/// one (e.g., this is just some random part of the CFG).
pub fn to_error_region(&self, r: RegionVid) -> Option<ty::Region<'tcx>> {
self.to_error_region_vid(r).and_then(|r| self.definitions[r].external_name)
}

/// Returns the [RegionVid] corresponding to the region returned by
/// `to_error_region`.
pub fn to_error_region_vid(&self, r: RegionVid) -> Option<RegionVid> {
if self.universal_regions.is_universal_region(r) {
return self.definitions[r].external_name;
Some(r)
} else {
let r_scc = self.constraint_sccs.scc(r);
let upper_bound = self.universal_upper_bound(r);
if self.scc_values.contains(r_scc, upper_bound) {
self.to_error_region(upper_bound)
self.to_error_region_vid(upper_bound)
} else {
None
}
Expand Down
40 changes: 40 additions & 0 deletions src/librustc_mir/diagnostics.rs
Expand Up @@ -2011,6 +2011,46 @@ match 5u32 {
```
"##,

E0515: r##"
Cannot return value that references local variable
Local variables, function parameters and temporaries are all dropped before the
end of the function body. So a reference to them cannot be returned.
```compile_fail,E0515
#![feature(nll)]
fn get_dangling_reference() -> &'static i32 {
let x = 0;
&x
}
```
```compile_fail,E0515
#![feature(nll)]
use std::slice::Iter;
fn get_dangling_iterator<'a>() -> Iter<'a, i32> {
let v = vec![1, 2, 3];
v.iter()
}
```
Consider returning an owned value instead:
```
use std::vec::IntoIter;
fn get_integer() -> i32 {
let x = 0;
x
}
fn get_owned_iterator() -> IntoIter<i32> {
let v = vec![1, 2, 3];
v.into_iter()
}
```
"##,

E0595: r##"
Closures cannot mutate immutable captured variables.
Expand Down
25 changes: 25 additions & 0 deletions src/librustc_mir/util/borrowck_errors.rs
Expand Up @@ -632,6 +632,31 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
self.cancel_if_wrong_origin(err, o)
}

fn cannot_return_reference_to_local(
self,
span: Span,
reference_desc: &str,
path_desc: &str,
o: Origin,
) -> DiagnosticBuilder<'cx> {
let mut err = struct_span_err!(
self,
span,
E0515,
"cannot return {REFERENCE} {LOCAL}{OGN}",
REFERENCE=reference_desc,
LOCAL=path_desc,
OGN = o
);

err.span_label(
span,
format!("returns a {} data owned by the current function", reference_desc),
);

self.cancel_if_wrong_origin(err, o)
}

fn lifetime_too_short_for_reborrow(
self,
span: Span,
Expand Down

0 comments on commit 2a3969a

Please sign in to comment.