Skip to content

Expand 'yield' in internal macro calls.#57

Merged
taiki-e merged 4 commits intotokio-rs:masterfrom
SergioBenitez:support-macros
May 22, 2021
Merged

Expand 'yield' in internal macro calls.#57
taiki-e merged 4 commits intotokio-rs:masterfrom
SergioBenitez:support-macros

Conversation

@SergioBenitez
Copy link
Contributor

Resolves #27.

As in the title.

@SergioBenitez
Copy link
Contributor Author

I can't replicate the test failures locally. It seems the test CI is simply running cargo test +stable --all-features, which passes on my machine with flying colors. What am I missing?

Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't replicate the test failures locally. It seems the test CI is simply running cargo test +stable --all-features, which passes on my machine with flying colors. What am I missing?

I'm not sure if it's a trybuild bug or a rustc bug, (sometimes) test results in CI may differ from those run locally...

Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This looks good to me aside from a few nits.

@SergioBenitez SergioBenitez force-pushed the support-macros branch 2 times, most recently from 90175d0 to c46ada9 Compare April 18, 2021 11:17
@SergioBenitez
Copy link
Contributor Author

Cool. This looks good to me, too!

@SergioBenitez
Copy link
Contributor Author

What's keeping this from being merged?

@taiki-e
Copy link
Member

taiki-e commented May 10, 2021

Sorry, I forgot about this, I'll review it again and merge it if it looks good.

Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this breaks code that has another async stream macro inside the macro.

use tokio::select;
use async_stream::{stream, try_stream};
use futures_util::stream::StreamExt;

async fn do_stuff_async() {}

let s = stream! {
    select! {
        _ = do_stuff_async() => {
            let another_s = try_stream! {
                yield;
            };
            let _: Result<(), ()> = Box::pin(another_s).next().await.unwrap();
        },
        else => {},
    }
    yield
};

@SergioBenitez
Copy link
Contributor Author

Good catch! Should be good to go.

@SergioBenitez
Copy link
Contributor Author

Fixed the rustfmt issue.

Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@taiki-e taiki-e merged commit 9b7e9a7 into tokio-rs:master May 22, 2021
@SergioBenitez
Copy link
Contributor Author

Any chance this can find its way into a release? I'm currently exporting stream from my fork but would love to depend on upstream instead. This would also make it possible to publish a release that depends on the macro.

@taiki-e taiki-e mentioned this pull request May 22, 2021
@taiki-e
Copy link
Member

taiki-e commented May 22, 2021

Filed #60 for next release.

@taiki-e
Copy link
Member

taiki-e commented May 23, 2021

Published in v0.3.2.

@SergioBenitez
Copy link
Contributor Author

Awesome, awesome, awesome. Thank you!

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.

yield inside tokio select! doesn't work

2 participants