rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.62k stars 12.48k forks source link

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

Closed nikomatsakis closed 6 years ago

nikomatsakis commented 8 years ago

Tracking issue for https://github.com/rust-lang/rfcs/pull/1561.

Roadmap: https://github.com/rust-lang/rust/issues/35896#issuecomment-277870744.

cc @nrc @jseyfried

ahicks92 commented 8 years ago

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 commented 8 years ago

@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 commented 7 years ago

c.f. https://github.com/rust-lang/rust/pull/37732#issue-188905131

jseyfried commented 7 years ago

Tasks

withoutboats commented 7 years ago

@jseyfried What's the status of this? Seems like the two items without checks have had their PRs merged. AFAIK the RFC was for this to be in prep for the next iteration of declarative macros, but some of the PRs seem like we could enable it for macro_rules! macros. Is that so? What's the migration story (potential breakages?) etc?

brson commented 7 years ago

@jseyfried @nikomatsakis I'm also interested in the status of this. Can it be activated for 1.19?

nikomatsakis commented 7 years ago

I don't have a good feeling for current status. @jseyfried?

jseyfried commented 7 years ago

@withoutboats

some of the PRs seem like we could enable it for macro_rules! macros. Is that so?

We can enable it for #[macro_export] macro_rules! in extern crates (and derive procedural macros in extern crates) but not crate-local macro_rules! (since crate-local macro_rules!'s scopes are too different from items' scopes).

This works today with #![feature(use_extern_macros)], which is implied by #![feature(proc_macro)] and #![feature(decl_macro)] (the latter has not yet landed).

@brson @nikomatsakis IIUC, we reached consensus on the current behavior of #![feature(use_extern_macros)] in the February design sprint.

That being said, supporting use for macros from extern crates but not for crate-local macro_rules! might be confusing for end users, especially since there's no way to define a crate-local macro that can be used until declarative macros 2.0 is stable.

I don't have a strong opinion on whether we should stabilize this now, wait for more experience with declarative macros 2.0 and then stabilize, or only stabilize when we stabilize declarative macros 2.0.

withoutboats commented 7 years ago

We can enable it for #[macro_export] macro_rules! in extern crates (and derive procedural macros in extern crates) but not crate-local macro_rules! (since crate-local macro_rules!'s scopes are too different from items' scopes).

Is it impossible to make crate local macro_rules! macros work with this? Probably we'd need some kind of syntactic change to avoid breakages? (Maybe just a visibility modifier?)

I think declarative macros 2.0 is pretty far off, because there are going to be many design decisions we'll be able to reconsider with the new syntax, and that's going to take time to work through. It'd be nice to support this feature sooner than that.

jseyfried commented 7 years ago

Is it impossible to make crate local macro_rules! macros work with this? Probably we'd need some kind of syntactic change to avoid breakages? (Maybe just a visibility modifier?)

It is possible with a visibility modifier (e.g. pub macro_rules! m { () => {} } could have item scope), but then

withoutboats commented 7 years ago

I'm personally not concerned about making macro_rules "good enough" - I think there will be enough improvements over the current system in macro macros, and I'm not against just straight up issuing deprecation warnings if you write a macro_rules macro someday. And I'm also alright with the special case.

However, I want the story about when macros use normal namespacing and when they don't to be easy to understand. I think a solution could be to introduce a new attribute which turns this scoping on, and require it (for both local and macro_export'd macros).

Later on we could introduce an attribute to turn old-school macro scoping on, and if we do the epoch shift, we could change which is the default.

jseyfried commented 7 years ago

@withoutboats Yeah, that makes sense, but I'm not sure it's worth the complexity of another dimension due to making namespacing orthogonal to macros 1.0 vs macros 2.0.

nrc commented 7 years ago

In general I'm opposed to blurring the lines between current macros and new macros. The 'good enough' argument is part of this - I fear that at some point macros 1.0 gets good enough that there is push back against further changes (I don't believe we can simply deprecate if the community doesn't want it). To some extent I think the macros 2.0 changes are a mixture of sugar and medicine - if we add all the nice changes to macros 1.0 (naming, syntax) then it makes it harder to sell the necessary changes which are left (e.g., hygiene).

A bigger worry for me is confusion for users - there is already confusion between old decl macros, new decl macros, procedural macros, etc. I worry that adding more layers to this - old macros with feature X, old macros with feature Y will make this even more confusing. On a technical level, supporting multiple versions of macros with different features opens the gates to many more bugs in the combinations of features which might be less tested.

brson commented 7 years ago

