Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SergioBenitez
Copy link
Contributor

@SergioBenitez SergioBenitez commented May 30, 2021

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
Copy link
Contributor Author

ping @taiki-e

@taiki-e
Copy link
Member

taiki-e commented Jun 16, 2021

I guess the current behavior is based on futures-await (alexcrichton/futures-await#32) or futures-async-stream (rust-lang/futures-rs#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
Copy link
Member

taiki-e commented Jun 16, 2021

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

@Kestrer
Copy link
Contributor

Kestrer commented Jun 17, 2021

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! {});

@carllerche
Copy link
Member

@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.

@Kestrer
Copy link
Contributor

Kestrer commented Jun 19, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants