Feat.rewrite.lifetime

[aneksteind]

Rewrites structs to reflect hypothetical lifetimes accepted by polonius.

e.g.

pub struct Data<'d> {
    pub pi: *mut i32,
    pub pa: *mut A<'d>,
    pub a: A<'d>,
}
pub struct A<'a> {
    pub rd: &'a Data<'a>,
    pub pra: *mut &'a mut A<'a>,
}

struct VecTup<'a> {
    bar: *mut Vec<(VecTup<'a>, *mut A<'a>)>,
}

gets rewritten to

pub struct Data<'d,'h0,'h1,'h2> {
    pub pi: &'h0 mut (i32),
    pub pa: &'h1 mut (A<'d,'h0,'h1,'h2>),
    pub a: A<'d,'h0,'h1,'h2>,
}

pub struct A<'a,'h0,'h1,'h2> {
    pub rd: &'a (Data<'a,'h0,'h1,'h2>),
    pub pra: &'h2 core::cell::Cell<(&'a (A<'a,'h0,'h1,'h2>))>,
}

struct VecTup<'a,'h3,'h4,'h0,'h1,'h2> {
    bar: &'h3 (Vec<(VecTup<'a,'h3,'h4,'h0,'h1,'h2>, &'h4 (A<'a,'h0,'h1,'h2>))>),
}

[spernsteiner]

adt_metadata.lifetime_params tells you what lifetimes are declared on the type, not what lifetimes should be passed at this particular use of it. Due to the way we infer hypothetical lifetimes in ADTs, it happens to be the case that the names match up within struct fields (given struct Foo<'h0> { ... }, all uses will look like struct Bar<'h0, 'h1> { foo: &'h1 Foo<'h0> }, with the same name 'h0 in both places), but this isn't true in general.

struct Foo<'h0> { ... }
fn foo1<'a>(x: Foo<'a>, y: Foo<'a>) { ... }
fn foo2<'a, 'b>(x: Foo<'a>, y: Foo<'b>) { ... }

Given Foo, both the foo1 and foo2 signatures are valid, and they mean different things, so we need a way to express both of them.

RewriteLabel probably needs a field like &'tcx [OriginArg<'tcx>] representing the lifetime arguments of a particular Ty, with similar meaning to the labels of FieldMetadata's origin_args field. That will allow for distinctions like the above foo1/foo2 example.

For struct field types, the lifetime arguments can be set from the existing FieldMetadata::origin_args labels. For types in function signatures, we'll eventually need a similar setup to generate hypothetical parameters and so on, but for now that doesn't exist, so any region arguments should either be elided or left as 'static. Inside function bodies, the regions can always be elided.

[aneksteind]

Thanks for the catch, the tests were wrong and didn't complain about this. I've addressed it in 37e14bc and took out the test for the function lifetime rewriting for the time being; I'll create an issue for it

[aneksteind]

Issue created #921

[aneksteind]

@spernsteiner would you mind taking another look at this?

[spernsteiner]

This generally looks okay, but I have questions about a couple specific rewrite cases

[spernsteiner]

I was expecting this to generate a rewrite on every ADT with at least one lifetime, which would lead to unnecessary unfolding of type aliases. But I didn't see that effect when I ran it on this code:

struct Test<'a>(&'a u8);
type Alias<'a> = Test<'a>;
fn f<'a>(x: Alias<'a>) {}

I was expecting a rewrite from x: Alias<'a> to x: Test<'a>, but no such rewrite occurred. Under what conditions is label.lifetime non-empty? I was expecting it to be set to the original lifetime args ['a] in this case. Is it empty for structs that have no hypothetical lifetimes?

[aneksteind]

Alias doesn't get rewritten because it doesn't appear in the ADT metadata table, and that's because it's DefKind is a TyAlias and not one of Struct | Enum | Union

[aneksteind]

Is support for TyAlias for structs something that can be done in a follow-up PR?

[aneksteind]

Issue created in #926

[spernsteiner]

Will this swallow constraints, rewriting struct Foo<T: Clone>(...) into struct Foo<T>(...)?

[aneksteind]

Issue created in #927. Would you mind elaborating on that issue as to how fresh hypothetical lifetimes would be spliced in?

[spernsteiner]

I think ideally we would be able to either insert additional lifetime arguments before/after some existing argument (<T> -> <'h0, T> or <'a> -> <'a, 'h0>, with the rewrite being placed on a zero-width span <{}T> or <'a{}>) or generate a whole generics list from scratch if none is present (Foo -> Foo<'h0>, again attaching the rewrite to a zero-width span Foo{}). This would minimize disruption of the existing source code. But it might be better to just add a TODO or open an issue to avoid this complexity for now.