std::ops::Try impl for std::task::Poll

[cramertj]

I originally left out the Try impl for Poll because I was curious if we needed it, and @MajorBreakfast and I had discussed the potential for it to introduce confusion about exactly what control-flow was happening at different points. However, after porting a pretty significant chunk of Fuchsia over to futures 0.3, I discovered that I was constantly having to do repetitive matching on Poll<Result<...>> or Poll<Option<Result<...>>> in order to propagate errors correctly. try_poll (propagate Poll::Ready(Err(..))s) helped in some places, but it was far more common to need some form of conversion between Result, Poll<Result<...>>, and Poll<Option<Result<...>>>. The Try trait conveniently provides all of these conversions in addition to a more concise syntax (?), so I'd like to experiment with using these instead.

cc @seanmonstar

r? @aturon

Note: this change means that far more futures 0.1 code can work without significant changes since it papers over the fact that Result is no longer at the top-level when using Stream and Future (since it's now Poll<Result<...>> or Poll<Option<Result<...>>> instead of Result<Poll<..>> and Result<Poll<Option<...>>>).

[seanmonstar]

So that I understand these type signatures correctly, this is for converting Poll<Result>s into Polls? As in, it's not quite the same as the older try_ready!, which would give me T back, not wrapped in Poll::Ready(t), right?

[cramertj]

@seanmonstar Right, this is just like if you were to use ? in the futures-0.1 world (propagates errors, not readiness). You can combine this with the ready! macro to propagate Poll::Pending. So 0.3 try_ready!(x) maps to 0.3 ready!(x?), and 0.1 x? maps to 0.3 `x?.

[aturon]

I like this factoring (of just handling the error conversions here, and leaving macros for readiness projection). And in any case, 👍 for trying it out and seeing how it feels!

@bors r+

[bors]

📌 Commit bce8a91 has been approved by aturon

[bors]

🌲 The tree is currently closed for pull requests below priority 99, this pull request will be tested once the tree is reopened

[MajorBreakfast]

I think it's a good idea to try this out. However, it has to prove itself. In particular I would like to see some of our combinators converted to this new style.

BTW where is the difference between:

let x = ready!(future.poll()?);

let x = ready!(future.poll())?;

I see that it's different for poll_next() though. Seems complicated

So, let's try it, but I remain skeptical

[MajorBreakfast]

Okay never mind. I understand the difference now: from_error returns Poll::Ready(Err(e))

[cramertj]

@MajorBreakfast

BTW where is the difference between:
let x = ready!(future.poll()?);
let x = ready!(future.poll())?;

The first one will first check for errors, return them if necessary, then check for Poll::Pending and return if necessary. The second will do the opposite order. There's no difference in the end result, since you can't have both Poll::Pending and Poll:Ready(Err(...))/Poll::Ready(Some(Err(...)))

[MajorBreakfast]

@cramertj No I was wrong there. The second one won't actually work because ready! extracts the Result<T, E> from the Poll::Ready(_). Then, ? returns the E from the Err(_) variant which doesn't match the function's return type (Poll). Only the first one works.

[cramertj]

@MajorBreakfast

No I was wrong there. The second one won't actually work because ready! extracts the Result<T, E> from the Poll::Ready(). Then, ? returns the E from the Err() variant which doesn't match the function's return type (Poll). Only the first one works.

When using ? on a value, that value's type doesn't have to match the return type. ready!(future.poll())?, if ready! succeeds, would be like doing ? on a Result<T, E>, which (after this PR) will work fine in a function which returns Poll<Result<T, E>> or Poll<Option<Result<T, E>>>.

[MajorBreakfast]

@cramertj Ah I see. It seems that Try works slightly different to how I thought it would. Thx for the explainer!