smol-rs / futures-lite

Futures, streams, and async I/O combinators.
Apache License 2.0
439 stars 25 forks source link

Example in `futures_lite::ready!` docs is incorrect #31

Closed ComputerDruid closed 3 years ago

ComputerDruid commented 3 years ago

https://docs.rs/futures-lite/1.11.1/futures_lite/macro.ready.html#examples

Lists an example usage of:

use futures_lite::future::FutureExt;
use futures_lite::ready;
use std::future::Future;
use std::pin::Pin;
use std::task::{Context, Poll};

// Polls two futures and sums their results.
fn poll_sum(
    cx: &mut Context<'_>,
    a: Pin<&mut impl Future<Output = i32>>,
    b: Pin<&mut impl Future<Output = i32>>,
) -> Poll<i32> {
    let x = ready!(a.poll(cx));
    let y = ready!(b.poll(cx));
    Poll::Ready(x + y)
}

I'm pretty sure this is incorrect because if a.poll() returns Ready but b.poll() returns Pending, a will get poll()ed twice, which isn't allowed: https://doc.rust-lang.org/std/future/trait.Future.html#panics

ComputerDruid commented 3 years ago

I'll also note that I'm generally confused about why ready! is useful for this reason; presumably it is useful because I remember seeing it in code I've read a bunch, but I haven't tried to hand-write very many futures myself.

ghost commented 3 years ago

Indeed, you're right. Now I copied the docs from std::task::ready! and published a new version: https://doc.rust-lang.org/nightly/std/task/macro.ready.html

ComputerDruid commented 3 years ago

Nice, perfect.