Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Procedural macros 1.1 #1681

Merged
merged 8 commits into from Aug 22, 2016
Merged

Conversation

@alexcrichton
Copy link
Member

alexcrichton commented Jul 18, 2016

Extract a very small sliver of today's procedural macro system in the compiler,
just enough to get basic features like custom derive working, to have an
eventually stable API. Ensure that these features will not pose a maintenance
burden on the compiler but also don't try to provide enough features for the
"perfect macro system" at the same time. Overall, this should be considered an
incremental step towards an official "plugins 2.0".

Rendered

Extract a very small sliver of today's procedural macro system in the compiler,
just enough to get basic features like custom derive working, to have an
eventually stable API. Ensure that these features will not pose a maintenance
burden on the compiler but also don't try to provide enough features for the
"perfect macro system" at the same time. Overall, this should be considered an
incremental step towards an official "plugins 2.0".
@aturon
Copy link
Member

aturon commented Jul 18, 2016

Suggestion: this might want to be titled Plugins 1.1, as it doesn't really impact the "macro" system (if you think of that as macro_rules, which I think many do).

@aturon
Copy link
Member

aturon commented Jul 18, 2016

@eddyb
Copy link
Member

eddyb commented Jul 18, 2016

@aturon I believe this is because @nrc wanted to avoid "plugins" for "procedural macros".

@alexcrichton
Copy link
Member Author

alexcrichton commented Jul 18, 2016

@aturon yeah it's true that this only really affects what we call plugins today, but I believe that @nrc wanted to start avoiding the term "plugin" as everything will eventually be a macro in the "macros 2.0" world.

* Sharing an API of the entire compiler
* Frozen interface between the compiler and macros

### `librustc_macro`

This comment has been minimized.

@eddyb

eddyb Jul 18, 2016 Member

This name is unfortunate, couldn't we use libmacro?

This comment has been minimized.

@alexcrichton

alexcrichton Jul 18, 2016 Author Member

This is currently done because macro is a keyword, and this doesn't propose changing that just yet. I was thinking that we could perhaps wholesale rename the crate one day as well if macro becomes an available name.

This comment has been minimized.

@MovingtoMars

MovingtoMars Jul 19, 2016

How about libmacros? This follows the approach that Go takes, ie the strings package.


Like the rlib and dylib crate types, the `rustc-macro` crate
type is intended to be an intermediate product. What it *actually* produces is
not specified, but if a `-L` path is provided to it then the compiler will

This comment has been minimized.

@sfackler

sfackler Jul 18, 2016 Member

This feels a bit of an abuse of "link" if we switch plugins to IPC executables - maybe a separate flag would be a good idea?

This comment has been minimized.

@alexcrichton

alexcrichton Jul 18, 2016 Author Member

