async-stream icon indicating copy to clipboard operation
async-stream copied to clipboard

Don't force zero-yield stream item type of '()'

Open SergioBenitez opened this issue 4 years ago • 6 comments
trafficstars

For streams that never yield, the implementation forces the Item type to be inferred as () with a yield that never occurs. This has the consequence of making it impossible to create an empty stream for items of types other than (). That is, to write the function:

fn any<T>() -> impl Stream<Item = T> {
    stream! {}
}

This is a pragmatic concern. In testing keep-alive heartbeats, I would like, but am unable, to write non-() streams of the following sort without falling back to a needless map(|_| unreachable!()):

stream! {
    sleep(Duration::from_secs(10)).await;
}

This is a breaking change with likely minimal impact.

P.S: It would be ideal to address #33 to mitigate this and other type-related issues in the future. Syntax to specify the return type could be:

stream! { String =>
    sleep(Duration::from_secs(10)).await;
}

SergioBenitez avatar May 30 '21 13:05 SergioBenitez

ping @taiki-e

SergioBenitez avatar Jun 08 '21 01:06 SergioBenitez

I guess the current behavior is based on futures-await (https://github.com/alexcrichton/futures-await/pull/32) or futures-async-stream (https://github.com/rust-lang/futures-rs/pull/1548), but I have no strong opinion on this at this time (to be exact: there isn't enough bandwidth to think about this breaking change thoroughly...).

taiki-e avatar Jun 16 '21 21:06 taiki-e

cc @carllerche @Darksonn @Kestrer: any thoughts on this?

taiki-e avatar Jun 16 '21 21:06 taiki-e

As an alternative, we can instead insert:

if false {
    yield unreachable!();
}

Like Sergio's solution it is able to produce a stream of any type, but while it is still breaking it breaks less because the following examples still compile:

stream! {};
fn takes_debug(stream: impl Stream<Item = impl Debug>) {}
takes_debug(stream! {});

Kestrer avatar Jun 17 '21 19:06 Kestrer

@SergioBenitez what do you think about @Kestrer's suggestion? In general, I'm inclined to go in any direction here.

It isn't a big deal to push out breaking releases of the async-stream crate.

carllerche avatar Jun 17 '21 22:06 carllerche

By the way: if you do implement this, use yield loop {}; instead of yield unreachable!(); because it doesn't need any hygiene shenanigans.

Kestrer avatar Jun 19 '21 13:06 Kestrer