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

Support reexporting from anywhere by wrapping in a declarative macro #46

Merged
merged 5 commits into from
Dec 27, 2020
Merged

Support reexporting from anywhere by wrapping in a declarative macro #46

merged 5 commits into from
Dec 27, 2020

Conversation

Kestrer
Copy link
Contributor

@Kestrer Kestrer commented Dec 27, 2020

This PR allows the stream! and try_stream! macros to be reexported from anywhere by having the procedural macro take a path to async_stream, and having the macros in async-stream be declarative macros that wrap the procedural ones and pass in $crate.

As a downside we get slightly worse error messages as the filename and
line number will point to the macro definition site instead of the call
site.
@Kestrer
Copy link
Contributor Author

Kestrer commented Dec 27, 2020

Why is CI failing? It works on my machine, and we're running the same version of rustc...

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 for the PR. I don't think this will work properly if async_stream (actual path of $crate) is not included in the direct dependency. I believe you need to use a trick like rust-lang/futures-rs#2124. EDIT: #46 (comment)

In any case, this PR needs a test to make sure reexporting works.

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.

Why is CI failing? It works on my machine, and we're running the same version of rustc...

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

async-stream/tests/ui/yield_in_async.stderr Outdated Show resolved Hide resolved
async-stream/tests/ui/yield_in_async.stderr Show resolved Hide resolved
async-stream/tests/ui/yield_in_closure.stderr Outdated Show resolved Hide resolved
@Kestrer
Copy link
Contributor Author

Kestrer commented Dec 27, 2020

The $crate method does seem to work, I added a test for it.

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! I'm surprised that this also works with reexport. I think I saw the code generated by --pretty=expanded and misunderstood it didn't work (sorry for the wrong review!).

@taiki-e taiki-e merged commit 200466e into tokio-rs:master Dec 27, 2020
@taiki-e taiki-e mentioned this pull request Jan 20, 2021
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.

None yet

2 participants