-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
Support trait with lifetime #30
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
src/expand.rs
Outdated
@@ -127,7 +147,7 @@ fn transform_sig( | |||
where_token: Default::default(), | |||
predicates: Punctuated::new(), | |||
}); | |||
for param in &sig.generics.params { | |||
for param in sig.generics.params.iter().chain(context.lifetimes()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This causes the impl to mismatch the trait when the impl generics contains more lifetimes.
#[async_trait]
trait Trait {
async fn f();
}
#[async_trait]
impl<'a> Trait for &'a () {
async fn f() {}
}
error[E0195]: lifetime parameters or bounds on method `f` do not match the trait declaration
--> tests/test.rs:315:5
|
312 | async fn f();
| - lifetimes in impl do not match this method in trait
...
315 | #[async_trait]
| ^^^^^^^^^^^^^^ lifetimes do not match method in trait
You will need to chain only those lifetimes that appear on the impl block's trait, i.e. impl Trait<'a> for Type<'b>
should chain 'a only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I'd like it if these lifetimes could be filtered down per-function.
#[async_trait]
trait Trait<'a, 'b> {
async fn f(_: &'a &'b ()); // chain 'a and 'b
async fn g(_: &'b ()); // chain 'b only
async fn h(); // do not chain
}
I have a feeling this will reduce the occurrence of lifetime headaches from this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 8ff8ec2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good.
Published in 0.1.12. |
Fixes #28