Change `impl Future for Either` to `Output = Either`

[thomaseizinger]

Currently, the implementation of Future for Either requires that both Futures resolve to the same type:

https://github.com/rayon-rs/either/blob/af9f5fbd8fd6f626121971f810fc2dc7f8290b69/src/lib.rs#L1127-L1141

This limits, which kinds of Futures can be expressed using Either. It would be more useful to set the Output type to Either<A::Output, B::Output>. This is an API breaking change.

The original behaviour can be restored using the Either::into_inner function. In case both Futures, A and B have the same Output, the into_inner function can be used to "unwrap" the Either:

https://github.com/rayon-rs/either/blob/af9f5fbd8fd6f626121971f810fc2dc7f8290b69/src/lib.rs#L916-L930

[cuviper]

Is there a reason you're not using futures::future::Either for that?

[thomaseizinger]

Is there a reason you're not using futures::future::Either for that?

I guess we could yeah! I'll try and report back.

[thomaseizinger]

It doesn't work at the moment because project is not exposed publicly. However, I think from the previous responses, people are okay with adding it so I'll submit a PR now.

Edit: PR submitted here: https://github.com/rust-lang/futures-rs/pull/2691

[thomaseizinger]

Another thing I just discovered is that futures::Either has a - IMO - very weird implementation of Future:

impl<A, B> Future for Either<A, B>
where
    A: Future,
    B: Future<Output = A::Output>,
{
    type Output = A::Output;

    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
        match self.project() {
            Either::Left(x) => x.poll(cx),
            Either::Right(x) => x.poll(cx),
        }
    }
}

I would have expected that a Either<F1, F2> yields me another Either type but the implementation in futures forces both futures to resolve to the same type.

[thomaseizinger]

Would you be up for merging an implementation of the futures traits that doesn't try to be clever and simply sets all associated types to Either, i.e. Future::Output, Stream::Item, etc

[cuviper]

Ah, note that Future is a re-export from core, and #77 already implemented that with a unified Output. I'm not sure why I did that -- probably just because that's how we treat Iterator too.

[thomaseizinger]

Ah, note that Future is a re-export from core, and #77 already implemented that with a unified Output. I'm not sure why I did that -- probably just because that's how we treat Iterator too.

Dang so that would be a breaking change :(

I guess I'll have to stick with my custom Either type then.

[thomaseizinger]

FWIW, a not unified output would have been more powerful because you can always call into_inner if they end up being the same type.

Not sure if you ever plan on doing 2.0 but that could be a consideration :)

[cuviper]

FWIW, a not unified output would have been more powerful because you can always call into_inner if they end up being the same type.

It's not exactly equivalent -- they can be different Future types (so into_inner won't work) yet still have the same Future::Output, enabling the current Future for Either.

[thomaseizinger]

FWIW, a not unified output would have been more powerful because you can always call into_inner if they end up being the same type.

It's not exactly equivalent -- they can be different Future types (so into_inner won't work) yet still have the same Future::Output, enabling the current Future for Either.

I am not sure I follow? What I meant was:

impl<A, B> Future for Either<A, B> {
	type Output = Either<A::Output, B::Output>;

	// ...
}

If A and B happen to resolve to the same type (like u32) you end up with Either<u32, u32> at which point you can call .into_inner.

Am I missing something?

[cuviper]

Oh, calling into_inner on the output, after polling/await -- yes that would work.

[thomaseizinger]

Oh, calling into_inner on the output, after polling/await -- yes that would work.

Would you like me to re-word this issue for changing the Future implementation (in a potential 2.0) release?

[cuviper]

Sure, it wouldn't hurt to have a clean summary of proposed changes.

[thomaseizinger]

Sure, it wouldn't hurt to have a clean summary of proposed changes.

Done!