Tracking issue for "macro naming and modularisation" (RFC #1561)

[nikomatsakis]

Tracking issue for rust-lang/rfcs#1561.

Roadmap: #35896 (comment).

cc @nrc @jseyfried

[ahicks92]

One question I don't think was ever answered on the RFC thread is if providing a way for current macros to opt in is feasible.

Just throwing this out there as to make sure it doesn't get forgotten. My vote is obviously for yes we can, but I'm also obviously not on a team, or even an active contributor beyond these discussions.

[nrc]

@camlorn this was actually address in my final edit to the RFC before merging. To summarise, it might be possible but we need implementation experience to be sure and to work out exactly how it might work. So, basically punting on the issue for now. It is certainly an area where I think we need to tread extremely carefully.

[jseyfried]

c.f. #37732 (comment)

[jseyfried]

Tasks

• Stackless macro expansion (PR macros: stackless expansion #36214).
• Fix $crate with inter-crate re-exports (PR macros: improve reexports #37463).
• Future-proof #[no_link] crates (PR macros: Future proof #[no_link] #37247).
• Parse paths in bang macro invocations (e.g. path::to::mac!();) (PR parser: support paths in bang macro invocations (e.g. path::to::macro!()) #36662).
• Back-port shadowing restrictions from Changes to name resolution rfcs#1560 where appropriate (PR Enforce the shadowing restrictions from RFC 1560 for today's macros #36767).
• Forbid user-defined macros named "macro_rules" (PR Forbid user-defined macros named "macro_rules" #36730).
• Miscellaneous groundwork in syntax, resolve, and metadata (PRs Assign node ids during macro expansion #36438, Load extern crates in resolve #36525, resolve: groundwork for building the module graph during expansion #36573, Assign def ids and build the module graph during expansion #36601, Load macros from #[macro_use] extern crates in resolve #36616, macros: persistent scopes #37036, macros: improve $crate #37213, Process #[macro_use] imports in resolve and clean up macro loading #37292, and Support #[macro_reexport]ing custom derives #37542).
• Land #![feature(use_extern_macro)] (PRs Support useing externally defined macros behind #![feature(use_extern_macros)] #37732 and macros: support invocation paths (e.g. foo::bar!()) behind #![feature(use_extern_macros)] #38082).
• Allow use to shadow macros from the prelude, currently is an ambiguity error (PR Allow use macro imports to shadow global macros #40501).
• Improve interaction with macros 1.0 exports in the same crate, i.e. emit more duplicate errors (PR Forbid conflicts between macros 1.0 exports and macros 2.0 exports #40509).
• Integrate with rustdoc, issue #[doc(hidden)] does not work for externally used macros #39436 (PR Rustdoc: memoize pub use-reexported macros so they don't appear twice in docs #40814).

[durka]

Another problem with local_inner_macros: #52726

I guess since cargo-fix can't assume anything beyond the current crate, the only things it could do that would actually work are (a) nothing, or (b) somehow crawl the macro expansion and see which macros you really need to import.

When cargo-fixing a macro-exporting crate, there's a question of whether to suggest A->B or A->C.

[withoutboats]

It sort of depends what you mean by compatible. It is possible to make a crate "not compatible with 2015" in the sense that removing edition = "2018" in its Cargo.toml would make it not compile anymore. But it is still compatible in the sense that other crates in the dependency graph can be on a different edition.

Yes, I would say it this way: a library itself uses only one edition, but is always compatible with all editions - that is, you can depend on it whatever edition you are in. When we say that multiple editions can be compiled together, I think we are not expressing this compatibility forcefully enough: it is not possible to write a library that can only be depended on by crates on one edition or another. Library authors don't even have to worry about compatibility with multiple editions; they simply are compatible always.

(Also unless somethings changed recently (and I don't think so, based on @dtolnay's post), the only thing editions change related to this issue are turning on some lints. Macro imports don't behave differently between 2015 and 2018 edition.)

[withoutboats]

@dtolnay I want to clarify one thing based on your chart: you suggest that the "1" column options (using #[macro_use]) will be linted against in 2018. However, based on @aturon's most recent comment, these lints will be allow by default until the ecosystem transitions more.

@Manishearth makes this comment that I don't understand:

But as it stands even if they're not on by default, we are asking people to use them post-transition, so we should either stop asking people to do so or ensure they're polished by the transition.

I don't know what it means to "ask people to use them;" I haven't heard about this plan and it seems dubious. If we think people should use these warnings, we should have them turned on. As a rule, I do not believe we should ever have allow by default lints we recommend that you turn on; this is use strict; and to me its a sign of something gone quite wrong.

From my perspective, it seems like we should evaluate each 2018 idiom lint for disruptiveness and turn it on as soon as it seems like a net benefit. This will probably leave the macro related ones off for some time until the ecosystem has moved off the "A" system onto the "B" and "C" systems. Its unfortunate, but its the long term cost we're paying for having stabilized the "A" system for 1.0.

[Manishearth]

As a rule, I do not believe we should ever have allow by default lints we recommend that you turn on;

It's not quite this, it's not "use strict";. We recommend you temporarily turn them on, like we do for the migration lints.

The original plan was that we recommend a two step edition migration process. In the first step you turn on the migration lints and fix those, then you upgrade the edition in Cargo, and do the same again with the idiom lints. Ideally, this would be managed by cargo fix. The idiom lints aren't ones you keep enabled in perpetuity; you flip them on when you use cargo fix (or cargo fix flips them on for you), and when you're done with the upgrade you flip them off. We can make them on by default on 2018 a couple months into the edition if we wish, with the hope that everyone has cargo fixed them already.

The reason they're not just on by default is because these lints are super noisy and really need to be run with cargo fix.

This is all being discussed in #52679

Given that many of the idiom lints are rather broken the current plan may just be to not recommend this for a while, and have a gradual rollout.

[withoutboats]

@Manishearth thanks for the clarification! I think really when you say we recommend that you turn them on, the issue is what behavior cargo fix demonstrates in light of this: the lint is really just a part of the API between rustc and cargo here, mostly an implementation detail (ideally, cargo fix flips the lint on and off for you, so that cargo fix can be a single atomic step).

I'll reply more about the general problem on the issue you linked.

So I think the open issue here can be scoped down to this: what will cargo fix do about macro imports, given that the upstream crate needs to be compatible first? And how?

For macro authors

If cargo fix can convert macro authors to the C scheme ($crate::) for their macros, that seems ideal. There's no reason I see to convert them to B since by depending on 2018, they inherently the require the 1.30+ compiler.

For macro users

Assuming cargo fix can upgrade a user from 1 (#[macro_use]) to 2 (normal imports), I think there are three reasonable options:

1. Be pessimistic: we don't try to move users from #[macro_use] to normal imports, because we assume too many of the upstream crates won't have upgraded yet.
2. Be optimistic: we do try to make the transition for all crates, breaking the user's code if the upstream crate has not transitioned.
3. Be discerning: we use some method (a whitelist, a heuristic?) to upgrade the user to 2018.

Its possible that our behavior should change over time.

It also occurs to me that if we do either 2 or 3, the user is likely to have a lock file that locks them to a version of the package that is incompatible. If we do any of these fixes, we should probably cargo update the macro-exporting package as a part of cargo fix so that users will be more likely to get the fixed code.

[Nemo157]

It also occurs to me that if we do either 2 or 3, the user is likely to have a lock file that locks them to a version of the package that is incompatible. If we do any of these fixes, we should probably cargo update the macro-exporting package as a part of cargo fix so that users will be more likely to get the fixed code.

Since I was just reading about --minimal-versions, this should maybe be a cargo upgrade as otherwise the users Cargo.toml will be claiming to work with a version that it might not actually work with.

[durka]

Probably a bad idea, but we could add another hack: #[macro_export(trust_me_its_2018_compatible)], for macro authors to use once they've updated to B or C. Seeing this attribute, cargo fix could recommend C2.

What I still don't get is the business about "foundational crates". Sure, crates that are managed by the core team are special and we trust y'all, but how is Joe the Macro Author supposed to know whether their crate is special enough to ignore the cargo fix advice? Should there be an official guideline, like, "you can apply this fix when you are ready to drop compatibility with rustc v1.XY"?

[withoutboats]

@durka Since the version needed to compile a project with rust = "2018" in the Cargo.toml is greater than or equal to the version needed to support option C, anyone running cargo fix --prepare-for 2018 has opted into dropping compatibility with rustc versions that don't support option C. For now, each crate author decides if they want to go onto 2018 or if they want to continue to support rustc versions from before the 2018 release.

[durka]

But lazy_static will be threading the needle by upgrading to B.

[Nemo157]

Be discerning: we use some method (a whitelist, a heuristic?) to upgrade the user to 2018.

Since options B2 and C2 work the only thing you have to watch out for is a dependency still using A. I assume detecting that a dependency is using option B is possible since that presumably is reflected in the metadata, so that should be easy to upgrade. Option C is likely undecidable, but the heuristic could just be "if dependency is Rust 2018 then they should be using option C" and upgrade the user (will break if a crate has updated to Rust 2018 but not transitioned to option C)?

[Manishearth]

@withoutboats this plan seems pretty good! I think changing behavior over time is definitely something we should also try for; be pessimistic at first for macro users , and over time start suggesting fixes. There's also a lot of interesting stuff here that can be done by teaching cargo fix about good and bad macro crate versions.

[withoutboats]

Building on @Nemo157's comment, if we can get this metadata for each of your dependencies:

1. Are any of their macros tagged local_inner_macros?
2. Are they on the 2018 edition?

That could be a good heuristic for whether or not we should upgrade their macro invokations. But I'm not sure how well the current set up allows rustfix to operate differently depending on the dependency metadata like this.

We could consider disallowing option C on 2015, so that anyone who wants to stay on 2015 will switch to B, which we can detect, and anyone who wants to switch to C will move to 2018, which we can also detect.

[Manishearth]

Well, we have crater, and we can use that to obtain this metadata ourselves and hardcode it (and hardcode what versions fix the problem)

I don't expect there to be that many macro crates affected, especially if we only consider popular ones. The problem is that people depend on these.

[glmdgrielson]

Wait, why is this still open? Everything on the roadmap is checked off. What's blocking this other than working out the kinks of a potential edition lint?

[Manishearth]

Tracking issues close when stabilized.

[ghost]

I have a question. Why do we need to import macros to use them? For example:

extern crate crossbeam;
use crossbeam::channel;

// Doesn't compile unless we uncomment this line:
// use self::channel::select;

fn main() {
    channel::select! {
        default => {}
    }
}

It is surprising to me that this code doesn't compile unless we import the macro. Is this intentional behavior or a bug?

[dtolnay]

@stjepang maybe channel::select! expands to an invocation of plain select!? That select! invocation would be the one that is not finding a resolution in scope.

The expansion should use $crate::channel::select! instead (or macro_export(local_inner_macros) if you need to support compilers older than 1.30.0).

[ghost]

@dtolnay Oh right, that was indeed the issue. Thank you! :)

[dhardy]

Could I please request some transparency on what exactly is implemented by this "tracking issue" and what continued plans there are related to RFC 1561?

My understanding of the process is that a tracking issue is not a place for discussion of how new features work (other than internal details), however I see a lot of discussion here of what exactly the new macro modularisation rules are. Perhaps part of the problem is that RFC 1561 is vague and far too broad.

For example, 1561 declares the following which does not appear to be covered here (and does not appear to be possible on the latest nightly under either edition):

If a macro baz (by example or procedural) is defined in a module bar which is nested in foo, then it may be used anywhere in the crate using an absolute path: ::foo::bar::baz!(...). It can be used via relative paths in the usual way, e.g., inside foo as bar::baz!().

All documentation I can find is either hopelessly out of date or refers back to this issue.

[alexcrichton]

@dhardy unfortunately the transparency here is all written down, but it takes some effort to sift through it. What's stabilized here is described online and further tracking issues track remaining work items for known unstable items in the compiler. Work that hasn't ever been implemented from the original RFC doesn't currently have tracking issues, but they can definitely be created!