Failure to fulfill higher-kinded "outlives" predicate could have better error message

[apasel422]

Example:

fn foo<T>() where for<'a> T: 'a {}

fn bar<'b>() {
    foo::<&'b i32>();
}

fn main() {}

Error:

foo.rs:4:5: 4:19 error: the type `&'b i32` does not fulfill the required lifetime
foo.rs:4     foo::<&'b i32>();
             ^~~~~~~~~~~~~~
note: type must outlive the static lifetime
error: aborting due to previous error

It's not necessarily obvious why the type has to be 'static in this case.

[brson]

Message is the same today. Technically this is not hard to do. The hard part is just coming up wit hsomething better.

[eddyb]

I would say something along the lines of "'a could be 'static", pointing to the for<'a>.
for<'a> T: 'a means that for every lifetime 'a, T: 'a must hold, and that includes T: 'static here.
In some cases there are "well-formed" preconditions which can limit 'a to the shortest lifetime in a different type, and the hard part here might be handling those cases - though we can probably still print a lifetime.

@nikomatsakis or @arielb1 should be able to provide an example there (&'b T function argument?).

[tbg]

I'm taking a look at implementing @eddyb's suggestion. Curious about the other examples mentioned, though.

[tbg]

I've understood the code well enough to see the problem, but not enough to
propose a concrete solution. Could someone give me some pointers on
a reasonable path forward?

• When process_predicates sees the call site,
  it registers a RegionObligation for that callsite's region to outlive the
  static region ReStatic. This loses most of the information about the
  cause. I think the only trace is in the ObligationCauses
  ObligationCauseCode (a DefId which perhaps you could use later to get
  a hold of the higher-kinded predicate), which is not very immediate.
• when the region checker visits
  this obligation, it turns it into the requirement that the &i32 outlives
  (i.e. is) 'static. It turns the ObligationCause into a SubregionOrigin
  (via code_to_origin), and this only keeps the Span from the original
  cause (it seems impossible to emit an error message which is aware of the
  circumstances at this point).
• eventually, collect_errors
  checks whether our region is a super-region of ReStatic (via
  a RelateParamBound to which the matter has in the meantime resolved),
  which it isn't, and emits a RegionResolutionError to that effect.

What seems clear is that while terminating the higher-kinded predicate as
a ReStatic is convenient, it makes it impossible to give a better error
message when the callsite's lifetime isn't 'static later. OTOH, there are
probably good reasons for not leaking the higher type higher up (I read up on
how they are type checked, but I wouldn't say I understand too well in which
order things happen).

One could try to plumb more information up along with the introduced ReStatic
(i.e. adding to the ObligationCause information on what causes the ReStatic
bound, which would presumably have to make it into the SubregionOrigin as
well, and then be used for constructing the user-visible error. But that also
seems like a bunch of plumbing very specific to this very specific situation
(even trying to find documentation on HRTBs, I only found the source code and
a blurb in the nomicon) and more
of a hack than a fix.

[eddyb]

@tschottdorf You can add an extra field to fulfill::RegionObligation, so that the definition span of the 'a in T: 'a (no matter where the 'a comes from, as long as it's a parameter of some sort) is tracked.

Then, a SubregionOrigin variant (if we don't have one already, there might be another source of these, not just fulfill) specific to an outlives bound with a parameter that turned into a 'static bound.

cc @nikomatsakis

[tbg]

@eddyb I think I know how to go about the second part (making a special SubregionOrigin that holds enough info to emit a better error) but I'm less sure about the first part.

Let me see if I understand you correctly - you're proposing adding something to ObligationCause (not RegionObligation directly), right? That would then get passed down to code_to_origin and allow me to do the second part.

Whatever I pass down would have to hold the information that we synthesized a 'static bound, and also the Span of the higher-ranked bound (i.e. something representing some part of fn foo<T>() where for<'a> T: 'a {}).

So perhaps a new ObligationCauseCode:

pub enum ObligationCauseCode<'tcx> {
    ///  An obligation incurred from treating a higher-ranked trait situated at the specified Span.
    HigherRankedRegion(Span) // do we need to keep a nested ObligationCauseCode here?
}
// code_to_error can now check the ObligationCauseCode if it sees a `ReStatic` bound
// and emit a custom error with both the obligation's Span (`&'b i32`) as well as the HRTB (`for<'a> T: 'a {}`).

Assuming that that's reasonable (may well not be), one very practical question I have is how to get the Span from the predicate (or anything contained within) in register_region_obligation (or higher up?). I do have a DefId somewhere in the binder; is that how it's done? If so, appreciate a pointer on the magical incantation.

[eddyb]

There was a reason I said it like that, because you can change the callers of code_to_origin to avoid polutting ObligationCause, because that refers to the original cause (you might have to make SubregionOrigin recursive if it's not already).
You can get a Span from the DefId inside the Region, if late-bound and named (which is always true for this issue) - check tcx.hir methods for something that only works in the same crate (otherwise you'd need to add Lifetime definition spans to crate metadata and that's a bit trickier).

[tbg]

Ah, gotcha - thanks for the pointers, I'll put something together.

[tbg]

@eddyb I have something working, but it's rough around the edges: tbg#1
If you don't mind taking a look - put my questions/remarks in the diff.

(can also open a PR if you prefer that, but it seemed immature).

[eddyb]

@tschottdorf You have a couple binaries you should not commit (run ./x.py test src/tools/tidy).
But other than that, it looks good enough for a PR - I want others to look at it too!

[metajack]

@tschottdorf Are you still working on this?

[tbg]

No, knock yourself out! It wasn't worth doing by the time I stopped working on it, though that might've changed in the meantime.

[Ygg01]

Hi, is this issue still open. On Rust stable and nightly version 2015/2018 I get following message:

   Compiling playground v0.0.1 (/playground)
error[E0477]: the type `&'b i32` does not fulfill the required lifetime
 --> src/main.rs:4:5
  |
4 |     foo::<&'b i32>();
  |     ^^^^^^^^^^^^^^
  |
  = note: type must satisfy the static lifetime

Is this the error the OP was suggesting?

[Angr1st]

With the current nightly the message is now:

   Compiling playground v0.0.1 (/playground)
error: lifetime may not live long enough
 --> src/main.rs:4:5
  |
3 | fn bar<'b>() {
  |        -- lifetime `'b` defined here
4 |     foo::<&'b i32>();
  |     ^^^^^^^^^^^^^^ requires that `'b` must outlive `'static`

error: could not compile `playground` due to previous error

Is this good enough? I think this was changed through #95565. I tested it again after reading the blog post by Jack Huey.

[SamB]

I believe the idea is to indicate that the static bound originates from the for<'a>.

It looks like 1.78 does a bit better here:

error: lifetime may not live long enough
 --> src/main.rs:4:5
  |
3 | fn bar<'b>() {
  |        -- lifetime `'b` defined here
4 |     foo::<&'b i32>();
  |     ^^^^^^^^^^^^^^ requires that `'b` must outlive `'static`
  |
note: due to current limitations in the borrow checker, this implies a `'static` lifetime
 --> src/main.rs:1:30
  |
1 | fn foo<T>() where for<'a> T: 'a {}
  |                              ^^

Rust Playground

This at least points to the universally-quantified region name, if not the quantification itself.

I'm not sure what the bit about "current limitations" is for, though. Perhaps the only relevant limitation for this example is the inability to explain the implication?