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

Elided or placeholdered generic lifetimes are not properly respected in trait implementation #64

Open
SergioBenitez opened this issue Jan 31, 2020 · 1 comment
Labels
help wanted Extra attention is needed

Comments

@SergioBenitez
Copy link
Contributor

SergioBenitez commented Jan 31, 2020

Consider the following, where the lifetime for & is elided in the trait implementation and a placeholder is used for the generic in the trait:

#[async_trait::async_trait]
trait Foo<'a>: Sized {
    async fn f(bar: &'a usize) -> Self;
}

#[async_trait::async_trait]
impl Foo<'_> for usize {
    async fn f(bar: &usize) -> Self {
        10
    }
}

This generates the following implementation:

impl Foo<'_> for usize {
    fn f<'y, 'life0>(bar: &'life0 usize) -> BoxFuture<'y, Self>
        where 'life0: 'y, Self: 'y
    {
        async fn __f(bar: &usize) -> usize { 10 }
        Box::pin(__f::<>(bar))
    }
}

This, of course, does not match the generated trait definition which has only a single lifetime generic in f. The "correct" implementation would look like:

impl<'f> Foo<'f> for usize {
    fn f<'y>(bar: &'f usize) -> BoxFuture<'y, Self>
        where 'f: 'y, Self: 'y
    {
        async fn __f(bar: &usize) -> usize { 10 }
        Box::pin(__f::<>(bar))
    }
}

This, of course, would require async-trait to know that the placeholdered generic lifetime in the trait corresponds to the elided lifetime in the implementation. In general, it's clear that async-trait can't do this as it doesn't have type information nor access to the AST of the definition.

I believe that async-trait should likely error when trait generics are placeholdered. At least then the user gets a nice error message as opposed to incorrect code being generated.

@steffahn
Copy link

This, of course, would require async-trait to know that the placeholdered generic lifetime in the trait corresponds to the elided lifetime in the implementation.

Note that rustc for ordinary traits does nothing like this either. Instead, the reason why the analogous situation compiles successfully for ordinary traits is merely because the following code compiles, too:

trait Foo<'a>: Sized {
    fn f(bar: &'a usize) -> Self;
}

impl<'a> Foo<'a> for usize {
    fn f<'b>(bar: &'b usize) -> Self {
        10
    }
}

i.e.

impl Foo<'_> for usize {
    fn f(bar: &usize) -> Self {
        10
    }
}

simply desugars to something like the impl in the first code block, not to an implementation where 'b and 'a are restricted to be the same; but the compiler doesn’t complain if a trait impl’s method is slightly more generic than necessary.


Of course, the suggested improvement of adding some errors still seems potentially reasonable. It should almost always be the case that a trait Foo<'a> actually uses the lifetime 'a in some method signature, so most trait implementations using '_ there would fail. On the other hand, '_ lifetimes in the Self type are probably quite common in working trait impls, so those should probably be exempt.

And of course the fact that the lifetime appearing nowhere, or only in non-async methods, makes for some use-cases that a new error could break means that adding an “error when trait generics are placeholdered” is likely too breaking a change for a semver-minor update. So unless a 0.2 version ever comes out, things should probably stay as-is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants