whatisaphone / genawaiter

Stackless generators on stable Rust.
https://docs.rs/genawaiter
437 stars 28 forks source link

Gobbled value in generator #35

Open dureuill opened 2 years ago

dureuill commented 2 years ago

Thank you for making genawaiter!

Configuration

Code

use genawaiter::{
    rc::gen,
    yield_,
};

fn main() {
    let mut printer = gen!({
        loop {
            let string = yield_!(());
            println!("{}", string);
        }
    });
    printer.resume_with("0");
    printer.resume_with("1");
    printer.resume_with("2");
    printer.resume_with("3");
}

Expected output:

0
1
2
3

or at least,

0
1
2

Observed output

1
2
3

It looks like the first value passed to the generator is never printed? I'm not sure why that is.

Is there something I'm misunderstanding?

dsemi commented 2 years ago

I can't say whether this is intentional, but I believe what's happening is that the initial state returned by gen! is "unstarted". i.e.

  1. Assign generator to printer
  2. resume_with("0") starts the generator, but then stops at the first yield!(())
  3. resume_with("1") resumes from the yield!(()), receives the "1" and prints it
  4. etc.

This aligns with how rust async functions work in that no work is performed until an await occurs, so in that sense the behavior tracks. However, the genawaiter doc page contains:

let mut printer = gen!({
    loop {
        let string = yield_!(());
        println!("{}", string);
    }
});
printer.resume_with("hello");
printer.resume_with("world");

which confuses users as in reality this only prints "world".

dureuill commented 2 years ago

I understand that there ought to be a method to move the generator from "unstarted" to the first value-accepting yield, however as-is the API feels "wrong" to me:

  1. it asks for a value that is not used and that is silently dropped
  2. that resume needs to be called with a dummy value is unclear and confusing, as even the documentation gets it wrong.

Maybe we need to expose the two states of the generator:

  1. unstarted, has a start method without argument.
  2. started, has the current resume_with method taking an argument.
marcgalois commented 2 years ago

I agree that an API which doesn't discard arguments would be preferable. I will note, though, that the documentation does at least say that "the first resume argument will be lost."

dureuill commented 2 years ago

I agree that an API which doesn't discard arguments would be preferable. I will note, though, that the documentation does at least say that "the first resume argument will be lost."

Thank you! I wasn't aware of this bit of documentation. This at least confirm that the behavior is intended.

It is unfortunate, however. As a user, it means my ResumeType must provide a "dummy value" that can safely be built and lost on the first call. This means than the inside of the generators has to account for the possibility that a ResumeType value has a "dummy value", even if it unreachable.

My concrete case here is an infinite looping generator that takes commands via resume, and returns results. The Command type has to include a Dummy or Start command that is only used for the purpose of starting the generator.

Types containing "dummy values" strike me as unidiomatic Rust, and can lead to error (such as clients sending the Start command on an already started generator).

I'm not sure what we could do here without breaking backcompatibility though.