Auto merge of #53175 - matthewjasper:more-return-stuff, r=nikomatsakis

[NLL] Returns are interesting for free regions Based on #53088 - creating now to get feedback. Closes #51175 * Make assigning to the return type interesting. * Use "returning this value" instead of "return" in error messages. * Prefer one of the explanations that we have a name for to a generic interesting cause in some cases. * Treat causes that involve the destination of a call like assignments.

Author: bors

src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs

@@ -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;
@@ -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 "),
  ConstraintCategory::Cast => write!(f, "cast "),
  ConstraintCategory::CallArgument => write!(f, "argument "),
  _ => write!(f, ""),
@@ -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) {
@@ -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={:#?}",
@@ -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 {
@@ -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);
@@ -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 {
@@ -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)) {
@@ -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
  ));
  },
  _ => {
@@ -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
  }
 }

src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs

@@ -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

src/librustc_mir/borrow_check/nll/region_infer/mod.rs

@@ -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).
@@ -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 --

src/librustc_mir/borrow_check/nll/type_check/mod.rs

@@ -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
  };
@@ -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,

src/test/ui/associated-types/cache/project-fn-ret-contravariant.krisskross.nll.stderr

@@ -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
@@ -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
 

src/test/ui/associated-types/cache/project-fn-ret-contravariant.transmute.nll.stderr

@@ -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
 

src/test/ui/associated-types/cache/project-fn-ret-invariant.krisskross.nll.stderr

@@ -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
@@ -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
 

src/test/ui/associated-types/cache/project-fn-ret-invariant.transmute.nll.stderr

@@ -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
 

src/test/ui/borrowck/borrowck-describe-lvalue.ast.nll.stderr

@@ -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
 

src/test/ui/borrowck/borrowck-describe-lvalue.mir.stderr

@@ -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