I believe the intention is that --extern works to load macros (it's what Cargo will pass), so this was just a natural extension of that. We could perhaps add something like -L macro=..., but at this point it doesn't seem strictly necessary as the compiler can always distinguish whether an artifact is a macro or an rlib.

This comment has been minimized.

@eddyb

eddyb Jul 18, 2016 Member

I believe -L for rustc means "library search path" not "link search path".

}
impl TokenStream {
pub fn from_source(cx: &mut Context,

This comment has been minimized.

@alexcrichton

alexcrichton Jul 18, 2016 Author Member

@nrc could you comment on the Context argument here? I believe both @eddyb and @cgswords think it may not be necessary, in which case the pointer could be dropped everywhere and these methods could just be fmt::Display and FromStr (standard traits)

This comment has been minimized.

@nrc

nrc Jul 18, 2016 Member

I agree it may not be necessary. To be clear you must have some handle to the compiler, let's call that MacroContext, then either you can pass that around in the various functions, or you can leave it in TLS. The former is less ergonomic, but the later might have problems with multi-threaded macros and is arguably not as good engineering practice. I'm undecided as to which way we should swing in terms of the cost/benefit, but it would make a big difference to any libmacro APIs (methods vs free functions + the argument itself).

This comment has been minimized.

@eddyb

eddyb Jul 18, 2016 Member

Keep in mind that if you do interning with integer indices you can't even implement fmt::Debug on Token without putting something in TLS, and there is no meaningful difference between passing &mut MacroContext around and having it in TLS except for needing to set up TLS in each thread, as opposed to passing the context to one child thread, if needed.

If any TLS is used (as is now needed to impl Debug for Token), then the threading problems are unavoidable and having the context usable cross-thread would let users write broken code.

This comment has been minimized.

@cgswords

cgswords Jul 18, 2016

In the discussion about parallel expansion, there needs to be a principled way to combine contexts / TLS: if a multi-threaded macro manages to manipualte its MacroContext in each of the threads (eg by interning things), combining these contexts in a principled way (eg unifying the interners and re-numbering some of the macro output) is basically the only way to place nice, anyway. To this end, it's TLS (or, more accurately, atomically-accessed globals) will end up the more ergonomic option.

Like the executable, staticlib, and cdylib crate types, the `rustc-macro` crate
type is intended to be a final product. What it *actually* produces is not
specified, but if a `-L` path is provided to it then the compiler will recognize
the output artifacts as a macro and it can be loaded for a program.

This comment has been minimized.

@eddyb

eddyb Jul 18, 2016 Member

Cargo's usage of --extern would also work.

@aturon
Copy link
Member

aturon commented Jul 18, 2016

Thanks @alexcrichton for the awesome writeup! We've been working for a while on some way enable usage of Serde and similar libraries on stable in the near term, without massive stabilization. For example, there's a code generation proposal (which sits entirely on the side from the macro/plugin system), as well as attempts to find a very small increment toward "macros 2.0". But this is the first proposal that can be implemented immediately, has a clear path toward speedy stabilization, and imposes virtually no maintenance burden. I'm incredibly excited to be so close to a solution for the biggest cause of nightly usage today.

One thing that wasn't totally obvious to me, given that the macros 2.0 plan is a bit out of cache: what, if any, parts of what you propose to stabilize here would end up being deprecated once macros 2.0 has fully landed?

(To be clear, I have zero problem stabilizing the small slice of functionality you've outlined here in the near term even if in the long term we plan to deprecate it.)

such as:

```rust
extern crate double;

This comment has been minimized.

@eddyb

eddyb Jul 18, 2016 Member

I would've expected at least #[macro_use] to be necessary.

This comment has been minimized.

@alexcrichton

alexcrichton Jul 18, 2016 Author Member

cc @nrc, I think that importing custom derive annotations was a somewhat unspecified extension of "macros 2.0" right now, so to me it seems somewhat up in the air about whether we'd require that or not.

I don't mind one way or another, but I'm curious what @nrc thinks as well.

Eventually of course custom-derive will be imported through the normal module system I believe, so this is purely just a temporary stopgap.

This comment has been minimized.

@eddyb

eddyb Jul 18, 2016 Member

The problem is that if you do anything by default now, you'll need to opt out to be able to use the normal module system.

This comment has been minimized.

@alexcrichton

alexcrichton Jul 18, 2016 Author Member

Aha, an excellent point! You've convinced me, I'll add it.

```

Plugin authors would have to ensure that this is not naively interpreted as
`Baz = 1 + 1 * 2` as this will cause incorrect results.

This comment has been minimized.

@eddyb

eddyb Jul 18, 2016 Member

Couldn't we add parenthesis around all such fragments at stringification time to avoid the misinterpretation?
Plugin authors can't do anything about this if all they have is conversion to String.

This comment has been minimized.

@alexcrichton

alexcrichton Jul 18, 2016 Author Member

Sure! This is just an implementation detail for the compiler though, right?

This comment has been minimized.

@eddyb

eddyb Jul 18, 2016 Member

It's visible to the plugins (and stable code can observe it with stringify! right now), so adding extra parens should be specified somewhere.

@nrc nrc changed the title RFC: Macros 1.1 RFC: Procedural macros 1.1 Jul 18, 2016
@alexcrichton
Copy link
Member Author

alexcrichton commented Jul 18, 2016

@aturon

what, if any, parts of what you propose to stabilize here would end up being deprecated once macros 2.0 has fully landed?

This is a good question! Of the pieces here, what I would imagine is:

  • If we decide to rename anything, we'd accumulate deprecations for old names.
  • The behavior of "go to a string, parse, then go back to a token stream" will likely be deprecated in favor of directly manipulating the token stream.
  • The definition form of custom #[derive] will likely change and the proposed form here would be deprecated. We likely eventually want one that's imported via use (somehow).

Other than that though I imagine these pieces to survive stabilization:

  • The concept of a "macro" crate type
  • The concept of a "macro crate" provided in the standard distribution, providing all details necessary to talk to the compiler.
  • The ability to inform Cargo of crates which are macro crates
  • Cargo's behavior of compiling macro crates for the host and passing them to the compiler for the next unit of compilation.
  • Transformers like #[derive] operating over token streams and allowing customization via attributes that are stripped during the expansion process
@strega-nil
Copy link

strega-nil commented Jul 18, 2016

First, I don't think this RFC has gone through enough design, and fast-tracking it just to get people off of nightly is not a good idea, IMO. However, I am in favor of eventually (soon-ish) getting this sliver.

Some of my specific issues:

First: librustc_macro is a terrible stable name! My vote's in for libmacro. It's not a rustc-specific thing, it's an all-of-Rust thing!

Second: Context seems like an unnecessary implementation detail, and, if they're used the same way as in the original proposal "Procedural macros should report errors, warnings, etc. via the MacroContext [Context]."... why. We already have an existing error reporting mechanism, with Results. This new way seems very un-Rustic.

Third: I don't like how macros are defined. I'd rather they be defined with something like macro instead, especially as we eventually are going to allow proc macros and normal stuff in the same library.

@eddyb
Copy link
Member

eddyb commented Jul 18, 2016

@ubsan I don't like Context either, but Result won't cut it, macros 2.0 will eventually want a full-blown diagnostics API, where you can emit multiple warnings and non-fatal errors.
If the necessary information is placed in TLS, the API would work without a Context type.

@sfackler
Copy link
Member

sfackler commented Jul 18, 2016

@ubsan Result is an all-or-nothing thing. In the compilation process, it is very common to be creating many diagnostics during one compilation pass. A compilation error is not necessarily fatal - you (within reason) want to continue as long as possible to avoid forcing the user to go through a recompilation cycle for every error when many could all be reported at once. In addition, warnings, notes, etc are certainly not fatal.

@arielb1
Copy link
Contributor

arielb1 commented Jul 18, 2016

I think librust_macro could be a fine name.

On the integration level: plugins imply some form of host ABI. Stable plugins mean a stable HOSTRUSTC. I think we can just leave HOSTRUSTC as an implementation-defined thing, but we must be careful not to change its definition on existing platforms.

@strega-nil
Copy link

strega-nil commented Jul 18, 2016

@sfackler makes sense. I still don't think Context is the best, though.

@sfackler
Copy link
Member

sfackler commented Jul 18, 2016

Do you have any suggestions for a better replacement?

@strega-nil
Copy link

strega-nil commented Jul 18, 2016

@sfackler No, but I think @eddyb does :P

@arielb1
Copy link
Contributor

arielb1 commented Jul 18, 2016

On naming: I see librust_macro as an ordinary plugin library, and rustc_macro_derive as an exposed proc macro that (unstable) compiler plugins should eventually be able to reproduce (BTW, what is the nomenclature wrt. proc macro vs. plugin vs. compiler extension?)

rustc_macro_derive is a bad name, but we still don't have modular macros. I think pseudo-modular #[WHATEVER_CRATE_NAME_WE_PICK_derive] would work.

As for the API: maybe have #[CRATE_derive_v0] for the Context-free version?

@sfackler
Copy link
Member

sfackler commented Jul 18, 2016

How does shoving Context into TLS hide implementation details?

@eddyb
Copy link
Member

eddyb commented Jul 18, 2016

@sfackler The TLS slot wouldn't be exposed, the APIs would just not take any context.

@sfackler
Copy link
Member

sfackler commented Jul 18, 2016

@ubsan's concern with Context seemed to be that it was an implementation detail, and I'm confused as to how moving all of the methods on Context to free functions hides any details of the implementation. It seems totally isomorphic in that regard.

@arielb1
Copy link
Contributor

arielb1 commented Jul 18, 2016

Yeah. A Context parameter is easy to ignore and hard to add later.

@nrc
Copy link
Member

nrc commented Aug 18, 2016

I'm in favour of this. I'm not sure how I feel about the namespacing issue, I'd like to see how this looks when implemented.

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Aug 22, 2016

Huzzah! The @rust-lang/lang team has decided to accept this RFC. We decided to ask @alexcrichton to move some of the detailed matters (e.g., prefixes, etc) into unresolved questions to be addressed during stabilization. Thanks everyone!

@nikomatsakis nikomatsakis merged commit ac22573 into rust-lang:master Aug 22, 2016
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Aug 22, 2016

Tracking issue: rust-lang/rust#35900

If you'd like to keep following the development of this feature, please subscribe to that issue, thanks! :)

nikomatsakis added a commit that referenced this pull request Aug 22, 2016
@llogiq
Copy link
Contributor

llogiq commented Aug 22, 2016

I still miss my comment about the inferior traceability of code through macros 1.1 as opposed to the current systems. Could we at least add it to the drawbacks section?

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Aug 22, 2016

@llogiq I pushed some text about spans

@llogiq
Copy link
Contributor

llogiq commented Aug 22, 2016

Ok then. I'll keep an eye on the implementation. 😉

untitaker added a commit to untitaker/rfcs that referenced this pull request Sep 11, 2016
nrc added a commit that referenced this pull request Sep 11, 2016
hauleth added a commit to rust-num/num that referenced this pull request Sep 18, 2016
Current solution follow syntax proposed in rust-lang/rfcs#1681.

Tracking issue rust-lang/rust#35900

Close #227
hauleth added a commit to rust-num/num that referenced this pull request Sep 18, 2016
Current solution follow syntax proposed in rust-lang/rfcs#1681.

Tracking issue rust-lang/rust#35900

Close #227
homu added a commit to rust-num/num that referenced this pull request Sep 30, 2016
phaazon pushed a commit to phaazon/rfcs that referenced this pull request Feb 10, 2017
phaazon pushed a commit to phaazon/rfcs that referenced this pull request Feb 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.