I don't understand all the parts at play here, but I want to be able to reexpert macro_rules macros from other crates in stdx.

withoutboats commented 7 years ago

@brson The conclusion of our lang team discussion was that we probably aren't going to migrate to this until macros 2.0.

est31 commented 6 years ago

So right now using the macros of a different crate works by doing:

#[macro_use] extern crate foo;

macro_from_foo!(...);

However, with RFC 2126, extern crate is being brought onto the path of deprecation, meaning that there will be lints for it and maybe even hard errors in a future epoch.

Now the epochs RFC has made one thing very clear: inter-epoch interop is always guaranteed, basically allowing a newer crate crate to use the older crate's functionality. For macros I've heard assurances that there will be "epoch hygiene" or something.

But how will this apply to #[macro_use]? I see the following options:

  1. Never lint/error for extern crate foo if there is a #[macro_use] next to it. I don't think this is a good idea however, as it would be the legacy syntax that one wants to get rid of, and it would look weird to have a bunch of #[macro_use] extern crate foo; statements inside your lib.rs
  2. Implement the macro naming and modularisation system for macros 1.0 as well.
  3. Implement some other system, like #[macro_use] use cratename::module; or macro_use! {cratename::macro_name}.

Regarding points 2 and 3, @withoutboats has expressed that there is a desire to make idiomatic code of the future epoch legal in the current epoch, so we need to do those backards compatibly inside the current epoch.

cc @nrc @aturon

jseyfried commented 6 years ago

@est31

  1. Implement the macro naming and modularisation system for macros 1.0 as well.

This is already implemented across crate boundaries. #[macro_export] macros from an upstream crate appear in the upstream crate's root. For example,

// crate foo
#[macro_export]
macro_rules! macro_from_foo { () => {} }

// crate bar
#![feature(use_extern_macros)]
// ^ implied by `#![feature(proc_macro)]`, `#![feature(decl_macro)]`

extern crate foo; // no #[macro_use]
use foo::macro_from_foo; // `pub use` also works, subsumes `#[macro_reexport]`
macro_from_foo!();

In other words, generally speaking you can replace

#[macro_use]
extern crate foo;

with

#![feature(use_extern_macros)]
extern crate foo;
use foo::{macro_rules_macro_1, macro_rules_macro_2, ...};
bluss commented 6 years ago

We talked about this somewhere before @jseyfried, but I'll remind us again that macro reexporting is stable when doing it with a glob use. Crate frunk already relies on it.

antoyo commented 6 years ago

I don't know if it is the right place to mention this, but here's what I would like to achieve: I'd like to have the possibility to put macro definitions in an impl to be able to do Struct::macro!(). My use case (in my crate tql) for that is to generate macro in a custom derive (i.e. SqlTable) and use these generated macros from another macro (sql!()). Without this feature, that would require the users to use macros that are not in code they wrote. Is it planned to add the ability to declare macros in an impl block or is there any alternative for this use case? Thank you.

jseyfried commented 6 years ago

@antoyo

I'd like to have the possibility to put macro definitions in an impl to be able to do Struct::macro!().

Implementing this would require dramatically restructuring the compiler, and isn't planned in the foreseeable future. Also, I believe it is a unresolved question if we can do sound type checking while macros are still being expanded (e.g. issues with coherence checks).

My use case (in my crate tql) for that is to generate macro in a custom derive (i.e. SqlTable) and use these generated macros from another macro (sql!()).

This is why we have hygiene :)

Without this feature, that would require the users to use macros that are not in code they wrote.

The point of hygiene is that names at the macro definition resolve at the macro definition, independently of where the macro is invoked. Similarly, names passed in to the macro as arguments resolve at the call site, e.g. they can't be accidentally shadowed by a name at the macro definition. For example,

#![feature(decl_macro)]

mod foo {
    pub fn f() {} // (1)

    pub macro m($arg:expr) {
        f(); // This resolves to (1)
        mod bar {
            fn f() { $arg }
        }
    }
}

fn main() {
    fn f() {} // (2) (note -- no conflict error with (1))
    foo::m!(f()); // The `f` argument resolves to (2) even though $arg is in a weird place
}

Roughly speaking, hygiene causes a macro m($arg:expr) { ... } to resolve like a corresponding fn m(arg: T) { ... } would.

This also applies to procedural macros. For example,

#[proc_macro_derive(Foo)]
fn foo(_input: TokenStream) -> TokenStream {
    quote! {
        // Due to hygiene, these names never cause conflict errors.
        extern crate sql_macros;
        use sql_macros::sql;

        ... sql!(Struct) ...
    }
}

