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

Translation of lifetimes in Fn trait input parameters #106

Closed
alecmocatta opened this issue Jun 8, 2020 · 3 comments · Fixed by #107
Closed

Translation of lifetimes in Fn trait input parameters #106

alecmocatta opened this issue Jun 8, 2020 · 3 comments · Fixed by #107

Comments

@alecmocatta
Copy link

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
Copy link
Contributor

taiki-e commented Jun 8, 2020

EDIT: Sorry, this comment is incorrect. I was trying to convert an irrelevant lifetime to 'static.
EDIT2: See #106 (comment)

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
Copy link
Contributor

taiki-e commented Jun 8, 2020

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants