Weird error with tokio::try_join! and mapped futures

Hello, I encountered a weird type error when attempting to use futures::future::FutureExt::map on futures that I later feed into tokio::try_join! (or futures::future::try_join).

I tried to create a small example which causes the compile-time error:

use async_trait::async_trait;
use futures::future::FutureExt;
use std::borrow::Cow;
use std::io;

struct S {}

#[async_trait]
trait Foo: Sync + Send {
    async fn foo(&self) -> io::Result<Cow<'_, ()>> {
        println!("Executed foo");
        Ok(Cow::Owned(()))
    }
    async fn bar<Other: Foo>(
        &self,
        other: &Other,
    ) -> io::Result<()> {
        // The trivial case:
        self.foo().await?;
        other.foo().await?;

        // Let's execute concurrently:
        tokio::try_join!(
            self.foo(),
            other.foo()
        )?;

        // Now we try to map the io::Result, which also works:
        self.foo().map(|x| x).await?;
        other.foo().map(|x| x).await?;

        // But this fails to compile:
        // tokio::try_join!(
        //     self.foo().map(|x| x),
        //     other.foo().map(|x| x)
        // )?;

        // It doesn't seem to be Tokio related, as this also fails:
        // futures::future::try_join(
        //     self.foo().map(|x| x),
        //     other.foo().map(|x| x)
        // ).await?;

        // But this works:
        futures::future::try_join(
            self.foo(),
            other.foo()
        ).await?;

        Ok(())
    }
}

#[async_trait]
impl Foo for S {}

#[tokio::main]
async fn main() -> io::Result<()> {
    let s1 = S {};
    let s2 = S {};
    s1.bar(&s2).await?;
    Ok(())
}

(Playground)

The error I get, when adding the commented out code, is:

error: implementation of `FnOnce` is not general enough
  --> src/main.rs:17:25
   |
17 |       ) -> io::Result<()> {
   |  _________________________^
18 | |         // The trivial case:
19 | |         self.foo().await?;
20 | |         other.foo().await?;
...  |
50 | |         Ok(())
51 | |     }
   | |_____^ implementation of `FnOnce` is not general enough
   |
   = note: closure with signature `fn(Result<Cow<'0, ()>, std::io::Error>) -> Result<Cow<'_, ()>, std::io::Error>` must implement `FnOnce<(Result<Cow<'1, ()>, std::io::Error>,)>`, for any two lifetimes `'0` and `'1`...
   = note: ...but it actually implements `FnOnce<(Result<Cow<'_, ()>, std::io::Error>,)>`

Apparently it has to do with the lifetime used for std::borrow::Cow in the above example. If I change the return type of foo to io::Result<()> and return Ok(()), then the problem doesn't occur.

I don't understand why .map(|x| x) makes a difference here (which also works fine when I don't try to join the futures). Am I doing something wrong, or is this an error of the compiler? I assume it's me doing something wrong, but I don't see what it is.

To explain why I need this: In my productive code, the .map isn't actually a no-op but will map error values (of concurrently executed methods) to different error values. In the Playground example above, I simplified the code to map x to x because it is sufficient to trigger/demonstrate the error.

I just found out that if I remove the trait (and thus remove async-trait also) and make the methods implemented directly for the type, the error will not occur either (Playground without traits).

So maybe this is an issue of the async-trait crate?


(Edit) For the expanded code of the original example, it's also possible to use "Tools" → "Expand macros" on the playground.

Sounds similar to Confusing: Implementation of `FnOnce` is not general enough · Issue #89976 · rust-lang/rust · GitHub

1 Like

From your bug report:

This fix is to change import to take an index instead of a reference in the last argument.

Does that mean that the compiler is right to report an error, but the error is just confusing?

Honestly, I don't know.

1 Like

I was able to come up with an example that shows the error without using #[async_trait]:

use futures::future::FutureExt;
use std::borrow::Cow;
use std::future::Future;
use std::pin::Pin;

struct S {}

impl S {
    fn foo<'a: 'b, 'b>(
        &'a self,
    ) -> Pin<
        Box<dyn 'b + Future<Output = Cow<'a, ()>> + Send>,
    > {
        Box::pin(async move {
            println!("Executed foo");
            Cow::Owned(())
        })
    }
    fn bar<'a, 'b, 'c>(
        &'a self,
        other: &'b S,
    ) -> Pin<Box<dyn 'c + Future<Output = ()> + Send>> // remove Send here!
    where
        'a: 'c,
        'b: 'c,
    {
        Box::pin(async move {
            futures::future::join(
                self.foo().map(|x| x),
                other.foo().map(|x| x),
            )
            .await;
        })
    }
}

#[tokio::main]
async fn main() {
    let s1 = S {};
    let s2 = S {};
    s1.bar(&s2).await;
}

(Playground)

Note that when I remove the Send bound on bar's result, the error will disappear.

(Note: I further simplified the example by editing this post. For previous versions, see history.)

I further modified the example to better show where the problem happens (and where modifications of the code would "fix" the compiler error):

use futures::future::FutureExt;
use std::borrow::Cow;
use std::future::Future;
use std::pin::Pin;

fn join<Fut1, Fut2>(future1: Fut1, future2: Fut2) -> impl Future<Output = ()>
where
    Fut1: Future,
    Fut2: Future,
{
    let future = futures::future::join(future1, future2);
    async move {
        future.await; // remove this line
    }
}

struct S {}

impl S {
    fn foo<'a: 'b, 'b>(
        &'a self,
    ) -> Pin<Box<dyn 'b + Future<Output = Cow<'a, ()>> + Send>>
    {
        Box::pin(async move {
            println!("Executed foo");
            Cow::Owned(())
        })
    }
    fn bar<'a, 'b, 'c>(
        &'a self,
        other: &'b S,
    ) -> Pin<Box<dyn 'c + Future<Output = ()> + Send>> // or remove Send here
    where
        'a: 'c,
        'b: 'c,
    {
        Box::pin(async move {
            join(
                self.foo().map(|x| x),  // or remove .map(…) here
                other.foo().map(|x| x), //    and here
            )
            .await;
        })
    }
}

#[tokio::main]
async fn main() {
    let s1 = S {};
    let s2 = S {};
    s1.bar(&s2).await;
}

(Playground)

Unfortunately, I don't understand why the error happens in the first place.

It seems that higher-ranked trait bounds might be necessary for this, but I would have little to no idea as to where (and how) to specify them properly. E0582 doesn't want go away no matter what.

1 Like

I think the error has to do with the closures not inferring that they should be using HRTBs. Unfortunately you can't really specify HRTBs on closures.

You have to force the inference, but it can be done.

fn higher_ranked_cow_bound<F>(f: F) -> F
where
    F: for<'a> FnOnce(Cow<'a, ()>) -> Cow<'a, ()>,
{
    f
}

// ...

        let id = higher_ranked_cow_bound(|x| x);
        Box::pin(async move {
            join(
                self.foo().map(id), 
                other.foo().map(id),
            )
            .await;
        })
2 Likes

So if I understand it right, the problem is that the compiler doesn't figure out that the two lifetimes must be equal, and I have to manually give a hint?

In my original example (at the top of this thread), I actually operate on an io::Result<Cow<'a, ()> instead of a Cow<'a, ()>. And what I want to achieve in the end isn't a .map(|x| x) but a .map(|x| map_error_type_of_result(x))

I tried to modify your higher_ranked_cow_bound method to fit my project, and I ended up with something like this:

fn id<F, T>(f: F) -> F
where
    F: for<'a> FnOnce(Result<Cow<'a, T>, io::Error>)
               -> Result<Cow<'a, T>, CustomError>,
    T: Clone,
{
    f
}
let (value1, value2) = try_join!(
    self
        .get_value()
        .map(id(|res| map_result_A(res))),
    other
        .get_value()
        .map(id(|res| map_result_B(res))),
)?;

That code above works in my project and will fix the compiler error! :+1:

However, in my real world code, the map_result_… functions (which are actually methods implemented on Result through an extension trait), operate on any Result<T, io::Error>, and not just on Cows. I thus tried to generalize the id function above:

fn id<F, T>(f: F) -> F
where
    F: for<'a> FnOnce(Result<T, io::Error>)
               -> Result<T, CustomError>,
{
    f
}

