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

Option<Future<T>> to Future<Option<T>> is too implicit #445

Closed
robey opened this issue Apr 11, 2017 · 4 comments
Closed

Option<Future<T>> to Future<Option<T>> is too implicit #445

robey opened this issue Apr 11, 2017 · 4 comments

Comments

@robey
Copy link

robey commented Apr 11, 2017

In option.rs, there's a short, clever implementation of Future<Option<T>> for Option<Future<T>>.

This is incredibly handy, but because it doesn't create a new type, the rust compiler has trouble figuring out when to use it. It looks like rustc will treat the option as a future if you use a function that exists on Future but not on Option... but since Future's API is crafted to match the shape of Option, most of the useful functions are on Option too, so rustc will try to use those.

For a small (contrived) example:

// -> Future<bool>
let f = Some(future::ok(23));  // Option<Future<usize>>
f.map(|n| n == Some(23))  // map is on Option, so Future == Some is invalid.

The obvious into_future converter doesn't work either, because rustc will use the Future facet to call that function, which returns self, which satisfies the contract because the original Option has a Future implementation... so it's a no-op.

The only way to trick the compiler into making a real future, is to call a special Future-only function that creates a new struct, like fuse:

// -> Future<bool>
let f = Some(future::ok(23));  // Option<Future<usize>>
f.fuse().map(|n| n == Some(23))  // works, relatively harmless since fuse doesn't do much

This shows the way out. Instead of implementing Future as a facet on Option, the futures library should implement IntoFuture for Option, allowing an explicit conversion that creates a one-field struct with the option inside it. (A struct with one by-value field should keep the overhead minimal or zero. It's the same content, but rustc will have a different concrete type for it.)

@alexcrichton
Copy link
Member

Ah yeah I believe the intention was not so much to enable calling methods on Option<Future<T>> but rather just having it satisfy various bounds.

If you'd like to use the methods I'd probably recommend UFCS (e.g. Future::map(f, |n| n == Some(32))) but we can perhaps consider removing this for 0.2

@robey
Copy link
Author

robey commented Apr 11, 2017

Yep... I filed the bug because the implicit actually prevents me from creating an explicit into_future myself. :) For now, I've worked around it by making one called to_future instead, but it would be nice to fix the root bug.

I can submit a PR here if yall are into it.

@alexcrichton
Copy link
Member

Oh nice! I wouldn't mind taking a look at a PR, sure

@alexcrichton
Copy link
Member

This landed in #731 for 0.2, so closing

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

No branches or pull requests

2 participants