NLL: bad error message when converting anonymous lifetime to 'static

[arielb1]

UPDATE: Mentoring instructions here.

---

Tested at 5165ee9 and current playpen

#![feature(nll)]

fn foo(x: &u32) -> &'static u32 {
    &*x
}

fn main() {}

Error message:

error: free region `` does not outlive free region `'static`
 --> src/main.rs:4:5
  |
4 |     &*x
  |     ^^^

error: aborting due to previous error

[nikomatsakis]

So, the NLL region checking errors currently uses the "nice region error" routines if it can:

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

Lines 990 to 996 in 27ede55

	if let (Some(f), Some(o)) = (fr_name, outlived_fr_name) {
	let tables = infcx.tcx.typeck_tables_of(mir_def_id);
	let nice = NiceRegionError::new(infcx.tcx, blame_span, o, f, Some(tables));
	if let Some(ErrorReported) = nice.try_report() {
	return;
	}
	}

If not, it fallback to this bad error message. This is the same basic strategy that the lexical region error reporting takes:

rust/src/librustc/infer/error_reporting/mod.rs

Lines 303 to 340 in 27ede55

	if !self.try_report_nice_region_error(&error) {
	match error.clone() {
	// These errors could indicate all manner of different
	// problems with many different solutions. Rather
	// than generate a "one size fits all" error, what we
	// attempt to do is go through a number of specific
	// scenarios and try to find the best way to present
	// the error. If all of these fails, we fall back to a rather
	// general bit of code that displays the error information
	RegionResolutionError::ConcreteFailure(origin, sub, sup) => {
	self.report_concrete_failure(region_scope_tree, origin, sub, sup).emit();
	}
	RegionResolutionError::GenericBoundFailure(origin, param_ty, sub) => {
	self.report_generic_bound_failure(
	region_scope_tree,
	origin.span(),
	Some(origin),
	param_ty,
	sub,
	);
	}
	RegionResolutionError::SubSupConflict(var_origin,
	sub_origin,
	sub_r,
	sup_origin,
	sup_r) => {
	self.report_sub_sup_conflict(region_scope_tree,
	var_origin,
	sub_origin,
	sub_r,
	sup_origin,
	sup_r);
	}
	}
	}
	}

You'll note that the error message you get without #![feature(nll)] is not particularly good (playground):

error[E0495]: cannot infer an appropriate lifetime for borrow expression due to conflicting requirements
 --> src/main.rs:2:5
  |
2 |     &*x
  |     ^^^
  |
note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the function body at 1:1...
 --> src/main.rs:1:1
  |
1 | / fn foo(x: &u32) -> &'static u32 {
2 | |     &*x
3 | | }
  | |_^
note: ...so that reference does not outlive borrowed content
 --> src/main.rs:2:5
  |
2 |     &*x
  |     ^^^
  = note: but, the lifetime must be valid for the static lifetime...
note: ...so that reference does not outlive borrowed content
 --> src/main.rs:2:5
  |
2 |     &*x
  |     ^^^

I think the ideal way to address this would be to make a "nice region error" that covers this case. Note that if we used a named lifetime like 'a, we get a better error. For example:

#![feature(nll)]

fn foo<'a>(x: &u32) -> &'a u32 {
    &*x
}

fn main() {}

gives:

error[E0621]: explicit lifetime required in the type of `x`
 --> src/main.rs:4:5
  |