But then I get (as could have been expected) the same error: implementation of `FnOnce` is not general enough.

I also tried weird stuff like:

fn id<'t, F, T>(f: F) -> F
where
    F: for<'a> FnOnce(Result<T, io::Error>)
               -> Result<T, CustomError>,
    T: 't,
{
    f
}

or

fn id<'t, F, T1, T2>(f: F) -> F
where
    F: for<'a> FnOnce(Result<T1, io::Error>)
               -> Result<T2, CustomError>,
    T1: 't,
    T2: 't,
{
    f
}

Of course, the for<'a> gets useless here, so no wonder I also get the error: implementation of `FnOnce` is not general enough again :sob:.

I guess I would need something like the following, which isn't valid Rust syntax:

fn id<F, T1<'t1>, T2<'t2>>(f: F) -> F
where
    F: for<'a> FnOnce(Result<T1<'a>, io::Error>)
               -> Result<T2<'a>, CustomError>,
{
    f
}

Or, I would need the compiler to correctly infer the lifetimes by itself. :pleading_face:

I think my use case isn't so uncommon, as mapping an error (or Result) type of a future and joining two of such futures should be something that happens in other projects too (unless you never map errors and only pass them on).

Not quite! The problem is that SomeTrait<'a> and SomeTrait<'b> are two different traits. In our case, the closure is inferred to implement only SomeTrait<'a> for one specific lifetime 'a, and not implement the trait for any other lifetime. However, what we want is that it implements all the infinitely many traits SomeTrait<'a>, SomeTrait<'b>, ... SomeTrait<'static>, and the way this is written is as for<'a> SomeTrait<'a>, where the for<'a> means "for all lifetimes 'a".

In the above I use SomeTrait<'a> has a shorthand for FnOnce(Result<Cow<'a, T>, io::Error>) -> Result<Cow<'a, T>, CustomError>. Note that even when it implements only SomeTrait<'a> for one specific lifetime 'a, the two lifetimes in the FnOnce are still the same lifetime.

So how about:

fn id<F>(f: F) -> F
where
    F: for<T> FnOnce(Result<T, io::Error>)
              -> Result<T, CustomError>,
{
    f
}

Unfortunately, that's not valid Rust syntax either :frowning_face:

(My problem here is, that I have several occurrences where I want to map errors, and having to define an id function with yet another long type signature each time would really mess up all my code, as I do a lot of error mapping for a clean interface).

Shouldn't the compiler be able to figure out by itself that when I map the error, the lifetimes of the Ok values aren't touched at all? It looks like it's impossible to specify it in a general way (e.g. with for<T> …) and also impossible to infer it (at least with the current compiler).

I'm asking myself: What am I supposed to do? Not map errors at all? Not execute futures concurrently? Or fill my code with id functions having verbose type signatures (which each map a value to itself) just to prove to the compiler that everything is okay? :weary:

P.S.: An alternative would be to only use 'static types (as the error didn't happen in that case, see Playground with Cows removed), but that doesn't sound like a good alternative as well. (For example, I introduced Cows in my code to avoid copying values unnecessarily.)


Note that T in my examples (or Cow) isn't a trait, but a type (or rather a type constructor in the latter case?).

