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

0.3: How can I return a Future from a trait function? #1058

Closed
ngg opened this issue Jun 30, 2018 · 8 comments
Closed

0.3: How can I return a Future from a trait function? #1058

ngg opened this issue Jun 30, 2018 · 8 comments

Comments

@ngg
Copy link
Contributor

ngg commented Jun 30, 2018

With 0.1.x and 0.2.x it was possible to return Box<Future<Item = ..., Error = ...>>, but with the 0.3 branch it's no longer possible.

trait T {
    fn f() -> Box<Future<Output = ()>>;
}

This code gives the following compile error:

error[E0038]: the trait `std::future::Future` cannot be made into an object
 --> src/main.rs:4:5
  |
4 |     fn f() -> Box<Future<Output = ()>>;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::future::Future` cannot be made into an object
  |
  = note: method `poll` has a non-standard `self` type

Returning with impl Trait is not allowed in trait functions, so that method does not work on any branch.

@MajorBreakfast
Copy link
Contributor

MajorBreakfast commented Jun 30, 2018

The plan is that this limitation will one day be lifted, also we one day want to avoid boxing entirely and use dyn by value. In the meantime we have TaskObj and LocalTaskObj to store a Future<Output = ()> + Send + 'static and Future<Output = ()> + 'static respectively.

See https://github.com/rust-lang-nursery/futures-rs/blob/0.3/futures-core/src/task/mod.rs#L42

let task_obj = TaskObj::new(PinBox::new(future)); // task_obj implements Future

@ngg
Copy link
Contributor Author

ngg commented Jun 30, 2018

I think the impl Trait limitation will not be lifted too soon. (probably not in the Rust 2018 release)

I would need something like this but with a generic output type to be able to migrate our current project that now uses the 0.2 series, but this is a blocking issue for that.

What do you think about converting TaskObj and LocalTaskObj to have generic outputs? So TaskObj<T> would store Future<Output = T> + Send + 'static and LocalTaskObj<T> would store Future<Output = T> + 'static? It's not too late to change this in core.

@MajorBreakfast
Copy link
Contributor

Yes, I agree. Since you opened that issue I've been working on exactly that. ^^' I think it is useful.

I've FutureObj<T> and LocalFutureObj<T> already working and integrated into the LocalPool executor and all seems well. I'll provide a PR to libcore very soon.

It could even be possible to make the lifetime generic as well. Although, I'm not sure how yet.

@MajorBreakfast
Copy link
Contributor

dyn Trait (trait object), not impl Trait BTW. And I also think that it won't be lifted for Rust 2018. However, that's not a big deal and certainly no roadblock for stabilization. Executor::spawn_obj has specifically a name that ends with _obj, so that we can use the nice name at some point in the future.

@ngg
Copy link
Contributor Author

ngg commented Jun 30, 2018

Yeah I don't see a problem with a generic T either, but the generic lifetime seems much trickier (because of the *mut () magic in UnsafeTask. If *mut () is replaced with something that captures the lifetime as well (ex. a struct with the pointer and a PhantomData<&'a ()>) then it might work.

It can obviously be replicated into futures-util with a generic output if we don't want to change core. (It might even be simplified if it would support only PinBox and not a generic UnsafeTask)

@MajorBreakfast
Copy link
Contributor

MajorBreakfast commented Jun 30, 2018

I tabled the lifetime thing for now. We should look into it however at some point. Edit: I'll do some experiments later

Here's my PR rust-lang/rust#51944

@ngg
Copy link
Contributor Author

ngg commented Jun 30, 2018

I checked our project and currently we do use custom lifetimes.

A simple use case that's currently working for us with the 0.2.x branch:

trait T {
    fn f<'a>(&'a self) -> PinBox<Future<Item = u32, Error = MyError> + 'a>;
}
struct S;
impl T for S {
    #[async(boxed)]
    fn f(&self) -> MyResult<u32> {
        await!(/*...*/);
        Ok(5)
    }
}

So it would be great to be able to write this with 0.3:

trait T {
    fn f<'a>(&'a self) -> LocalFutureObj<'a, MyResult<u32>>;
}
struct S;
impl T for S {
    fn f<'a>(&'a self) -> LocalFutureObj<'a, MyResult<u32>> {
        LocalFutureObj::new(PinBox::new(async {
            await!(/*...*/);
            Ok(5)
        }))
    }
}

bors added a commit to rust-lang/rust that referenced this issue Jul 2, 2018
Make custom trait object for `Future` generic

- `TaskObj` -> `FutureObj<'static, ()>`
- The `impl From<...> for FutureObj<'a, T>` impls are impossible because of the type parameter `T`. The impl has to live in libstd, but `FutureObj<'a, T>` is from libcore. Therefore `Into<FutureObj<'a, T>>` was implemented instead. Edit: This didn‘t compile without warnings. I am now using non-generic Form impls.

See rust-lang/futures-rs#1058

r? @cramertj

Edit: Added lifetime
@MajorBreakfast
Copy link
Contributor

Now that this is merged we just need to wait for the next nightly. Currently there seem to be some troubles with the build infrastructure.

https://rust-lang-nursery.github.io/rust-toolstate/

In the meantime, a version of FutureObj and LocalFutureObj is already available under futures::future::FutureObj and futures_core::future::FutureObj. It's not a reexport from libcore yet, though. Executor::spawn_obj still expects the TaskObj. This will change as soon as we have a new nightly.

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

2 participants