rustler icon indicating copy to clipboard operation
rustler copied to clipboard

Fix lifetime handling in rustler_codegen

Open evnu opened this issue 2 years ago • 2 comments

This PR builds on the previous work of @turion (https://github.com/rusterlium/rustler/pull/430), @SeokminHong (https://github.com/rusterlium/rustler/pull/430#issuecomment-1175816881) and @neosimsim (https://github.com/rusterlium/rustler/pull/471), so kudos to them, this PR only attempts to complete their prior work.

Handling lifetimes for encoders within rustler_codegen is obvious: we can directly use split_for_impl() from syn to split the generics in a way which allows us to use them for a generated impl. For decoders, handling lifetimes is more involved. First of all, we need to ensure that the special lifetime of the decoder (named '__rustler_decode_lifetime in the generated code) outlives the lifetimes of the type to which we want to decode. The reason for this is that references to parts of the term may not outlive the term itself. On the other hand, the lifetimes of the type need to be able to outlive the term's lifetime, as we return a new struct/enum from the decoder. To ensure this, we generate a where clause for a lifetime 'a like this:

// .. here could be other predicates
'a: '__rustler_decode_lifetime,
'__rustler_decode_lifetime: 'a

With that, the lifetimes 'a and '__rustler_decode_lifetime are practically the same.

Fix #428 Close #430 Close #471

evnu avatar Aug 03 '22 15:08 evnu

@turion @SeokminHong @neosimsim Maybe you can take a look into this PR? Note that I will not be very responsive the next two weeks.

evnu avatar Aug 03 '22 15:08 evnu

The generalized helper functions look so good! I think it would be better if there were some tests for them, like this:


pub mod lifetimes {
    use rustler::{Binary, NifMap, NifRecord, NifStruct, NifTuple};

    #[derive(NifMap)]
    pub struct GenericMap<'a, 'b> {
        pub i: Binary<'a>,
        pub j: Binary<'b>,
    }

    #[derive(NifStruct)]
    #[module = "GenericStruct"]
    pub struct GenericStruct<'a, 'b> {
        pub i: Binary<'a>,
        pub j: Binary<'b>,
    }

    #[derive(NifRecord)]
    #[tag = "generic_record"]
    pub struct GenericRecord<'a, 'b> {
        pub i: Binary<'a>,
        pub j: Binary<'b>,
    }

    #[derive(NifTuple)]
    pub struct GenericTuple<'a, 'b>(Binary<'a>, Binary<'b>);

    #[derive(NifMap)]
    pub struct WhereClause<'a, 'b>
    where
        'a: 'b,
    {
        pub i: Binary<'a>,
        pub j: Binary<'b>,
    }

    #[derive(NifMap)]
    pub struct LifetimeBounded<'a, 'b: 'a> {
        pub i: Binary<'a>,
        pub j: Binary<'b>,
    }
}

SeokminHong avatar Aug 07 '22 09:08 SeokminHong

@SeokminHong I added your tests as-is as a simple compile tests. Do you think that's ok for now? I think we might hit one-or-another issue some time down the road when actually using something with lifetimes, but more concrete examples will make fixing those issues easier I think.

evnu avatar Sep 01 '22 08:09 evnu

@SeokminHong I added your tests as-is as a simple compile tests. Do you think that's ok for now? I think we might hit one-or-another issue some time down the road when actually using something with lifetimes, but more concrete examples will make fixing those issues easier I think.

I think it's good. I believe it's a rare case to use lifetimes for serialized things.

SeokminHong avatar Sep 01 '22 09:09 SeokminHong