What I need (I think) is that I want the compiler to understand that it works on SomeType<'a>, SomeType<'b>, …, but also AnotherType<'a>, AnotherType<'b>, etc…, i.e. where a closure C fulfills the bound C: FnOnce(Result<WhicheverType<'a>, io::Error>) -> Result<WhicheverType<'a>, CustomError>.

But I'm not sure. I'm actually confused due to the complexity of this issue. :woozy_face: All I would like is mapping error types and executing futures concurrently. :sweat_smile:

I just found an old issue Higher-ranked types in trait bounds #1481 proposing something like for<T>. Maybe that is related to the problem?

I still feel like the compiler should be able to solve this automatically, but maybe I understand too little about the implications involved with that.

Note that my use-case isn't all theoretic, but something I'd expect to be needed in practice in many cases: mapping error types with closures + using non-'static types. (Not sure how the futures play into this problem.)

I finally found a solution for my particular problem, that I would like to present here. :smiley:

In fact, the type of the Ok value doesn't change at all (in my particular use case). So what I did was to create an extension trait for the Future trait:

use std::future::Future;
use futures::future::FutureExt as _;

pub trait FutureMapResErrExt<M, O>: Future + Sized {
    type Ok;
    type Err;
    type FutureMapResErr<E2>: Future<Output = Result<O, E2>>;
    fn map_res_err<E2>(self, mapper: M) -> Self::FutureMapResErr<E2>
    where
        M: FnOnce(Self::Err) -> E2;
}

impl<F, O, E, M> FutureMapResErrExt<M, O> for F
where
    F: Future<Output = Result<O, E>>,
{
    type Ok = O;
    type Err = E;
    type FutureMapResErr<E2> = impl Future<Output = Result<O, E2>>;
    fn map_res_err<E2>(self, mapper: M) -> Self::FutureMapResErr<E2>
    where
        M: FnOnce(Self::Err) -> E2,
    {
        self.map(|result| result.map_err(mapper))
    }
}

This extension trait extends all Future<Output = Result<O, E>> by a method map_res_err, which gets a closure as argument that maps the error inside the Result of the Output of the future from type E to some other error type E2.

The resulting future has type FutureMapResErr<E2> which is bound to be Future<Output = Result<O, E2>>, where O is the Ok type of the Result of the Output of the original future, i.e. the compiler knows that the Ok type (including any lifetimes) isn't modified by the mapping.

I can then use the new method as follows:

let (value1, value2) = tokio::try_join!(
    self.get_value(index).map_res_err(|x| mapperA(x)),
    other.get_value(index).map_res_err(|x| mapperB(x)),
)?;

I modified the original Playground to use the map_res_err method:

Modified Playground (uses nightly Rust).

I'd appreciate any feedback on my solution. Is it a reasonable fix/workaround for this problem? Have I implemented the extension trait (FutureMapResErrExt) correctly? And what do you think about whether the compiler should have figured this out by itself (and can someone explain why (not))?

This was a really tough problem for me. :crazy_face: Thanks for all the guidance, without your help I'd have been totally clueless! (And maybe there's still something wrong with my solution? Anyway, for now it seems to work in my code…)


Update:

Apparently, I can get rid of the two associated types Ok and Err:

use std::future::Future;
use futures::future::FutureExt as _;

pub trait FutureMapResErrExt<M, O, E>: Future + Sized {
    type FutureMapResErr<E2>: Future<Output = Result<O, E2>>;
    fn map_res_err<E2>(self, mapper: M) -> Self::FutureMapResErr<E2>
    where
        M: FnOnce(E) -> E2;
}

impl<F, O, E, M> FutureMapResErrExt<M, O, E> for F
where
    F: Future<Output = Result<O, E>>,
{
    type FutureMapResErr<E2> = impl Future<Output = Result<O, E2>>;
    fn map_res_err<E2>(self, mapper: M) -> Self::FutureMapResErr<E2>
    where
        M: FnOnce(E) -> E2,
    {
        self.map(|result| result.map_err(mapper))
    }
}

Or, alternatively, I can get rid of the type parameters O and E in the trait definition if I use associated types again (and then use Self::Ok instead of O and Self::Err instead of E). Not sure what's the better solution.

For some reason, it seems like I can't get rid of the parameter M to FutureMapResErrExt.

1 Like

Different closures (Ms) become part of your returned future in your implementation, so you can have different FutureMapResErr<E2>s for the same E2. This isn't allowed within a single implementation (like when M is a parameter on the fn), but it's okay across multiple implementations if M is the same within each single implementation (like when M is a parameter on the impl/trait).

