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

allow importing macros without helpers in the 2018 edition #1041

Merged
merged 1 commit into from
Apr 12, 2019

Conversation

euclio
Copy link
Contributor

@euclio euclio commented Apr 2, 2019

This PR adds the $crate:: prefix to all uses of helper macros, which will allow users of the 2018 edition to import the macros without needing to also import their helpers.

See https://doc.rust-lang.org/edition-guide/rust-2018/macros/macro-changes.html#macros-with-crate-prefix for more info.

While this is a cleaner solution than using #[macro_export(local_inner_macros)], it also requires bumping the minimum supported compiler version to 1.30. If this is acceptable, I'll add a commit to update the Travis configuration and docs.

This PR now uses local_inner_macros to preserve compatibility with the current minimum Rust version.

@asomers
Copy link
Member

asomers commented Apr 2, 2019

Nix has traditionally been pretty conservative about raising the minimum compiler version, and this is a big bump. It could make Nix unusable by some distros' Rust packages. Worse, it might break platforms not supported by rustup that don't yet have compiler 1.30.0. Do you have any data about how widespread this compiler is?

Also, how did you tell github that this PR is a draft? I haven't seen that feature before.

@euclio
Copy link
Contributor Author

euclio commented Apr 2, 2019

I don't have any data for the usage of 1.30.0, unfortunately. If we're not comfortable bumping to 1.30.0, I can see how feasible using local_inner_macros is.

When you create a PR, you can click a dropdown next to the PR submission button to mark a PR as "Draft" :)

@euclio
Copy link
Contributor Author

euclio commented Apr 2, 2019

Actually, here is some data on 1.30.0 usage: https://repology.org/project/rust/versions

@euclio euclio force-pushed the use-dollar-crate branch from 4c8bd2c to 4ef53b2 Compare April 3, 2019 17:30
@euclio euclio marked this pull request as ready for review April 3, 2019 17:36
@euclio
Copy link
Contributor Author

euclio commented Apr 3, 2019

@asomers I'm now using local_inner_macros, which should preserve compatibility with the current minimum Rust version.

@asomers
Copy link
Member

asomers commented Apr 3, 2019

Thanks for making that change. And thanks for the Rust versions link; I didn't know about that. But this needs a CHANGELOG entry. Can you please add one and describe how the visibility of macros has changed?

This will allow users of the 2018 edition to import the macros without
needing to also import their helpers.
@euclio euclio force-pushed the use-dollar-crate branch from 4ef53b2 to 8c9ac5a Compare April 4, 2019 18:01
@euclio
Copy link
Contributor Author

euclio commented Apr 4, 2019

Added an entry to the changelog.

@asomers
Copy link
Member

asomers commented Apr 5, 2019

If the helper macros are no longer importable, then they should be mentioned in the "Removed" section of the changelog.

@euclio
Copy link
Contributor Author

euclio commented Apr 5, 2019

They're still importable, it's just that you don't need to explicitly import them to use macros that use them internally anymore.

@euclio
Copy link
Contributor Author

euclio commented Apr 12, 2019

Friendly ping @asomers

Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

Ok, I get it. It's too bad we didn't notice local_inner_macros before; it could've been useful!

bors r+

bors bot added a commit that referenced this pull request Apr 12, 2019
1041: allow importing macros without helpers in the 2018 edition r=asomers a=euclio

~This PR adds the `$crate::` prefix to all uses of helper macros, which will allow users of the 2018 edition to import the macros without needing to also import their helpers.~

See https://doc.rust-lang.org/edition-guide/rust-2018/macros/macro-changes.html#macros-with-crate-prefix for more info.

~While this is a cleaner solution than using `#[macro_export(local_inner_macros)]`, it also requires bumping the minimum supported compiler version to 1.30. If this is acceptable, I'll add a commit to update the Travis configuration and docs.~

This PR now uses `local_inner_macros` to preserve compatibility with the current minimum Rust version.

Co-authored-by: Andy Russell <arussell123@gmail.com>
@bors
Copy link
Contributor

bors bot commented Apr 12, 2019

Build succeeded

@bors bors bot merged commit 8c9ac5a into nix-rust:master Apr 12, 2019
@euclio euclio deleted the use-dollar-crate branch April 12, 2019 19:16
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.

2 participants