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

Experiment with a proc-macro-free API #71

Open
SabrinaJewson opened this issue Mar 29, 2022 · 2 comments · May be fixed by #74
Open

Experiment with a proc-macro-free API #71

SabrinaJewson opened this issue Mar 29, 2022 · 2 comments · May be fixed by #74

Comments

@SabrinaJewson
Copy link
Contributor

I've been thinking that it would be possible for this crate to provide an API that doesn't use proc macros at all, which has a couple of benefits:

The API could look like this:

/// async-stream ///

pub fn stream<T, F, Fut>(f: F) -> impl Stream<Item = T>
where
    F: FnOnce(Yielder<T>) -> Fut,
    Fut: Future<Output = ()>,
{ /* ... */ }

pub fn try_stream<T, E, F, Fut>(f: F) -> impl Stream<Item = Result<T, E>>
where
    F: FnOnce(Yielder<T>) -> Fut,
    Fut: Future<Output = Result<(), E>>,
{ /* ... */ }

// This macro will shadow the yielder with a function that borrows from a local.
//
// It will panic if called after the first poll or from a different stream.
#[macro_export]
macro_rules! start_stream {
    // $yielder must be of type Yielder<T>
    ($yielder:ident) => { /* ... */ };
}

/// Usage ///

let stream = async_stream::stream(|yielder| async move {
    // Must be called in the first poll, otherwise the stream will panic
    start_stream!(yielder);
    yielder(1).await;
    yielder(2).await;
    yielder(3).await;
});

I'm pretty sure this would be sound. Ergonomically, we'd lose the nice for await and yield syntax as well as the ability to use ? in regular streams (although users can always use a try_stream and then flatten the results if they want something like that), but we'd also gain the ability to specify the type of stream with turbofish syntax. I think it might be nice to support both versions in the library, depending on users' preferences. Any thoughts on the design?

@SabrinaJewson SabrinaJewson linked a pull request May 25, 2022 that will close this issue
@Nugine
Copy link

Nugine commented Feb 2, 2023

I have already implemented this kind of API two years ago. See https://github.com/Nugine/transform-stream.

In async-stream, it may cause unsoundness when

  • the yielder escapes the scope of stream
  • two stream exchange their yielder

There is no zero-cost solution without nightly features.

Related:

// Tracks the pointer to `Option<T>`.
//
// TODO: Ensure wakers match?
thread_local!(static STORE: Cell<*mut ()> = Cell::new(ptr::null_mut()));

@SabrinaJewson
Copy link
Contributor Author

My implementation in the linked PR is indeed not zero cost — but async streams are in general not really zero cost, but since the cost is a single pointer and it wasn’t zero cost to begin with, it is probably unproblematic.

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 a pull request may close this issue.

2 participants