Or when M is an associated type or maybe GAT... but that would limit you to one specific closure for each concrete future or such :sweat_smile:. (And it's probably not possible to end up with a defining case for TAIT via a call site?)

Not sure if I fully understand you, but I gave it another try to pass the type of M as associated type:

use std::future::Future;
use futures::future::FutureExt as _;

pub trait FutureMapResErrExt<O, E1, E2>: Future + Sized {
    type Map: FnOnce(E1) -> E2;
    type FutureMapResErr: Future<Output = Result<O, E2>>;
    fn map_res_err(self, mapper: Self::Map) -> Self::FutureMapResErr;
}

impl<F, O, E1, E2, M> FutureMapResErrExt<O, E1, E2> for F
where
    F: Future<Output = Result<O, E1>>,
    M: FnOnce(E1) -> E2,
{
    type Map = M;
    type FutureMapResErr = impl Future<Output = Result<O, E2>>;
    fn map_res_err(self, mapper: M) -> Self::FutureMapResErr {
        self.map(|result| result.map_err(mapper))
    }
}

That gives me:

error[E0207]: the type parameter `M` is not constrained by the impl trait, self type, or predicates
  --> src/future_ext.rs:10:20
   |
10 | impl<F, O, E1, E2, M> FutureMapResErrExt<O, E1, E2> for F
   |                    ^ unconstrained type parameter

As I can't omit M in the type parameter list of the trait, I decided to go for this syntax that seems shortest:

use std::future::Future;
use futures::future::FutureExt as _;

pub trait FutureMapResErrExt<O, E1, E2, M>: Future + Sized {
    type FutureMapResErr: Future<Output = Result<O, E2>>;
    fn map_res_err(self, mapper: M) -> Self::FutureMapResErr;
}

impl<F, O, E1, E2, M> FutureMapResErrExt<O, E1, E2, M> for F
where
    F: Future<Output = Result<O, E1>>,
    M: FnOnce(E1) -> E2,
{
    type FutureMapResErr = impl Future<Output = Result<O, E2>>;
    fn map_res_err(self, mapper: M) -> Self::FutureMapResErr {
        self.map(|result| result.map_err(mapper))
    }
}

That code works fine for me. Note that in the last example, I don't need GATs anymore (but still use TAITs).

I assume it's also possible to implement this without TAITs (type-alias-impl-trait), but it would make me have to define my own future type manually – and cause me some extra headache :sweat_smile:

Some questions remain, though. First of all, while this "solution" works for me, I don't think it is a generic solution for everyone. My solution is specific to mapping the error part of Result types. Any types other than Results aren't covered by my approach (and would require their own extension trait). It might thus be questionable to extend the futures crate with such an extension trait.

On the other hand, what I do doesn't seem so exotic:

Isn't that something other people might want to do as well? Anyone who tries this would likely run into the same problems as I did? Or was it something specific I did, which got me into this trouble, which could have been avoided?

I'm worried other people can run into this problem and will get:

error: implementation of `FnOnce` is not general enough

@alice came across this compiler error in another context where it was also highly confusing for the user:

I still don't understand the whole problem well enough to say whether

  • the compiler should simply give a better error message,
  • the type inference mechanism of the compiler should be improved to not throw this error in the first place (such that it's not required to create an extension trait like I proposed or use identity funcitons to force the inference like @quinedot demonstrated), and whether
  • the type inference mechanism could be improved at all (or if there are some type-theoretic limitations on this).

I would like to point out that my solution with impl<F, O, E1, E2, M> FutureMapResErrExt<O, E1, E2, M> for F is far from being simple to write down for programmers who might not expect to run into this sort of problem. It cost me several days to understand all this. I'm happy I spent the time on it, as I – once more – learned a lot more about Rust, but other people might be less happy about running into these problems.

I haven’t been following this thread so you may already know this or there may be other reasons to implement it, but your FutureMapResErrExt extension looks a lot like TryFutureExt::map_err from the futures crate at a quick glance

1 Like

I just figured out that if I hadn't used the futures::future::FutureExt::map method but did it manually with async move and match, the problem wouldn't have occurred, see Playground. So I was thinking "too functional" here. But other people might as well prefer the functional way of expressing this and thus run into the problem.

Oh, this actually solves my problem out of the box! Thanks a lot of that hint. There is no problem (for me) with using that. Here's a link to TryFutureExt::map_err. I might use that instead of my own extension trait.

So if I had either used a manual match or if I used the Result-specific extension trait, the problem wouldn't have occurred. Going the middle course (using map but not map_err) got me into trouble here. I still believe other people might run into that issue (perhaps in different contexts, other than involving Futures with Results).