resolve: Modularize crate-local #[macro_export] macro_rules
#52234
+415
−32
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try |
petrochenkov
added
S-waiting-on-crater
bors
added a commit
that referenced
this pull request
Jul 10, 2018
[Experiment] resolve: Modularize crate-local `#[macro_export] macro_rules` Based on #50911, cc #50911 (comment) `#[macro_export] macro_rules` items are collected from the whole crate and are planted into the root module as items, so the external view of the crate is symmetric with its internal view and something like `$crate::my_macro` where `my_macro` is `#[macro_export] macro_rules` works both locally and from other crates.
|
ping @pietroalbini |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
💔 Test failed - status-travis |
bors
added
S-waiting-on-author
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
1 similar comment
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Is it necessary to fix the std doc generation mode ( |
|
I don't think std docs needs to be working to get a crater run, but a successful try build is needed. |
|
@bors try |
|
⌛ Trying commit 1f844a847d62ded4322840a4ddbb499e0228cd48 with merge d39a6f7eddafd45dcc60e99001b8cd08c2bd3255... |
petrochenkov
added
S-waiting-on-crater
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
☀️ Test successful - status-travis |
|
@craterbot run start=master#ae5b629efd79de78e6ba7ef493c32857bd7f9cf9 end=try#d39a6f7eddafd45dcc60e99001b8cd08c2bd3255 mode=check-only |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across the whole Rust ecosystem. Learn more |
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across the whole Rust ecosystem. Learn more |
|
🎉 Experiment
|
|
|
|
@CAD97 The issue is that any Indeterminacies like this cause import resolution to stuck, like it happened in #53144 and similar cases. Perhaps the error can be relaxed a bit / made more fine-grained, but I haven't thought how to do that in detail. |
|
Hello, I've got a bit of a question, as I've just run into this error during implementation. Since referral by absolute paths is an error, how is the external use of a macro-generated macro supposed to occur? |
|
@Kixiron |
|
The error mentions "by absolute paths", but if in the (failing) test case 3: one changes to a definitely-relative path like Furthermore, if one encloses the macros inside a module: Then the compiler suggests using |
31 tasks
|
I don't think I'm understanding how I'm meant to work around this. Do I really need to |
|
Indeed, this seems like a huge problem for hygine. I was writing a macro that needed to expand a helper macro and I found it incredibly problematic, as the fcw triggered when adding |
|
Regarding Regarding a more general workaround, the trick is to "append a
@petrochenkov IIUC, if the issue with |
nwtnni
added a commit
to nwtnni/xic-rs
that referenced
this pull request
May 10, 2022
Using the re-exporting trick from rust-lang/rust#52234 (comment) to reference the macro recursively at a known path. This way any external callers don't have to import the macro or its dependencies.
nwtnni
added a commit
to nwtnni/xic-rs
that referenced
this pull request
May 10, 2022
Using the re-exporting trick from rust-lang/rust#52234 (comment) to reference the macro recursively at a known path. This way any external callers don't have to import the macro or its dependencies.
nwtnni
added a commit
to nwtnni/xic-rs
that referenced
this pull request
May 10, 2022
Using the re-exporting trick from rust-lang/rust#52234 (comment) to reference the macro recursively at a known path. This way any external callers don't have to import the macro or its dependencies.
|
For anyone stumbling here, I fixed it using like the following, #[macro_export]
macro_rules! impl_str_id {
($t:ident) => {
::paste::paste! {
#[derive(Clone, Copy, Hash, Deref)]
#[display("{0}")]
pub struct $t(TinyStr16);
impl_new_type_debug_no_wrap!($t);
#[macro_export]
macro_rules! [<_ $t:snake:lower>] {
($s:literal) => {{ $t::new(tinystr::tinystr!(16, $s)) }}
}
// Fixes macro-expanded `macro_export` macros from the current crate cannot be referred to by absolute paths
#[doc(hidden)]
pub use [<_ $t:snake:lower>] as [<$t:snake:lower>]; |
|
Regarding #[macro_export]
macro_rules! recur {
(()) => {$crate::recur!{}};
() => ()
}However, this strategy requires accessing the macro by an absolute path so if the macro_rules! mk_recur {() => (
#[macro_export]
macro_rules! recur {
(()) => {$crate::recur!{}};
() => ()
}
pub use recur;
)}
pub mod m1 {
mk_recur!{}
}
mod m2 {
crate::m1::recur!{()}
} |
dannymcgee
added a commit
to dannymcgee/gramatika
that referenced
this pull request
Apr 22, 2024
# Breaking changes
## Generated token macros are no longer `#[macro_export]`
The `Token` derive macro generates a `macro_rules!` macro for each of the enum
variants. E.g., for Lox's `Token` enum in `examples/lox`:
```rs
// examples/lox/src/tokens.rs
#[derive(DebugLispToken, PartialEq, Token, Lexer)]
pub enum Token {
#[subset_of(Ident)]
#[pattern = "and|class|else|false|for|fun|if|nil|or|print|return|super|this|true|var|while"]
Keyword(Substr, Span),
#[pattern = "[a-zA-Z_][a-zA-Z0-9_]*"]
Ident(Substr, Span),
#[pattern = r"[(){}]"]
Brace(Substr, Span),
#[pattern = "[,.;]"]
Punct(Substr, Span),
#[pattern = "[=!<>]=?"]
#[pattern = "[-+*/]"]
Operator(Substr, Span),
#[pattern = "[0-9]+"]
NumLit(Substr, Span),
#[pattern = r#""[^"]*""#]
StrLit(Substr, Span),
}
```
We get `macro_rules!` macros named `keyword`, `ident`, `brace`, `punct`,
`operator`, `num_lit` and `str_lit`. These are mostly useful for the `Parse`
implementations, e.g.:
```rs
// examples/lox/src/decl.rs
impl Parse for ClassDecl {
type Stream = ParseStream;
fn parse(input: &mut Self::Stream) -> Result<Self> {
// ...
let superclass = if input.check(operator![<]) {
input.consume(operator![<])?;
Some(input.consume_kind(TokenKind::Ident)?)
} else {
None
};
// ...
}
}
```
Previously, those generated macros were declared like this:
```rs
#(
#[macro_export]
macro_rules! #snake_case_variant_ident {
// ...
}
)*
```
That has always [caused some headaches](rust-lang/rust#52234 (comment))
which I was never really sure how to deal with. In the examples here, and in my
consuming projects, I had resorted to using `*` imports everywhere to work
around the problem (in hindsight though, I think that likely just hides the lint
by obscuring what we're doing instead of actually addressing the issue).
This PR attempts an alternative solution. `#[macro_export]` is intended for
libraries to expose macros to external consumers, but I don't foresee the macros
generated by the `Token` derive being actually useful in that context. So
instead, the generated macros are now declared like this:
```rs
#(
macro_rules! #snake_case_variant_ident {
// ...
}
pub(crate) use #snake_case_variant_ident;
)*
```
This is a breaking change for two reasons:
1. If you were depending on those `#[macro_export]` attributes to make the
generated macros available to external consumers of your library, that is no
longer going to work. Again, I don't imagine this was actually a real-world
use case for anyone, but I've been wrong before! Let me know if this is a
problem for you and I'll see what we can do about it.
2. If you had been importing those macros from the root of your crate, but your
`Token` enum is _not_ declared in the crate root, you'll need to update your
import paths to instead import them from the module where the enum is
declared. E.g.:
```rs
mod token {
use gramatika::{DebugLisp, Span, Substr, Token as _};
#[derive(DebugLispToken, PartialEq, Token, Lexer)]
pub enum Token {
#[pattern = ".+"]
Word(Substr, Span),
}
}
mod foo {
// use crate::word; // 👎
use crate::token::word; // 👍
}
```
On the bright side, tools like `rust-analyzer` should now find and
automatically suggest the correct import paths for those macros, so the
fastest way to migrate will probably be to just delete your existing `use`
statement and invoke your editor's suggestion feature to re-import any
unresolved symbols from their correct paths.
# Other changes
* Updated the Rust edition to 2021 and fixed any resulting errors
* Fixed any new `clippy` lints as a result of upgrading my environment to
v1.77.2
* Performed some low-hanging-fruit dependency upgrades.
`regex-automata` and `syn` are still out of date for now — I attempted
to update the former, but couldn't figure out how to migrate after ~10 minutes
of poking around, and unfortunately I have other priorities that need to take
precedence. Didn't even attempt `syn` because it's a major version bump, and
that crate is basically the backbone of this whole project, so it'll have to
wait for now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #50911, cc #50911 (comment)
#[macro_export] macro_rulesitems are collected from the whole crate and are planted into the root module as items, so the external view of the crate is symmetric with its internal view and something like$crate::my_macrowheremy_macrois#[macro_export] macro_rulesworks both locally and from other crates.Closes #52726