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

Cannot ergonomically match between diverging futures #683

Closed
mqudsi opened this issue Dec 25, 2017 · 5 comments
Closed

Cannot ergonomically match between diverging futures #683

mqudsi opened this issue Dec 25, 2017 · 5 comments

Comments

@mqudsi
Copy link

mqudsi commented Dec 25, 2017

I'm trying to come up with a good issue that describes the various type incompatibility problems I've run into with futures-rs in the past. This is a very simple and contrived use case, but I think it demonstrates the basic problem.

I don't think there's an ergonomic way of handling a case where, depending on the result of one action the called futures undergo different transforms that arrive at the same result.

Here's the code (git repo here: https://git.neosmart.net/mqudsi/futuretest/src/branch/futures-rs-683)

#![feature(conservative_impl_trait)]

extern crate futures;
extern crate tokio_core;
use futures::future::{self, FutureResult};
use futures::future::*;
use tokio_core::reactor::Core;

#[derive(Debug)]
enum ErrorCode
{
    CanHandleWithAsync,
    CanHandleWithSync,
    CannotHandle
}

#[derive(Debug)]
enum ErrorCode2
{
}

fn main() {
    let mut core = Core::new().expect("Failed to initialize tokio_core reactor!");

    let f = async_entry_point()
        .or_else(move |err| {
            //the problem is that the three matches below resolve to different future types
            match err {
                ErrorCode::CanHandleWithAsync => async_error_handler()
                    .map_err(|_| ErrorCode::CannotHandle),
                ErrorCode::CanHandleWithSync => future::result(sync_error_handler()),
                ErrorCode::CannotHandle => future::err(ErrorCode::CannotHandle),
            }
        })
    ;

    core.run(f).unwrap();
}

fn async_entry_point() -> FutureResult<(), ErrorCode> {
    future::ok(())
}

fn async_error_handler() -> FutureResult<(), ErrorCode2> {
    future::ok(())
}

fn sync_error_handler() -> Result<(), ErrorCode> {
    Ok(())
}

which returns the following compiler error:

   Compiling futuretest v0.1.0 (file:///mnt/d/GIT/futuretest)
error[E0308]: match arms have incompatible types
  --> src/main.rs:28:13
   |
28 | /             match err {
29 | |                 ErrorCode::CanHandleWithAsync => async_error_handler()
30 | |                     .map_err(|_| ErrorCode::CannotHandle),
31 | |                 ErrorCode::CanHandleWithSync => future::result(sync_error_handler()),
32 | |                 ErrorCode::CannotHandle => future::err(ErrorCode::CannotHandle),
33 | |             }
   | |_____________^ expected struct `futures::MapErr`, found struct `futures::FutureResult`
   |
   = note: expected type `futures::MapErr<futures::FutureResult<_, ErrorCode2>, [closure@src/main.rs:30:30: 30:57]>`
              found type `futures::FutureResult<_, ErrorCode>`
note: match arm with an incompatible type
  --> src/main.rs:31:49
   |
31 |                 ErrorCode::CanHandleWithSync => future::result(sync_error_handler()),
   |                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

error: Could not compile `futuretest`.

To learn more, run the command again with --verbose.

Normally, my workaround would be to use future::result() to force parallel construction of the futures, but since in this case two of the error cases can be resolved immediately but the third requires the evaluation of another future, the end result is trying to compare a single-level future (FutureResult<_, ErrorCode>) with a multi-level future (MapErr<FutureResult<_, ErrorCode>>), which obviously causes the type error above.

In terms of solutions that would make this more ergonomic, what comes to my mind is if the return type of all the future transforms were unified as members of an enum wrapped in an opaque struct rather than unified via implementation of a shared trait (especially since impl trait is pretty useless as it currently stands). I'm sure there's a catch there and it's not as simple as I'm thinking it is, but there has to be a better way than this. In its current state, futures-rs becomes very hard to use when dealing with what is essentially a state machine (which is ironic since rust itself is particularly well suited for state machine composition with its powerful pattern matching) as the different responses each trigger the evaluation of differing futures.

@srijs
Copy link
Contributor

srijs commented Dec 25, 2017

Hey @mqudsi, did you know that you can use futures::future::Either to return two "different" futures from the same function?

In your case, this could look like:

use futures::future::Either;

// ...

            match err {
                ErrorCode::CanHandleWithAsync => Either::A(async_error_handler()
                    .map_err(|_| ErrorCode::CannotHandle)),
                ErrorCode::CanHandleWithSync => Either::B(future::result(sync_error_handler())),
                ErrorCode::CannotHandle => Either::B(future::err(ErrorCode::CannotHandle)),
            }

For more complex state-machine patterns, you can also check out the state_machine_future crate.

@Thomasdezeeuw
Copy link
Contributor

You can also use Box<Future>. The problem is that Rust needs a unique type. It can't return type a or b, but using a box virtual dispatch is used which allows you to return "something that implements trait Future". Also see the impl Trait rfc rust-lang/rfcs#105, this allows you to return impl Future, avoidding the allocation of using a Box.

@mqudsi
Copy link
Author

mqudsi commented Dec 26, 2017

@srijs I didn't. Thank you for that tidbit.

@Thomasdezeeuw impl Future is better than Box, but neither Box nor impl Future can be used as-is in this particular context.

@Thomasdezeeuw
Copy link
Contributor

@mqudsi Well if you implement std::error::Error for both errors you can return FutureResult<(), Box<std::error::Error>>, that should work.

But that does have a performance overhead, and like @srijs suggested an enum that has a variant for each possible error type does work better.

@alexcrichton
Copy link
Member

Thanks for the report! Unfortunately there's not really much we can do about this I think. This is sort of what happens I believe when you mix a statically compiled language with the generics-based implementation of futures.

That being said though this is something that async/await is directly targeted at solving. Conditional branches, multiple returns, etc, are far more ergonomic with async/await.

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

No branches or pull requests

4 participants