Support pub on macro_rules

[petrochenkov]

This PR shows the amount of effort required to implement

    pub macro_rules! foo {
        ...
    }

which turns out to be... negative?

pub (or pub(...)) here makes the macro use regular module scoping (with specified visibility) used by macros 2.0 and all other non-macro items instead of the traditional macro_rules scoping.

pub provides a way to make a macro public without #[macro_export], so that #[macro_export] can be eliminated on the next edition, for example.

cc https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Getting.20rid.20of.20.60.23.5Bmacro_export.5D.20macro_rules.60.20on.202021.20edition
Tracking issue - #78855.

(This PR still needs a feature gate and more tests though.) UPD: Feature gate and tests are added.

[rust-highfive]

r? @varkor

(rust_highfive has picked a reviewer for you, use r? to override)

[matklad]

Impl effort for rust-analyzer would be slightly higher, but still minuscule, and, hey, I'd gave a kingdom for every removal of scoping breaching feature, if I had several spare kingdoms.

[petrochenkov]

Updated with a feature gate and tests.

[varkor]

r=me after @Aaron1011's comment has been addressed.

[bors]

☔ The latest upstream changes (presumably #78889) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

[petrochenkov]

TODO: Add a cross-crate test, looks like I forgot to "export" these macros during HIR lowering.

[joshtriplett]

We discussed this in the @rust-lang/lang meeting today.

We're quite enthusiastic about this.

We do think, procedurally, this needs at least a minimal spec (even if that spec is as simple as "this behaves the same as #[macro_export]). Some discussion about this potentially needing an RFC.

Is there any other minor transition on macros that we'd want to key off of this?

This is a major user-visible feature. We'll need to document it.

[joshtriplett]

Also, after this becomes stable, we should ideally have a rustfix-able lint on #[macro_export], even if that lint starts out allow-by-default.

[petrochenkov]

We do think, procedurally, this needs at least a minimal spec (even if that spec is as simple as "this behaves the same as #[macro_export]).

There's a documentation in the top post.

pub (or pub(...)) here makes the macro use regular module scoping (with specified visibility) used by macros 2.0 and all other non-macro items instead of the traditional macro_rules scoping.

It's indeed pretty simple.
pub(in path) macro_rules! m { ... } would behave in the same way as e.g. pub(in path) fn m() { ... } from name resolution point of view.

(Behaving same as #[macro_export] would make this change purely syntactic and would do only harm, because the goal is to eliminate the unconventional behavior exhibited by #[macro_export].)

---

Is there any other minor transition on macros that we'd want to key off of this?

Minor? Probably not.
All the further possible steps are major.

• Removing #[macro_export] on the next edition.
• Removing #[macro_use] on the next edition.
• Modularize macro_rules without pub on the next edition (i.e. make macro_rules equivalent to pub(self) macro_rules).

---

Also, after this becomes stable, we should ideally have a rustfix-able lint on #[macro_export], even if that lint starts out allow-by-default.

This is a major issue.
You can migrate from #[macro_export] or #[macro_use] to modularized macros in rustfixable way only in simplest cases.
It's not just replacing one syntax with another, the migration may require inserting imports into many modules, inserting public reexports, moving the macro from one module to another.
The last item (modularize macro_rules without pub) would be especially hard because there are non-trivial uses for macro_rules intentionally shadowing each other, which will need to be entirely rewritten perhaps using very different design.

[joshtriplett]

@petrochenkov Thank you for the clarification. Making macro_rules! behave just like other items sounds ideal. I would love to see a move to make macro_rules! private by default in a new edition, and require a visibility to make it more visible, just like every other item.

Based on the language used in https://doc.rust-lang.org/reference/macros-by-example.html#scoping-exporting-and-importing , would it be accurate to say that a macro_rules! macro that has a visibility attached will exclusively use path-based scoping, never textual scoping?

If so, I'd personally be happy to accept this PR as-is, as long as there's a patch to the reference before we stabilize it.

In theory, would it be possible for the 2021 edition to make path-based scoping the default, but provide some backwards-compatible way to ask for textual scoping, and do textual scoping if you write #[macro_export]? That might make it easier to migrate incrementally.

As for the lint, nevermind about rustfix; now that you've clarified what you mean, I think it'll be reasonable to just have a transition lint, once we have something to transition to.

[petrochenkov]

@joshtriplett

Based on the language used in https://doc.rust-lang.org/reference/macros-by-example.html#scoping-exporting-and-importing , would it be accurate to say that a macro_rules! macro that has a visibility attached will exclusively use path-based scoping, never textual scoping?

Yes, pub switches the macro to path-based scoping in that terminology.

[joshtriplett]

@petrochenkov OK, in that case, I'm leaving this nominated to re-discuss at next week's meeting.

[nikomatsakis]

I'm of the opinion that we need an RFC before this can be stabilized. This doesn't have to block the PR from landing, so long as the work is feature gated, but I'd like to fit this work into an active project group that can draft up the RFC and shepherd the feature along.

I think my ideal end-point would be that we are able to change the behavior so that macro_rules! behaves exactly like other items with the exception of those that are tagged as #[macro_export]. I'm not sure how hard that would be though, I imagine that the transition pathway for a macro-rules without any visibility specified would be to convert the macro invocation into something like super::macro!(...) for cases where it is not within the current module?

(This is precisely the work I think would be good to see figured out in the context of a project group with a known charter.)

[nikomatsakis]

Basically this feels like more than a "tweak" of an existing feature, but a major initiative -- one that would be quite welcome! -- to peel off a part of the "macros 2.0" work and make it work universally (specifically, path-based imports).

[Mark-Simulacrum]

The language team discussed this during triage last week.

We had a question for @petrochenkov -- how much work would it be to gate the behavior change here on an edition? And, relatedly, would migration tooling to move users over to the new edition (for example, by importing the macros in submodules and switching to pub(crate) over macro_export attributes on modules) be feasible / something you'd be willing to invest time in implementing/mentoring an implementation of?

It was generally the feeling that landing the change such that adding a visibility of any kind to a macro switches you to the new system, but a visibility-free macro is in the lexical scoping system (rather than module private) would be pretty confusing and likely not what we'd want.

If we are to stabilize this, I believe that the lang team wanted an RFC laying out the plan for moving from lexical scoped macro_rules to the regular scoping of Rust items, including to what extent that would need the edition pieces above. I would be interested in helping to work on this, FWIW. To land this PR a chartered project group would likely be sufficient. There is some discussion on Zulip laying out some more details as well.

[petrochenkov]

how much work would it be to gate the behavior change here on an edition?

Making this change or similar changes edition-specific should be nearly trivial.

And, relatedly, would migration tooling to move users over to the new edition (for example, by importing the macros in submodules and switching to pub(crate) over macro_export attributes on modules) be feasible / something you'd be willing to invest time in implementing/mentoring an implementation of?

Migration lints are feasible (for all items listed in #78166 (comment)), but they may have higher failure rate than those lints that we prepared for 2018 edition, because the migration is non-trivial in general case.
I wouldn't want to implement the lints myself, but I can review others' PRs and answer questions if necessary.

[petrochenkov]

To lang team: this PR has lower priority than #79078.

[petrochenkov]

Current lang team discussion about this issue - https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/pub.20macro.20rules
It proposes a different plan from this PR, so closing.

[joshtriplett]

@petrochenkov The intention was to write an RFC to document more-or-less what this PR was proposing to do, not to diverge substantially. The hope was to revise and accept the RFC, and then accept this PR with very little change expected (possibly just appropriate edition handling).

[petrochenkov]

@joshtriplett
This may need some additional changes to rustc_metadata to work with cross-crate use of such macros correctly.
I'll reopen the PR and return to this work once I have time.

[spastorino]

@petrochenkov we were discussing with @nikomatsakis and @rylev to land this PR with the feature flag included so we can start testing, and working on edition handling, migration, etc.
I see you mentioned cross crate testing and that this won't work? did you already implement the thing? I may be able to take care of this if you want? I'm not sure exactly how to implement this but I'm going to try to figure out, meanwhile any pointer would be appreciated.

[nikomatsakis]

My thought is that we should land this PR as a first step-- I think @spastorino plans to rebase.