Adds Stream::cmp

[assemblaj]

Very similar to partial_cmp, primary difference is that cmp is used for the comparison instead of partial_cmp and as such the code was changed to accommodate that (Item's implement Ord instead of PartialOrd, Ordering is returned instead of Option), etc

[yoshuawuyts]

cc/ @montekki can I ask you to review this? 😊

[montekki]

@assemblaj @yoshuawuyts generally LGTM, but it would be super great to explore in the future some single generic version of these comparison combinators and implement them all with it. Does the regular std do something of this kind? Probably we need to take a look.

[assemblaj]

@montekki For the most part std just uses duplicate code for all of the comparison functions, I think. But those implementations are more simple in nature.
https://doc.rust-lang.org/1.3.0/src/core/iter.rs.html#3229-3243
https://github.com/rust-lang/rust/blob/master/src/libcore/iter/traits/iterator.rs
Do you think it would be feasible to use (or build) something like Zip that could just iterate both streams at once, and then compare the values?

[montekki]

it looks like partial_cmp is implemented with partial_cmp_by https://github.com/rust-lang/rust/blob/master/src/libcore/iter/traits/iterator.rs#L2630, this also goes for cmp which is implemented with cmp_by. It looks like there is no way to make a silver bullet generic comparator because of different order operations and types, but at least for cases like cmp and cmp_by code can probably be re-used.

[assemblaj]

partial_cmp_by and cmp_by seem to be helper functions that are only used by partial_cmp and cmp, respectively. The rest of the comparison functions (lt, gt, ne, etc) are built with partial_cmp, cmp, and eq though. Using that approach seems like a good idea, if feasible. What's the syntax for using a Future within another Future? Would you just use '.await'?

[yoshuawuyts]

What's the syntax for using a Future within another Future?

You would need to store the future internally, and then manually poll it to completion in a new future. A good example of this would be @montekki's recent series of patches in #262.

[ghost]

Is Self: Stream necessary at all?

[assemblaj]

When I remove + Stream from this clause , it builds and passes tests but I get the following error from rustdoc:

error[E0277]: the trait bound `Self: stream::stream::Stream` is not satisfied
    --> src/stream/stream/mod.rs:1180:9
     |
1180 | /         fn cmp<S>(
1181 | |            self,
1182 | |            other: S
1183 | |         ) -> impl Future<Output = Ordering> [CmpFuture<Self, S>]
...    |
1189 | |             CmpFuture::new(self, other)
1190 | |         }
     | |_________^ the trait `stream::stream::Stream` is not implemented for `Self`
     |
     = help: consider adding a `where Self: stream::stream::Stream` bound

[ghost]

Are we missing Self::Item: Ord here? I see Iterator::cmp has this bound.

[assemblaj]

This is correct, I'd erroneously removed this to pass rustdoc tests. Pushing a commit to fix this.