What about adding a new vis specifier (allowed only on macro_rules) that keeps the current behavior? Maybe something like
pub(macro_rules) macro_rules! ...
The existing declarative macros can be gracefully migrated to this new vis, but new ones will be more hygienic.
This gets complicated for whether it's now something that I can pass to any other macro using a vis matcher...
Can you elaborate? I'm having a hard time thinking of reasons why you wouldn't just allow it ina vis matcher, or why not allowing it would be a problem.
The only reason pub(crate) works is that the restricted visibility path starts with a strong keyword, and macro_rules is a weak/contextual keyword. I.e. you can name an item macro_rules and rustc is perfectly fine with that. (Even a macro_rules! macro_rules, apparently.) It's not a complete deal breaker like pub(in path) since it's not unbounded lookahead, but it's still not desirable to need to look at pub ( macro_rules ) $peek to determine if that's a restricted visibility or a parenthesized type name. And that lookahead is probably incompatible with macro matcher parsing, which is mostly contextless.
If we really want to change unannotated macro_rules! to have pub(self) behavior, the best way imo is to annotate the macro_rules! item with #[macro_use] for non-exporter macros or #[macro_export] for exported macros. (New-style exported macros should be pub macro_rules!, and have no need of #[macro_export]).
But even then, textual scoping of macros is very useful for macro-generated helper macros; e.g. if I'm defining a continuation-passing macro, I can just emit macro_rules! __continuation and never run into item redefinition errors. This is a niche use case, to be fair, but it is a) probably the most deliberate benefactor of current macro scoping rules, b) potentially impossible to emulate with item-scoped macros, and c) impossible to rustfix, as its in the expansion of a macro. (Though it'd ofc use the edition of the defining crate, so it'd only break if the defining crate upgrades edition, not consuming crates.)
Thanks for explaining
This change will have to be done on an edition boundary anyways, right? There's nothing stopping us from making macro_rules a strict keyword on the same edition boundary, is there? Seems like it probably should be a keyword anyways, and it's unlikely to break much.
Regardless, if an attribute would work just as well, you could add an attribute when migrating instead:
#[textual_macro]
macro_rules! foo { ... }
It is correct that we could make macro_rules a strong keyword, and that would mean pub(macro_rules) wouldn't have any issues. But I still think using the existing #[macro_use] and #[macro_export] attributes and having them imply legacy macro namespacing is preferable to introducing a pub(macro_rules) visibility.
This also means that it's possible to write warning-free code on editionNow which compiles (ideally warning-free) with the same semantics on editionNext. It isn't a hard requirement that this holds with the default lint set (migration lints may sometimes be allow-by-default), but it is a requirement that code can be written that satisfies this property, in order for rustfix to work. (It applies all of the migration fixes in editionNow, and the code should be testable in that state before flipping the edition switch.)
Bonus(?): #[macro_use] is already permitted on macro_rules! items with the desired semantics (changing nothing in editionNow). It's an unused attributes warning:
warning: `#[macro_use]` only has an effect on `extern crate` and modules
--> src/lib.rs:1:1
|
1 | #[macro_use]
| ^^^^^^^^^^^^
2 | macro_rules! m {
| ^
|
= note: `#[warn(unused_attributes)]` on by default
This means the change would "just" need to be:
$:vis macro_rules! items, which are item-scoped names instead of legacy macro-scoped.#[macro_use] to a macro_rules! item.#[macro_use] and #[macro_export] are a hard error when used on an item-scoped macro definition (instead of warn(unused_attributes)).#[macro_use] to any legacy-scoped macro without a visibility modifier or #[macro_use]. (machine_applicability=always)#[macro_use] and #[macro_export] are permitted on default-visibility macro_rules! items, causing them to have legacy macro scoping instead of item scoping.macro_rules! items without one of those attributes have pub(self) item-scopedmacro_rules keyword. (Using the item name edition is also a potentially relevant choice, but I believe an inferior one w.r.t. composition, e.g. macro expanded macros, e.g. expanding macro_rules! $ident should get my edition's behavior, not $ident's.)macro_rules! m {} $vis use m; pattern and suggests to use an item-scoped $vis macro instead. (machine_applicability=iffy)#[macro_use] on a module without any legacy-scoped macros to use as an unused attribute.#[macro_export], suggesting to use pub instead. (machine_applicability=iffy)#[macro_use] macros, suggesting to use pub(crate) instead. (machine_applicability=iffy)warn(unused_attributes) always a hard error for #[macro_use] and #[macro_export].pub(self) macro_rules! items to remove the now unnecessary pub(self).I included a bunch of nicety things of varying difficulty, but none of those are needed for an edition migration to be viable. And even with solely $vis macro_rules! to provide item-scoped macros and doing nothing else[1] on this list, it's still majorly beneficial IMHO.
... Interesting consequence: $:vis matches the empty string. So $vis:vis macro_rules! $m:ident { $(tt:tt)* } will match a default-visibility macro definition, but emitting it as $vis macro_rules! $m { $($tt)* } might be recognized as an item-scoped macro instead of a default-visibility one, depending on the exact impl. Plus, we're replacing the call-site macro_rules! token with a def-site one, so we'd use the def-site edition. An area to be careful; either choice could potentially be argued as preferable, and both result in the same behavior for an editionNext-defined macro. (An editionCurrent macro emitting $vis macro_rules! also likely expects it to be item scoped. I'm biased towards this direction; it's imo not particularly weirder than other edition mixing oddities, and likely easier and more predictable to use than making an empty $vis capture create a legacy scoped macro in editionNow.)
Well, actually, making #[macro_use]/#[macro_export] a hard error for item-scoped macros is also pretty necessary to avoid potential confusion. âŠī¸
Mostly that it seems weird to have other_macro! { pub(macro_rules) struct Foo } or similar start accepting it at macro matching time.
It's not the end of the world, certainly, but it'd be nice if a temporary workaround didn't change what all uses of the macro matcher going forward will accept.
This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.