Skip to content

[NLL] Returns are interesting for free regions #53175

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.

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 7 commits into from
Aug 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc::hir::def_id::DefId;
use rustc::infer::error_reporting::nice_region_error::NiceRegionError;
use rustc::infer::InferCtxt;
use rustc::mir::{self, Location, Mir, Place, Rvalue, StatementKind, TerminatorKind};
use rustc::ty::RegionVid;
use rustc::ty::{TyCtxt, RegionVid};
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_errors::Diagnostic;
use std::collections::VecDeque;
Expand Down Expand Up @@ -42,7 +42,7 @@ impl fmt::Display for ConstraintCategory {
// Must end with a space. Allows for empty names to be provided.
match self {
ConstraintCategory::Assignment => write!(f, "assignment "),
ConstraintCategory::Return => write!(f, "return "),
ConstraintCategory::Return => write!(f, "returning this value "),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how this reads, though it's less "concise", which I think some people value. Still, seems ok to me.

ConstraintCategory::Cast => write!(f, "cast "),
ConstraintCategory::CallArgument => write!(f, "argument "),
_ => write!(f, ""),
Expand All @@ -67,6 +67,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
fn best_blame_constraint(
&self,
mir: &Mir<'tcx>,
tcx: TyCtxt<'_, '_, 'tcx>,
from_region: RegionVid,
target_test: impl Fn(RegionVid) -> bool,
) -> (ConstraintCategory, Span, RegionVid) {
Expand All @@ -92,7 +93,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// Classify each of the constraints along the path.
let mut categorized_path: Vec<(ConstraintCategory, Span)> = path
.iter()
.map(|&index| self.classify_constraint(index, mir))
.map(|&index| self.classify_constraint(index, mir, tcx))
.collect();
debug!(
"best_blame_constraint: categorized_path={:#?}",
Expand Down Expand Up @@ -123,13 +124,15 @@ impl<'tcx> RegionInferenceContext<'tcx> {
let constraint = &self.constraints[path[i]];

let constraint_sup_scc = self.constraint_sccs.scc(constraint.sup);
if constraint_sup_scc == target_scc {
return false;
}

match categorized_path[i].0 {
ConstraintCategory::Boring => false,
_ => true,
ConstraintCategory::Other => {
// other isn't interesting when the two lifetimes
// are unified.
constraint_sup_scc != self.constraint_sccs.scc(constraint.sub)
}
_ => constraint_sup_scc != target_scc,
}
});
if let Some(i) = best_choice {
Expand Down Expand Up @@ -231,6 +234,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
&self,
index: ConstraintIndex,
mir: &Mir<'tcx>,
tcx: TyCtxt<'_, '_, 'tcx>,
) -> (ConstraintCategory, Span) {
let constraint = self.constraints[index];
debug!("classify_constraint: constraint={:?}", constraint);
Expand All @@ -254,7 +258,34 @@ impl<'tcx> RegionInferenceContext<'tcx> {
debug!("classify_constraint: terminator.kind={:?}", terminator.kind);
match terminator.kind {
TerminatorKind::DropAndReplace { .. } => ConstraintCategory::Assignment,
TerminatorKind::Call { .. } => ConstraintCategory::CallArgument,
// Classify calls differently depending on whether or not
// the sub region appears in the destination type (so the
// sup region is in the return type). If the return type
// contains the sub-region, then this is either an
// assignment or a return, depending on whether we are
// writing to the RETURN_PLACE or not.
//
// The idea here is that the region is being propagated
// from an input into the output place, so it's a kind of
// assignment. Otherwise, if the sub-region only appears in
// the argument types, then use the CallArgument
// classification.
TerminatorKind::Call { destination: Some((ref place, _)), .. } => {
if tcx.any_free_region_meets(
&place.ty(mir, tcx).to_ty(tcx),
|region| self.to_region_vid(region) == constraint.sub,
) {
match place {
Place::Local(mir::RETURN_PLACE) => ConstraintCategory::Return,
_ => ConstraintCategory::Assignment,
}
} else {
ConstraintCategory::CallArgument
}
}
TerminatorKind::Call { destination: None, .. } => {
ConstraintCategory::CallArgument
}
_ => ConstraintCategory::Other,
}
} else {
Expand Down Expand Up @@ -304,7 +335,12 @@ impl<'tcx> RegionInferenceContext<'tcx> {
) {
debug!("report_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr);

let (category, span, _) = self.best_blame_constraint(mir, fr, |r| r == outlived_fr);
let (category, span, _) = self.best_blame_constraint(
mir,
infcx.tcx,
fr,
|r| r == outlived_fr
);

// Check if we can use one of the "nice region errors".
if let (Some(f), Some(o)) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) {
Expand Down Expand Up @@ -417,7 +453,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
diag.span_label(span, format!(
"{} was supposed to return data with lifetime `{}` but it is returning \
data with lifetime `{}`",
mir_def_name, fr_name, outlived_fr_name,
mir_def_name, outlived_fr_name, fr_name
));
},
_ => {
Expand Down Expand Up @@ -446,10 +482,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
crate fn find_outlives_blame_span(
&self,
mir: &Mir<'tcx>,
tcx: TyCtxt<'_, '_, 'tcx>,
fr1: RegionVid,
fr2: RegionVid,
) -> Span {
let (_, span, _) = self.best_blame_constraint(mir, fr1, |r| r == fr2);
let (_, span, _) = self.best_blame_constraint(mir, tcx, fr1, |r| r == fr2);
span
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
infcx.extract_type_name(&return_ty)
});

let mir_node_id = tcx.hir.as_local_node_id(mir_def_id).expect("non-local mir");
let mir_node_id = tcx.hir.as_local_node_id(mir_def_id).expect("non-local mir");

let (return_span, mir_description) = if let hir::ExprKind::Closure(_, _, _, span, gen_move)
= tcx.hir.expect_expr(mir_node_id).node
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/borrow_check/nll/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
longer_fr, shorter_fr,
);

let blame_span = self.find_outlives_blame_span(mir, longer_fr, shorter_fr);
let blame_span = self.find_outlives_blame_span(mir, infcx.tcx, longer_fr, shorter_fr);

if let Some(propagated_outlives_requirements) = propagated_outlives_requirements {
// Shrink `fr` until we find a non-local region (if we do).
Expand Down Expand Up @@ -1128,7 +1128,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
};

// Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
let span = self.find_outlives_blame_span(mir, longer_fr, error_region);
let span = self.find_outlives_blame_span(mir, infcx.tcx, longer_fr, error_region);

// Obviously, this error message is far from satisfactory.
// At present, though, it only appears in unit tests --
Expand Down
19 changes: 16 additions & 3 deletions src/librustc_mir/borrow_check/nll/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -877,8 +877,9 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
// they are not caused by the user, but rather artifacts
// of lowering. Assignments to other sorts of places *are* interesting
// though.
let is_temp = if let Place::Local(l) = place {
!mir.local_decls[*l].is_user_variable.is_some()
let is_temp = if let Place::Local(l) = *place {
l != RETURN_PLACE &&
!mir.local_decls[l].is_user_variable.is_some()
} else {
false
};
Expand Down Expand Up @@ -1119,7 +1120,19 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
match *destination {
Some((ref dest, _target_block)) => {
let dest_ty = dest.ty(mir, tcx).to_ty(tcx);
let locations = term_location.interesting();
let is_temp = if let Place::Local(l) = *dest {
l != RETURN_PLACE &&
!mir.local_decls[l].is_user_variable.is_some()
} else {
false
};

let locations = if is_temp {
term_location.boring()
} else {
term_location.interesting()
};

if let Err(terr) = self.sub_types(sig.output(), dest_ty, locations) {
span_mirbug!(
self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ LL | fn transmute<'a,'b>(x: &'a u32, y: &'b u32) -> (&'a u32, &'b u32) {
| |
| lifetime `'a` defined here
LL | let a = bar(foo, y);
| ^^^^^^^^^^^ argument requires that `'b` must outlive `'a`
| ^^^^^^^^^^^ assignment requires that `'b` must outlive `'a`

error: unsatisfied lifetime constraints
--> $DIR/project-fn-ret-contravariant.rs:54:12
Expand All @@ -29,7 +29,7 @@ LL | fn transmute<'a,'b>(x: &'a u32, y: &'b u32) -> (&'a u32, &'b u32) {
| lifetime `'a` defined here
LL | let a = bar(foo, y);
LL | let b = bar(foo, x);
| ^^^^^^^^^^^ argument requires that `'a` must outlive `'b`
| ^^^^^^^^^^^ assignment requires that `'a` must outlive `'b`

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ warning: not reporting region error due to nll
LL | bar(foo, x) //[transmute]~ ERROR E0495
| ^^^

error: borrowed data escapes outside of function
error: unsatisfied lifetime constraints
--> $DIR/project-fn-ret-contravariant.rs:48:4
|
LL | fn baz<'a,'b>(x: &'a u32) -> &'static u32 {
| - `x` is a reference that is only valid in the function body
| -- lifetime `'a` defined here
LL | bar(foo, x) //[transmute]~ ERROR E0495
| ^^^^^^^^^^^ `x` escapes the function body here
| ^^^^^^^^^^^ returning this value requires that `'a` must outlive `'static`

error: aborting due to previous error

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ LL | fn transmute<'a,'b>(x: Type<'a>, y: Type<'b>) -> (Type<'a>, Type<'b>) {
| |
| lifetime `'a` defined here
LL | let a = bar(foo, y); //[krisskross]~ ERROR E0623
| ^^^^^^^^^^^ argument requires that `'b` must outlive `'a`
| ^^^^^^^^^^^ assignment requires that `'b` must outlive `'a`

error: unsatisfied lifetime constraints
--> $DIR/project-fn-ret-invariant.rs:64:12
Expand All @@ -29,7 +29,7 @@ LL | fn transmute<'a,'b>(x: Type<'a>, y: Type<'b>) -> (Type<'a>, Type<'b>) {
| lifetime `'a` defined here
LL | let a = bar(foo, y); //[krisskross]~ ERROR E0623
LL | let b = bar(foo, x);
| ^^^^^^^^^^^ argument requires that `'a` must outlive `'b`
| ^^^^^^^^^^^ assignment requires that `'a` must outlive `'b`

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ warning: not reporting region error due to nll
LL | bar(foo, x) //[transmute]~ ERROR E0495
| ^^^

error: borrowed data escapes outside of function
error: unsatisfied lifetime constraints
--> $DIR/project-fn-ret-invariant.rs:58:4
|
LL | fn baz<'a,'b>(x: Type<'a>) -> Type<'static> {
| - `x` is a reference that is only valid in the function body
| -- lifetime `'a` defined here
...
LL | bar(foo, x) //[transmute]~ ERROR E0495
| ^^^^^^^^^^^ `x` escapes the function body here
| ^^^^^^^^^^^ returning this value requires that `'a` must outlive `'static`

error: aborting due to previous error

Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ LL | | //[mir]~^ ERROR cannot borrow `x` as mutable more than
LL | | *y = 1;
LL | | drop(y);
LL | | }
| |_________________^ requires that `'1` must outlive `'2`
| |_________________^ returning this value requires that `'1` must outlive `'2`
|
= note: closure implements `FnMut`, so references to captured variables can't escape the closure

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/borrowck/borrowck-describe-lvalue.mir.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ LL | | //[mir]~^ ERROR cannot borrow `x` as mutable more than
LL | | *y = 1;
LL | | drop(y);
LL | | }
| |_________________^ requires that `'1` must outlive `'2`
| |_________________^ returning this value requires that `'1` must outlive `'2`
|
= note: closure implements `FnMut`, so references to captured variables can't escape the closure

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ LL | S { pointer: &mut *p.pointer }
| ^^^^^^^^^^^^^^^

error: unsatisfied lifetime constraints
--> $DIR/borrowck-reborrow-from-shorter-lived-andmut.rs:19:18
--> $DIR/borrowck-reborrow-from-shorter-lived-andmut.rs:19:5
|
LL | fn copy_borrowed_ptr<'a,'b>(p: &'a mut S<'b>) -> S<'b> {
| -- -- lifetime `'b` defined here
| |
| lifetime `'a` defined here
LL | S { pointer: &mut *p.pointer }
| ^^^^^^^^^^^^^^^ requires that `'a` must outlive `'b`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a`

error: aborting due to previous error

Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ LL | invoke(&x, |a, b| if a > b { a } else { b }); //~ ERROR E0495
| ^^^^^^

error: unsatisfied lifetime constraints
--> $DIR/E0621-does-not-trigger-for-closures.rs:25:26
--> $DIR/E0621-does-not-trigger-for-closures.rs:25:45
|
LL | invoke(&x, |a, b| if a > b { a } else { b }); //~ ERROR E0495
| -- ^^^^^ requires that `'1` must outlive `'2`
| -- ^ returning this value requires that `'1` must outlive `'2`
| ||
| |return type of closure is &'2 i32
| has type `&'1 i32`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ error: unsatisfied lifetime constraints
--> $DIR/must_outlive_least_region_or_bound.rs:16:44
|
LL | fn explicit<'a>(x: &'a i32) -> impl Copy { x }
| -- lifetime `'a` defined here ^ return requires that `'a` must outlive `'static`
| -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static`

error: unsatisfied lifetime constraints
--> $DIR/must_outlive_least_region_or_bound.rs:22:69
|
LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x }
| -- lifetime `'a` defined here ^ return requires that `'a` must outlive `'static`
| -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static`

error: unsatisfied lifetime constraints
--> $DIR/must_outlive_least_region_or_bound.rs:29:5
Expand All @@ -57,7 +57,7 @@ LL | fn move_lifetime_into_fn<'a, 'b>(x: &'a u32, y: &'b u32) -> impl Fn(&'a u32
| lifetime `'a` defined here
LL | //~^ ERROR lifetime mismatch
LL | move |_| println!("{}", y)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b`

error[E0310]: the parameter type `T` may not live long enough
--> $DIR/must_outlive_least_region_or_bound.rs:34:5
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/in-band-lifetimes/mismatched.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ error: unsatisfied lifetime constraints
--> $DIR/mismatched.rs:16:46
|
LL | fn foo2(x: &'a u32, y: &'b u32) -> &'a u32 { y } //~ ERROR lifetime mismatch
| -- -- ^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a`
| -- -- ^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b`
| | |
| | lifetime `'b` defined here
| lifetime `'a` defined here
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-14285.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ LL | B(a) //~ ERROR 22:5: 22:9: explicit lifetime required in the type of
| ^

error[E0621]: explicit lifetime required in the type of `a`
--> $DIR/issue-14285.rs:22:7
--> $DIR/issue-14285.rs:22:5
|
LL | fn foo<'a>(a: &Foo) -> B<'a> {
| ---- help: add explicit lifetime `'a` to the type of `a`: `&'a (dyn Foo + 'a)`
LL | B(a) //~ ERROR 22:5: 22:9: explicit lifetime required in the type of `a` [E0621]
| ^ lifetime `'a` required
| ^^^^ lifetime `'a` required

error: aborting due to previous error

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-15034.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ LL | Parser { lexer: lexer }
| ^^^^^^

error[E0621]: explicit lifetime required in the type of `lexer`
--> $DIR/issue-15034.rs:27:25
--> $DIR/issue-15034.rs:27:9
|
LL | pub fn new(lexer: &'a mut Lexer) -> Parser<'a> {
| ------------- help: add explicit lifetime `'a` to the type of `lexer`: `&'a mut Lexer<'a>`
LL | Parser { lexer: lexer }
| ^^^^^ lifetime `'a` required
| ^^^^^^^^^^^^^^^^^^^^^^^ lifetime `'a` required

error: aborting due to previous error

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-3154.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ LL | thing{ x: x } //~ ERROR 16:5: 16:18: explicit lifetime required in the
| ^^^^^

error[E0621]: explicit lifetime required in the type of `x`
--> $DIR/issue-3154.rs:16:15
--> $DIR/issue-3154.rs:16:5
|
LL | fn thing<'a,Q>(x: &Q) -> thing<'a,Q> {
| -- help: add explicit lifetime `'a` to the type of `x`: `&'a Q`
LL | thing{ x: x } //~ ERROR 16:5: 16:18: explicit lifetime required in the type of `x` [E0621]
| ^ lifetime `'a` required
| ^^^^^^^^^^^^^ lifetime `'a` required

error: aborting due to previous error

Expand Down
Loading