'missing lifetime specifier' regression in protobuf-1.0.1

[brson]

As reported by crater, Rust 1.3 nightlies appear to have introduced a regression in lifetime elision, probably related to default trait bounds.

cc @nikomatsakis @nrc @pnkfelix

Nominating in hopes a fix for this will appear before 1.3 beta.

[eefriedman]

Reduces to:

trait T:'static {}
fn main() {
    fn _message_down_cast(_m: &T) -> &i8 {
        panic!()
    }
}

[eefriedman]

RFC 141 says that "a lifetime position is anywhere you can write a lifetime in a type". According to that rule, fn(m: &(Sized+'static)) -> &i8 is ambiguous because there are two lifetime positions in the input, and this isn't a bug.

On the other hand, I'm pretty sure that isn't actually what we want, so we need a more precise rule which specifies exactly what lifetimes qualify as a "lifetime position".

Other random observations: nightly accepts the following, but stable doesn't:

fn main() {
    type T<'a> = &'a &'a i8;
    fn s(m: T) -> &i8 { m }
    static S: i8 = 1;
    static S2: &'static i8 = &S;
    s(&S2);
}

Neither nightly nor stable accept the following:

fn main() {
    type T<'a> = &'a &'static i8;
    fn s(m: T) -> &i8 { m }
    static S: i8 = 1;
    static S2: &'static i8 = &S;
    s(&S2);
}

[arielb1]

I think I accidentally introduced the regression as part of #26667. The previous version ignored object lifetime bounds - I should have caught this. I think the root cause here is that lifetime elision wasn't updated to respect object bounds.

[brson]

@nikomatsakis Per irlo this seems to affect some crates. Is there any way we can get this fixed this week in time for beta, or if not sometime in the next cycle for a backport?

[brson]

cc @rust-lang/compiler

[nikomatsakis]

triage: P-high

[nikomatsakis]

Sorry, I dropped the ball on compiler triage, but trying to get that wagon back on track now. Anyway, I'll look at this.

[nikomatsakis]

Ah, yes, I remember this issue now. I think this relates to the discussion about '_, and this comment of mine in particular. Basically I agree with @eefriedman that technically there is no bug here -- rather, a bug was fixed. However, the behavior doesn't seem right nonetheless, for the reasons I argued on the thread. It seems to me that trait object lifetimes, which follow distinct defaulting rules, should not affect elision in quite the way they currently do.

[nikomatsakis]

To be more specific, I think the rule we want is that object lifetimes only affect the elision rules if they are written explicitly, and not if defaults are used. I have to investigate how easy that will be to implement.

[nikomatsakis]

cc @rust-lang/lang -- see previous two comments

[nikomatsakis]

also cc @wycats -- see previous two comments

[nikomatsakis]

OK, as I thought, the elision determination is made based on the Ty<'tcx>, which has no information about whether the object lifetime was derived from a default or not. So this would be mildly tricky to implement. Basically, we would want to walk the AST instead of the type, I think. Of course we're already doing this walk when we convert, so it might be nice if we could gather the information that we need as we do it. I'll have to think about it. But it IS an interesting question as to precisely what rules we want here that we ought to settle first.

I think that approaching this basic on the position where the lifetime appears is probably best. However, one might also approach by making special treatment for 'static (after all, the object lifetime defaulting rules will either pick 'static or they will pick the lifetime of an enclosing & or some lifetime parameter placed on the trait, so if we behave differently around 'static that would also cover this case). For example, one might expect this to compile:

struct Foo<'s> { x: &'s i32 }
fn foo(x: &Foo<'static>) -> &Foo<'static> { x }
fn main() { }

But it does not today, since there are two lifetimes (in principle) one might choose in the return type: the anonymous one, or 'static. If we made the rules treat 'static as "non-conflicting" -- i.e., only pick 'static if there is nothing else to pick, it would address this case, but perhaps that is bit too creepy.

(Mostly just thinking through the possibilities here, not endorsing a plan of action.)

[nikomatsakis]

I'm going to re-nominate, but this time for T-lang, since I think we need a policy call.

[brson]

This is going into stable.

[aturon]

Note: we need to discuss the logistics that let this slip through and try to make sure it doesn't happen again.

[arielb1]

If we want to change this, it probably requires an RFC. The rules as they are today are somewhat confusing, which is somewhat inevitable with the invisible trait lifetime bounds.

The new rules means that code like

fn foo(t: Box<Trait>) -> &u8

will compile with the interpretation fn(Box<Trait+'static>) -> &'static u8

Because duplicate lifetimes are ignored, this code can only cause an issue if either the user explicitly specifies an object lifetime bound (e.g. fn foo<'a>(a: Box<Trait+'a>, b: &u8) -> &u8), which I feel OK with breaking, or an object lifetime gets forced to 'static.

My preferred solution is to prevent elision to 'static, probably after a warning cycle with soft elision. Functions like fn foo(x: &'static u8) -> &u8 (fine under old and current, will break) itself feel confusing.

[nikomatsakis]

We discussed in the language subteam meeting today and reached the conclusions that:

1. The current behavior is correct in that it follows the RFC.
2. Changes might be warranted but an RFC amendment is required, and further discussion would be helpful.

To that end, I'm going to close this issue and open (shortly) an RFC issue with a summary of the various cases that arise and the proposed solutions.