Annoyingly, we can't declare the extern crate sql_macros; use sql_macros::sql; in the proc-macro crate, since items in the proc-macro crate are compiled for the host platform. The target platform can only see the resulting procedural macros, not the underlying functions (vice versa for the host).

Once the Cargo.toml for procedural macros crates supports declaring target dependencies in addition to today's host dependencies, the target dependencies will automatically be in scope inside quote!.

alexcrichton commented 6 years ago

This was discussed in the recent work week with an eye towards how we might be able to stabilize slices of macros 2.0 in the Rust 2018 edition release. When we discussed this in a group it was concluded that as far as we knew this feature was working as intended.

While there are known "oddities" with respect to how macro_rules! works locally within a crate it was decided that this is an important enough feature that it should be ok to stabilize with such behavior.

@rust-lang/lang, could I convince one of y'all to enter into FCP on this issue? In terms of timeline I'd like to ensure that a chunk of macros 2.0 (not all of it) stabilizes all at once, so if this passes through FCP and the other pieces fall through then I think we won't stabilize this issue, but if other pieces come through as well I think we'll stabilize this.

withoutboats commented 6 years ago

@rfcbot fcp merge

Let me clarify first what feature we're stabilizing, as I understand it:

extern crate serde_derive as serde;

#[derive(serde::Serialize)]
struct Foo { }
extern crate serde_derive;

use serde_derive::Serialize;

#[derive(Serialize)]
struct Foo { }
extern crate foo;

use foo::bar;

bar! { ... }

As a result of this, the #[macro_use] system can become a legacy syntax, replaced by using normal path imports for macros from other crates. This makes macros from other crates act more like any other item, and it is useful for making extern crate a legacy syntax as well.

rfcbot commented 6 years ago

Team member @withoutboats has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

alexcrichton commented 6 years ago

@withoutboats indeed! That is my understanding as to what we're stabilizing as well :)

cramertj commented 6 years ago

@withoutboats

macro_rules! macros from other crates with the #[macro_export] attribute can be imported using normal imports. I do not know if they are at the location they are declared at or in the root of the crate (someone clarify).

They show up at top level of the crate:

// crate foo
extern crate bar;
use bar::baz; // works
use bar::bazmod::baz; // errors
fn main() { baz!() }

// crate bar
pub mod bazmod {
    #[macro_export]
    macro_rules! baz { () => { println!("hello world!"); } }
}
withoutboats commented 6 years ago

Sounds good, that's what I expected since its where they show up in docs.

golddranks commented 6 years ago

I tried this feature now, generally it's very nice with simple macros, but it seems to be unusable with macros that expand to another macros, because one needs to manually import also those, even thought they can be considered as implementation details. So macros don't at the moment, "lexically close" references to another macros, right? I.e. they expand unhygienically. Is this an expected thing?

Do macros that use the new use syntax "close over" the macros they expand into?

SimonSapin commented 6 years ago

Yes, macro_rules! is known to be unhygienic. Fixing this is one of the goals of declarative macros 2.0 https://github.com/rust-lang/rust/issues/39412

est31 commented 6 years ago

As extern crate will be removed from the language in the 2018 epoch (I think it will??) we need a replacement for #[macro_use]. Stabilizing this is one way to do this. :+1:

golddranks commented 6 years ago

So is the following true: macro_rules! expand always without hygiene, so it would be possible to have foo! { "yahoo" } that expands into bar! { "yahoo" }, where bar is not imported, or depended by the f crate at all, so that what the bar! expands into is decided at the call site, depending on what the caller has imported?

I was thinking if it would have been possible to backward-compatibly enable a limited version of "macro import hygiene" in the cases where the new use syntax is used. However, if the expansion is done completely in the context of the caller, it seems hard, because the macro authors may have written their macros with the expectation that their macro expands to some another macro that is not even in scope at the definition site, so it can't be "closed over".

golddranks commented 6 years ago

However, in the vast majority of the cases, macros that are implementation details are defined in the same crate as the user-facing macros, so they are in scope at definition site AND at call site. The use syntax causes them no longer be in scope at call site, which is the whole point of the feature, but it would improve ergonomics if there were a hygienic "closed over reference" fallback for the old-style macros imported in the new way in case the macros they expand into are not in scope at call site. That would enable people to actually use the new import syntax.

alexcrichton commented 6 years ago

@golddranks unhygienic expansion is a feature of macros 1.0 at this point, and so this feature, for macros 1.0, would basically be inheriting that

golddranks commented 6 years ago

