I think using self.as_mut().stream1().try_poll_next(cx)? here will avoid the problem you mentioned.

Wouldn't that cause the two halves of the zipper to become unaligned? That is,

A: Ok(1), Err(2), Ok(3)
B: Ok(4), Ok(5), Ok(6)

Would result in

Ok((1, 4)), Err(2), Ok((3,5))

Is that desired? (Perhaps this also suggests why this combinator hasn't been created before now).

Hmm, but that's the same behavior as Stream::zip of futures 0.1.

https://github.com/rust-lang-nursery/futures-rs/blob/5654784d5b677dadc4b86affc575d12aa737e4a7/src/stream/zip.rs#L37

the same behavior as Stream::zip of futures 0.1

It sure is:

use futures::{future, stream, Future, Stream}; // 0.1.26

fn main() {
    let a: Vec<Result<i32, i32>> = vec![Ok(1), Err(2), Ok(3)];
    let b: Vec<Result<i32, i32>> = vec![Ok(4), Ok(5), Ok(6)];

    stream::iter_result(a)
        .zip(stream::iter_result(b))
        .then(|x| future::ok::<_, ()>(eprintln!("{:?}", x)))
        .for_each(|_| Ok(()))
        .wait();
}

(Playground)

Output:

Ok((1, 4))
Err(2)
Ok((3, 5))

But I think the question remains: is this the expected and desired behavior?

is this the expected and desired behavior?

If we are open to changing behaviors, the most comprehensive solution is to have

enum ZipError<A, B> {
    A(A),
    B(B),
    Both(A, B)
}

Hmm... I have no clear opinion on which is better, for now.

FWIW, as another thing we may need to consider, the behavior of current this PR's TryZip can be written using Zip, but the behavior of 0.1's Zip cannot be written using either 0.3's Zip or current this PR's TryZip.

cc @cramertj @Nemo157 Do you have any thoughts?

I would've personally expected that try_zip would stop returning at all after the first error, but I don't have a strong opinion here.

Bump-- @shepmaster what are your thoughts here?

Sadly, I don't even remember what my original use case was for this, and I don't recall what my thoughts were. 😊

I've posted the open questions back on the issue. The fact that there aren't people clamoring for this feature is perhaps a sign that it's not useful, yet.

Okay! Let me know if you're interested in revisiting at some point.

This should technically be

impl<St1: TryStream, St2: TryStream> Unpin for TryZip<St1, St2>
where
    Fuse<IntoStream<St1>>: Unpin,
    Fuse<IntoStream<St2>>: Unpin,
{}

In case either Fuse or IntoStream don’t have the equivalent Unpin implementations.

Makes sense, but it's unfortunate that the internal implementation details need to leak out like that.

Should the impl Unpin for Zip be updated to use Fuse as well? Is this a broader issue?

it's unfortunate that the internal implementation details need to leak out like that.

Yeah, that was my feeling as well as I wrote the comment. I rationalized it as being the same as the auto-trait's structural rules, if TryZip were to rely on the implicit Unpin you would still get error messages saying "Fuse does not implement Unpin".

Is this a broader issue?

Probably, I'll try to remember to open a ticket about it. (This is one of the reasons I prefer pin-project for projection, it can derive the Unpin implementation for you so that you don't have to keep it consistent with the current field definitions).

I was just looking into this a little more as I encountered the situation in another library where the Unpin definition depends on a crate internal type. It looks like rustdoc has special handling for the automatic implementations that looks through the field types and shows the bounds that result from them instead of revealing the internal types, it would be nice if there were some way to request that to happen for manual implementations as well.

For example

struct Foo<T>(T);

pub struct Bar<T>(Foo<T>);

unsafe impl<T: Send + Iterator> Send for Foo<T> {}

shows in the docs

impl<T> Send for Bar<T>
where
    T: Iterator + Send,