dtolnay / async-trait

Type erasure for async trait methods
Apache License 2.0
1.81k stars 84 forks source link

Translation of lifetimes in Fn trait input parameters #106

Closed alecmocatta closed 4 years ago

alecmocatta commented 4 years ago

I think this is a fairly niche and easy to work around case but it's a surprising footgun that might be worth a fix?

use async_trait::async_trait;
use core::future::Future;

#[async_trait]
pub trait ProcessPool: Send + Sync {
    type ThreadPool;

    async fn spawn<F, Fut, T>(&self, work: F) -> T
    where
        F: FnOnce(&Self::ThreadPool) -> Fut + Send,
        Fut: Future<Output = T> + 'static;
}

#[async_trait]
impl<P: ?Sized> ProcessPool for &P
where
    P: ProcessPool,
{
    type ThreadPool = P::ThreadPool;

    async fn spawn<F, Fut, T>(&self, work: F) -> T
    where
        F: FnOnce(&Self::ThreadPool) -> Fut + Send,
        Fut: Future<Output = T> + 'static,
    {
        (*self).spawn(work).await
    }
}

Playground

Self::ThreadPool on line 23 is translated to <&P as ProcessPool>::ThreadPool. I believe due to how unnamed lifetimes are treated in the call syntax of Fn traits, this is desugared to include a HRTB? This leads to the failure:

expected a `std::ops::FnOnce<(&<&P as ProcessPool>::ThreadPool,)>` closure, found `F`

This can be resolved by translating instead to <&'static P as ProcessPool>::ThreadPool. I don't know if this breaks other use cases however?

taiki-e commented 4 years ago

EDIT: Sorry, this comment is incorrect. I was trying to convert an irrelevant lifetime to 'static. EDIT2: See https://github.com/dtolnay/async-trait/issues/106#issuecomment-640927871

This can be resolved by translating instead to <&'static P as ProcessPool>::ThreadPool. I don't know if this breaks other use cases however?

IIUC this breaks use cases like FutureExt::inspect:

use async_trait::async_trait;
use std::future::Future;

#[async_trait]
pub trait FutureExt: Future + Send + Sync {
    async fn inspect<F>(self, f: F) -> Self::Output
    where
        F: FnOnce(&Self::Output) + Send,
        Self: Sized;
}

#[async_trait]
impl<Fut: ?Sized + Future + Send + Sync> FutureExt for Fut {
    async fn inspect<F>(self, f: F) -> Fut::Output
    where
        F: FnOnce(&Fut::Output) + Send,
        Self: Sized,
    {
        let output = self.await;
        f(&output);
        output
    }
}

playground

if using &'static:

-         F: FnOnce(&Fut::Output) + Send,
+         F: FnOnce(&'static Fut::Output) + Send,
error[E0310]: the associated type `<Fut as std::future::Future>::Output` may not live long enough
   --> tests/test.rs:924:12
    |
924 |         F: FnOnce(&'static Fut::Output) + Send,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |

playground

taiki-e commented 4 years ago

Actually, it seems that it can be fixed by using an explicit lifetime in type &P, not static. (my above comment is incorrect, sorry)

- impl<P: ?Sized> ProcessPool for &P
+ impl<'a, P: ?Sized> ProcessPool for &'a P
use async_trait::async_trait;
use std::future::Future;

#[async_trait]
pub trait ProcessPool: Send + Sync {
    type ThreadPool;

    async fn spawn<F, Fut, T>(&self, work: F) -> T
    where
        F: FnOnce(&Self::ThreadPool) -> Fut + Send,
        Fut: Future<Output = T>;
}

#[async_trait]
impl<'a, P: ?Sized> ProcessPool for &'a P
where
    P: ProcessPool,
{
    type ThreadPool = P::ThreadPool;

    async fn spawn<F, Fut, T>(&self, work: F) -> T
    where
        F: FnOnce(&Self::ThreadPool) -> Fut + Send,
        Fut: Future<Output = T>,
    {
        (*self).spawn(work).await
    }
}

playground

alecmocatta commented 4 years ago

Thanks for the quick response and fix @taiki-e!