Thanks; I see. So, likewise, the expansion failing in absence of the macros in the callsite scope can considered a feature, as odd as it sounds?

alexcrichton commented 6 years ago

Indeed yes, it's not a feature we'd choose to have but it was a tradeoff for 1.0 stabilization

rfcbot commented 6 years ago

:bell: This is now entering its final comment period, as per the review above. :bell:

glmdgrielson commented 6 years ago

How do macro! and macro_rules! interact? I fear that since there's already macro_rules! everywhere, this won't get anywhere unless either:

  1. We come out with the shiny new Rust 2.0! (wait...) This is the Python approach where we'd have to maintain two separate but related languages.
  2. We keep both and push for the new one. This is the JS approach where the headache is still there, we have to figure out how they should interact and, worse, there may be some who just don't migrate for whatever reason. All I ask is which one are we going with? Because simply punting it and saying "Eh, I'll get back to on that one." won't work when it's actually implemented.

(P.S. Where is the rfcbot icon from? That looks really cool!)

SimonSapin commented 6 years ago

We’re not doing Rust 2.0, macro_rules! is here to stay. It’s ok if a lot of existing code doesn’t migrate. The point is providing a better alternative for new code (or code the has someone interested in refactoring/rewriting it).

ExpHP commented 6 years ago

@golddranks' issue sounds to me like a good case for not being able to use macro_rules! macros, as it puts them in an entirely new context that the author could not have anticipated. (but the new proc_macros are okay, since they have to be referred to by path, and the author can clearly see from day one that their generated invocations of other macros defined in the same crate do not work without adjustment)

Edit: Turns out #[macro_use(foo)] is a thing, and therefore this problem is already faced by macro_rules macros today.

petrochenkov commented 6 years ago

So, how macro_export works. If we have two crates - library and main, then

What I think we should do before stabilizing use of legacy macros from other crates:

petrochenkov commented 6 years ago

Stabilization of use_extern_macros will also collaterally stabilize paths with >1 segment in attributes (#[a::b::c]). Resolution for such paths is total mess and the rules may change with https://github.com/rust-lang/rust/issues/44690, so I'd prefer to keep them unstable for now (the workaround is to use use a::b::c; #[c]).

alexcrichton commented 6 years ago

Thanks for taking a look @petrochenkov! All of removing macro_reexport, probhibiting duplicate macro_export, and only allowing one-segment paths in attributes sounds great to me.

withoutboats commented 6 years ago

Sounds good to me too. Hopefully we can get multi-segment paths in attributes working soon, but I don't think we should either block this on that or stabilize it before its ready.

rfcbot commented 6 years ago

The final comment period is now complete.

alexcrichton commented 6 years ago

Ok great! I'm going to hold off on any actual stabilization here until the rest of Macros 1.2 is finished in FCP for stable.

spearman commented 6 years ago

Can #[doc(hidden)] be made to work for macros reexported as in https://github.com/rust-lang/rust/pull/37732 ? or is this a limitation of current macros and rustdoc?

petrochenkov commented 6 years ago

@spearman That's a bug, I've seen it while removing macro_reexport but haven't reported until now - https://github.com/rust-lang/rust/issues/50647.

petrochenkov commented 6 years ago

Import regression with use_extern_macros: https://github.com/rust-lang/rust/issues/50725. Needs to be fixed before it's enabled by default.

spearman commented 6 years ago

Will #[macro_use] eventually be phased out? Trying to figure out a sane way to re-export macros that are required for using a public macro. I suppose the idea is to eventually use paths like $crate::path::to::reexported_macro! so that a user never has to explicitly import any reexported macros, but if those macros don't themselves use $crate paths in macro invocations, then futher macros need to be imported at the root level, which is where #[macro_use] comes in handy. The alternative would be to put the macro definition together with any reexports in to a single module so it can be glob imported. Again if I understand correctly, if all macros correctly use $crate paths to refer to reexports then this won't be necessary and importing a macro by itself is sufficient to use it.

jjpe commented 6 years ago

Perhaps it is a good idea to make such non-$crate imports within macro definitions to be illegal?

In other words: when using use in a macro definition, it would always start with the crate name as a path root.

That way the problem is avoided in the first place. And it's not a theoretical problem either: I've run into issues like this where it's only after trying to use the crate that I discovered that the macro was exported but unusable without extra imports.

alexcrichton commented 6 years ago

@spearman #[macro_use] on cross crate macro usages is being phased out in favor of use crate_name::macro with this issue, although within a crate you'll still use #[macro_use]

@jjpe perhaps yeah! Although something like that will likely require an RFC