3 | fn foo<'a>(x: &u32) -> &'a u32 {
  |            - consider changing the type of `x` to `&'a u32`
4 |     &*x
  |     ^^^ lifetime `'a` required

I believe this error is produced by the try_report_named_anon_conflict method:

rust/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs

Lines 20 to 120 in 27ede55

	pub(super) fn try_report_named_anon_conflict(&self) -> Option<ErrorReported> {
	let NiceRegionError { span, sub, sup, .. } = *self;
	debug!(
	"try_report_named_anon_conflict(sub={:?}, sup={:?})",
	sub,
	sup
	);
	// Determine whether the sub and sup consist of one named region ('a)
	// and one anonymous (elided) region. If so, find the parameter arg
	// where the anonymous region appears (there must always be one; we
	// only introduced anonymous regions in parameters) as well as a
	// version new_ty of its type where the anonymous region is replaced
	// with the named one.//scope_def_id
	let (named, anon, anon_arg_info, region_info) = if self.is_named_region(sub)
	&& self.is_suitable_region(sup).is_some()
	&& self.find_arg_with_region(sup, sub).is_some()
	{
	(
	sub,
	sup,
	self.find_arg_with_region(sup, sub).unwrap(),
	self.is_suitable_region(sup).unwrap(),
	)
	} else if self.is_named_region(sup) && self.is_suitable_region(sub).is_some()
	&& self.find_arg_with_region(sub, sup).is_some()
	{
	(
	sup,
	sub,
	self.find_arg_with_region(sub, sup).unwrap(),
	self.is_suitable_region(sub).unwrap(),
	)
	} else {
	return None; // inapplicable
	};
	debug!("try_report_named_anon_conflict: named = {:?}", named);
	debug!(
	"try_report_named_anon_conflict: anon_arg_info = {:?}",
	anon_arg_info
	);
	debug!(
	"try_report_named_anon_conflict: region_info = {:?}",
	region_info
	);
	let (arg, new_ty, br, is_first, scope_def_id, is_impl_item) = (
	anon_arg_info.arg,
	anon_arg_info.arg_ty,
	anon_arg_info.bound_region,
	anon_arg_info.is_first,
	region_info.def_id,
	region_info.is_impl_item,
	);
	match br {
	ty::BrAnon(_) => {}
	_ => {
	/* not an anonymous region */
	debug!("try_report_named_anon_conflict: not an anonymous region");
	return None;
	}
	}
	if is_impl_item {
	debug!("try_report_named_anon_conflict: impl item, bail out");
	return None;
	}
	if let Some((_, fndecl)) = self.find_anon_type(anon, &br) {
	if self.is_return_type_anon(scope_def_id, br, fndecl).is_some()
	|| self.is_self_anon(is_first, scope_def_id)
	{
	return None;
	}
	}
	let (error_var, span_label_var) = if let Some(simple_name) = arg.pat.simple_name() {
	(
	format!("the type of `{}`", simple_name),
	format!("the type of `{}`", simple_name),
	)
	} else {
	("parameter type".to_owned(), "type".to_owned())
	};
	struct_span_err!(
	self.tcx.sess,
	span,
	E0621,
	"explicit lifetime required in {}",
	error_var
	).span_label(
	arg.pat.span,
	format!("consider changing {} to `{}`", span_label_var, new_ty),
	)
	.span_label(span, format!("lifetime `{}` required", named))
	.emit();
	return Some(ErrorReported);
	}

The problem here is that the code is written to detect the case of one anonymous region and one region for which is_named_region is true. But is_named_region returns false for statics:

rust/src/librustc/infer/error_reporting/nice_region_error/util.rs

Lines 202 to 212 in 27ede55

	// This method returns whether the given Region is Named
	pub(super) fn is_named_region(&self, region: Region<'tcx>) -> bool {
	match *region {
	ty::ReFree(ref free_region) => match free_region.bound_region {
	ty::BrNamed(..) => true,
	_ => false,
	},
	ty::ReEarlyBound(_) => true,
	_ => false,
	}
	}

I think the simplest fix here would be to make that method return true for the ReStatic variant. Looking through the code, I don't see any reason that this would go wrong.

(Also, it appears that is_named_region is only used from named_anon_conflict; I think we should move the definition from util.rs to named_anon_conflict.rs.)

[davidtwco]

I'll tackle this one.

[pnkfelix]

I'm not certain that this was resolved completely. The unit test case that was filed with #47329 is using -Z verbose.

• I don't know what test I was looking at when I wrote the above, but it wasn't the test added with NLL: bad error message when converting anonymous lifetime to 'static #47329.
• The point below still stands in that there remain examples of anonymous lifetimes where we print things like "free region `` ..."

We need to check that we are producing usable output, especially when invoking the compiler without -Z flags.

[pnkfelix]

Note in particular that I don't expect this to be considered fixed until the output at ui/underscore-lifetime/dyn-trait-underscore.nll.stderr has been made legible. At the time of this writing that stderr file indicates that we emit:

error: free region `` does not outlive free region 'static

and messages that say "free region ``" are not considered legible to end users.

[davidtwco]

I'll look into this one (again).

[nikomatsakis]

@davidtwco and I had a discussion about how to fix this (and many other region errors besides) which is available here https://youtu.be/Vj0FuKTVP94.

[pnkfelix]

Okay we are declaring this now "fixed" because the output of dyn-trait-underscope.rs looks like this:

error: unsatisfied lifetime constraints
  --> $DIR/dyn-trait-underscore.rs:18:5
   |
LL | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |                - let's call the lifetime of this reference `'1`
LL |     //                      ^^^^^^^^^^^^^^^^^^^^^ bound *here* defaults to `'static`
LL |     Box::new(items.iter()) //~ ERROR cannot infer an appropriate lifetime
   |     ^^^^^^^^^^^^^^^^^^^^^^ cast requires that `'1` must outlive `'static`