Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding additional lifetime constraints to a trait impl #90

Closed
Nemo157 opened this issue Jan 30, 2017 · 7 comments
Closed

Adding additional lifetime constraints to a trait impl #90

Nemo157 opened this issue Jan 30, 2017 · 7 comments

Comments

@Nemo157
Copy link

Nemo157 commented Jan 30, 2017

I just tried implementing std::convert::From for a reference to a struct that has a lifetime parameter in a proc_macro_derive, it took a while, but I eventually came up with something that "worked"

let a = syn::LifetimeDef::new("'a");
let mut b = syn::LifetimeDef::new("'b");
b.bounds.push(a.lifetime.clone());
let mut generics = ast.generics.clone();
generics.lifetimes.push(a.clone());
generics.lifetimes.push(b.clone());
let (impl_generics, _, _) = generics.split_for_impl();
let (_, ty_generics, where_clause) = ast.generics.split_for_impl();
let a = a.lifetime;
let b = b.lifetime;
quote! {
    impl #impl_generics ::std::convert::From<&#a ::clap::ArgMatches<#b>> for #ident #ty_generics #where_clause {
        fn from(#matches: &#a ::clap::ArgMatches<#b>) -> Self {
            #body
        }
    }
}

I feel like there must be a better way to do this, maybe some kind of enhanced split_for_impl that supports injecting extra lifetimes. (I also just realised this would probably fail if the struct passed in had a 'a or 'b lifetime, I assume there's no hygiene in proc_macro_derives at all 😮)

@dtolnay
Copy link
Owner

dtolnay commented Jan 31, 2017

I would love to make this better. I agree with you that there must be a way.

Two things that might help in this case are a way to parse generics from a string:

let arg_matches_generics = parse_generics("<'_a, '_b: '_a>").unwrap();

and a way to combine two generics:

let generics = arg_matches_generics.merge(ast.generics);

But even with both of those, your conceptually simple code would still be tricky to write correctly so I think we can do even better. Let's both keep thinking.

@dtolnay
Copy link
Owner

dtolnay commented Jan 8, 2018

This is somewhat better in the upcoming 0.12 release.

let mut generics = ast.generics.clone();
generics.params.push(parse_quote!('_a));
generics.params.push(parse_quote!('_b: '_a));

@Arnavion
Copy link
Contributor

generics.params.push(parse_quote!('_a));

If generics.params already contains a syn::GenericParam::Type then pushing lifetimes at the end emits <T, 'a> and results in lifetime parameters must be declared prior to type parameters.

In 0.11 and the early days of 0.12, syn::Generics had two separate fields for lifetime parameters and type parameters, so one could push the new lifetime into the former and get correct output.

It could still work if Punctuated gave a way to insert(0) into its internal Vec so that one could prepend the lifetime, but it doesn't.

So it seems the easiest way is to create a new Punctuated with std::iter::once(parse_quote!('__a))).chain(generic.params).collect() ?

@mystor
Copy link
Collaborator

mystor commented Jan 11, 2018

I'd also be open to doing some error correction work in the ToTokens implementation here. We could print out the generics in 2 passes with the first printing out all lifetime parameters.

In general we try not to let well-formed syn syntax trees produce malformed rust syntax when calling ToTokens.

@Arnavion
Copy link
Contributor

(0.12.4 has Punctuated::insert, but also emits the generic parameters in the correct order so that the original suggestion works unchanged.)

@Soft
Copy link

Soft commented Mar 5, 2018

About the question of lifetime hygiene, is there a good way to guarantee that the added lifetimes do not conflict with existing ones? I guess I could just go over generics.params and try to come up with a new name for a lifetime but that approach seems rather messy.

@dtolnay
Copy link
Owner

dtolnay commented Dec 25, 2018

I am going to call this done as of Syn 0.15 and recommend the approach in #90 (comment). Lifetimes now get printed before type parameters even if you push them on the end of the parameter list (fixed in #321).

@dtolnay dtolnay closed this as completed Dec 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants