estebank / iterator_item Goto Github PK

I don't understand why a specific marker (like the star next to fn) would be needed? I don't think that it was correctly justified.

Bjarne Stroustroup, creator of C++, said that for every new features, people wants extra marker, but once they are used to it, they want it removed. It's why there is template<typename T> void foo(t T) in C++ and not void foo<T>(t T)> in C++.

I also don't understand why a marker need to be in the public part of the API. Does this means that if at one point I want to replace a manually written iterator by a generator (or vice-versa) this is a breaking change even if it should only be an implementation detail?

I have toyed with generator syntax myself, and my favorite syntax would be to use => for the yield type, which IMHO is very readable and succinct, using the mnemonic "one-lined arrow for the final return that happens once, two (i.e., multi)-lined arrow for yields that may happen more than once/multiple times". This syntax composes quite nicely with -> for the return type, and with async streams. Using the type names from the Generator trait:

fn(...) -> Return { ... }                 // regular function
fn(...) => Yield { ... }                  // generator function
fn(...) => Yield -> Return { ... }        // generator function that has a non-() final return

async fn(...) -> Return { ... }           // regular async function
async fn(...) => Yield { ... }            // async stream
async fn(...) => Yield -> Return { ... }  // async stream, returning non-() final return

|...| => Yield -> Return { ... }          // generator closure
async | ... | => Yield -> Return { ... }  // async stream closure

Arguably, that the difference between => and -> could be hard to spot on a glance, and a separate keyword in front, in order to indicate that the body of the function is re-written into a state machine, would be preferable. In this case I would argue for the commonly proposed yield fn syntax, but still with using => to indicate the yield type:

fn(...) -> Return { ... }                       // regular function
yield fn(...) => Yield { ... }                  // generator function
yield fn(...) => Yield -> Return { ... }        // generator function that has a non-() final return

async fn(...) -> Return { ... }                 // regular async function
async yield fn(...) => Yield { ... }            // async stream
async yield fn(...) => Yield -> Return { ... }  // async stream, returning non-() final return``` 

yield | ... | => Yield -> Return { ... }        // generator closure
async yield | ... | => Yield -> Return { ... }  // async stream closure

Finally, one could shoe-horn the semicoroutine resumption type R in here as well if required. My suggestion for this would be:

yield fn (...) => Yield <= | R1, R2, ... | -> Return { ... }  // R1, R2, ... being members of the resumption tuple R

I very much like the proposed yield syntax, and I think we should use that syntax.

However, I don't want to hide the iterator type behind yields. I'd like to show the iterator type, with something like -> impl Iterator<Item = ReturnType> (or analogously for AsyncIterator).

I acknowledge that this is more verbose. However, I think it'll make the behavior and function type more obvious, and I consider it analogous to -> Result<T, E> (which we don't hide).

This issue is primarily meant to log the semantic alternative I prefer. As it revolves around syntax to make fn() -> impl FnMut() -> Option<T> rather than fn() -> impl Iterator<Item=T>, it doesn't quite seem correct to implement it as a macro sketch here.

Full text: https://internals.rust-lang.org/t/rust-generators-exploration-of-potential-syntax/15586/8?u=cad97

It could look like:

fn merge_overlapping_intervals(input: impl Iterator<Interval>) -> impl Iterator<Item = Interval> {
    //         !1              !2
    std::iter::from_fn(move || loop {
        //                        !3
        let mut prev = input.next()?;
        for i in input {
            if prev.overlaps(&i) {
                prev = prev.merge(&i);
            } else {
                yield Some(prev); // !4
                prev = i;
            }
        }
        yield Some(prev); // !4
        yield None;       // !5
    });
}

Notes:

1. Our semicoroutine just produces impl FnMut() -> Option<Interval>. To turn this into an iterator, we use iter::from_fn.
2. We wrap the body in a loop. This is for convenience: otherwise, the implicit tail expression would return a (), which is not Option<Interval>, leading to a type error. As loop evaluates to type !, this coerces to our closure's return type just fine. Alternatively, we could have written the last line as a tail None. Note that return None; would not work^[4], as we would still have the implicit tail expression returning () after it.
3. ? works here. As our closure returns Option<Interval>, ? just works as normal. If input.next() yields a None, we resume from the top of the function the next time the closure is called.
4. We yield Some(interval) here, because this is a -> Option<Interval> closure.^[5] Through from_fn, this fills the Iterator contract of returning items via Some(interval).
5. We yield None to indicate the end of the iterator stream. If and only if input is fused, we could instead leave this off, looping back to the start of the closure, and input.next()? would yield None; however, because we don't require input to be fused, we have to yield it ourselves to ensure None is yielded before calling next on the input iterator again. It also just helps with code clarity to do so explicitly. This could also be written as a tail-return None rather than the closure being a big loop (see point 2).

A more limited stabilization would avoid yield closures and instead only allow yield fn:

yield fn merge_overlapping_intervals(input: impl Iterator<Interval>) -> Option<Interval> {
    let mut prev = input.next()?;
    for i in input {
        if prev.overlaps(&i) {
            prev = prev.merge(&i);
        } else {
            yield Some(prev);
            prev = i;
        }
    }
    yield Some(prev);
    None
}

See the linked irlo post (practically a blog post) for more context into why I think this model is a good model.