search

rust-lang / rust

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

Tracking issue for "Macros 1.1" (RFC #1681) #35900

Closed nikomatsakis closed 7 years ago

nikomatsakis commented 7 years ago

Tracking issue for rust-lang/rfcs#1681.

cc @alexcrichton

Stabilization TODO

Litmus tests:

Features:

Known bugs:

alexcrichton commented 7 years ago

I've updated the issue description with a checklist of what needs to be done. It's likely not exhaustive but it should get us 90% of the way there hopefully

alexcrichton commented 7 years ago

Ok, I'm going to look into this today and see how far I get.

alexcrichton commented 7 years ago

cc https://github.com/rust-lang/rust/pull/35957, an implementation

nrc commented 7 years ago

From #35957: we should bikeshed the name of the librustc_macro crate some more. In particular, this is intended to be a long-lived crate which will have essentials for all macro authors in it, so limiting to rustc_macro (which in my mind, at least) is just about the 1.1 idea seems bad. I'd previously wanted libmacro for this, but since macro is a reserved word (and we might want it as a keyword in the future) that is impossible. @cgswords and I have been using libproc_macro, and I think that is not a bad name, although I am not 100% happy with it.

eternaleye commented 7 years ago

@nrc: Okay, my immediate thoughts on naming:

aturon commented 7 years ago

@nrc It seems like an important aspect of this question is the naming of our various macro styles over all -- in particular, if we go with libproc_macro, we're making a hard commitment to the "procedural macro" terminology. I don't have a strong opinion here, but I'm not sure if we've openly explored the space of terminology.

To be clear, are you thinking that today's macro_rules would simply be "macros", i.e. the default thing we mean by macros, while you have to qualify "procedural macros"? That seems like a pretty reasonable plan to me. And in that world, I'd argue that libproc_macro is better than libmacros.

nrc commented 7 years ago

My thinking here is that all macros are "macros", and when we need to make a distinction based on the implementation (the usage should be exactly the same for all kinds) we use "procedural macros" vs "macros by example". I would like to banish "syntax extension" and "compiler plugin" entirely (and one day re-use the latter for actual plugins).

But, yes, I strongly want to get behind the "procedural macro" terminology.

aturon commented 7 years ago

@nrc Makes good sense to me! While "macros by example" is a bit unwieldy, it's also a very evocative/intuitive term. My one worry about "procedural macro" is that "procedure" is not a thing in Rust. I wonder if there's a way of making the connection more direct. "Function macro" isn't quite right, but maybe gives you a sense of what I mean?

nrc commented 7 years ago

Yeah, it's not quite perfect, but given that it is a well-known/used term outside of Rust I think it is better than coining our own term. "Programmatic macro" is possible, but I prefer "procedural".

eternaleye commented 7 years ago

Perl's term for the closest equivalent it has to "procedural macros" is "source filters", which (especially with the shift from AST to tokens) is a pretty fitting description.

mystor commented 7 years ago

Perhaps 'Syntax transformers' or 'programmatic macros' would work well as names? I don't have a problem with procedural macros however.

jimmycuadra commented 7 years ago

"Procedural" is already what people call this, and I think it's clearly understood what it means, especially in contrast to "macros by example." I wouldn't worry about trying to find a different name.

nikomatsakis commented 7 years ago

I like the term "procedural macro" for regular use (or maybe "custom macro"). I particularly like using the word macro so that it's clear that they can (eventually...) be used in the same way as "regular macros". For this reason, I don't like "source filters" (I also expect a filter to just drop data, not transform it, though I know the term is used for both).

I am fine with either libproc_macro or libmacros. I sort of prefer the latter just because I don't love having _ in crate names when it can be readily avoided. =)

nikomatsakis commented 7 years ago

One question: do we ever expect to have "support routines" that could be used from non-procedural macros? I know of no such plans, but if we did, and we wanted them in the same crate, then libmacros would be a better name. =)

(I am thinking a bit about e.g., @dherman's comment from the eager-expansion RFC.)

eternaleye commented 7 years ago

@nikomatsakis: A related but subtly different question is a use case I brought up in https://github.com/rust-lang/rfcs/pull/1561#discussion_r60459479 - will we want procedural macros' implementation functions to be able to call other procedural macros' implementation functions?

I can easily see wanting to allow one custom derive to call another, and that would essentially make procedural macro definitions themselves capable of being used as such "support routines"

But yes, I think @dherman's gensym example is pretty compelling. Of course, if the answer to my question above is "yes", gensym is both a macro (which could be used by macros-by-example) and a support function (which could be used by procedural macros).

dtolnay commented 7 years ago

I have a cargo PR https://github.com/rust-lang/cargo/pull/3064 which should check all the "cargo integration" boxes in the checklist.

Ericson2314 commented 7 years ago

I left a comment on the cargo PR, but I think we want a different type of dependency, not just a different type of package. Firstly, I think this just is just better aestheticly and ergnomicly, but that's just my opinion. But I have two concrete reasons too.

dtolnay commented 7 years ago

Does serde work?

Yes https://github.com/serde-rs/serde/releases/tag/v0.8.6

(except for container attributes in some cases #36211)

alexcrichton commented 7 years ago

Awesome, thanks for the updates @dtolnay!

White-Oak commented 7 years ago

Are there docs on these macros? I suppose, the only example of using them is in serde, am I right?

Ericson2314 commented 7 years ago

OK cargo stuff has landed. That's fine, but it would be nice to revisit https://github.com/rust-lang/rust/issues/35900#issuecomment-243976887 sometime before stabilization. [For what its worth, I meant to bring this up in the original RFC but forgot.]

jessstrap commented 7 years ago

I think "macros by example" and "procedural macros" could be better catagorized as "declarative macros" and "imperative macros" respectively. This gives informative parallels to the more widely known declarative/imperative categorization of programming languages. As imperative is treated as a superset or synonym of procedural, it should be close enough for people used to "procedural macro" terminology to make the jump. It should also avoid any confusion with procedure/function/method concepts in rust itself. This naming scheme gives us a macro_imp crate and module to parallel macro_rules. macro_rules could eventually become a module of a more general macro_dec crate.

@nrc, When you refer to "actual plugins" are you including things like metacollect and clippy, things like rustw, rustfmt, and the Rust Language Server, or some other category of program?

SimonSapin commented 7 years ago

For what it’s worth, I mildly dislike the "by example" name (since $foo patterns aren’t "examples" in my mind). Declarative vs imperative sounds better to me.

sgrif commented 7 years ago

Playing around with this I've noticed one issue. It looks like Rust's provided derives sometimes add attributes which the compiler knows to ignore. This context gets lost when it goes through a custom derive. I'd expect the identify function given as a custom derive to be a no-op, but it will cause errors around the #[structural_match] attribute getting added.

Reproduction script (In a crate named `demo_plugin`) ``` rust #![feature(rustc_macro, rustc_macro_lib)] extern crate rustc_macro; use rustc_macro::TokenStream; #[rustc_macro_derive(Foo)] pub fn derive_foo(input: TokenStream) -> TokenStream { input } ``` (in another crate) ``` rust #![feature(rustc_macro)] #[macro_use] extern crate demo_plugin; #[derive(PartialEq, Eq, Foo)] struct Bar { a: i32, b: i32, } ``` Removing the `#[derive(Eq)]` causes everything to work fine.
alexcrichton commented 7 years ago

@sgrif ah yes indeed thanks for reminding me!

So there's a few things going on here:

So, some solutions we could do:

I'd be in favor of either not emitting these attributes or using a different mechanism. This is really tricky, though, because #[derive(Copy, Foo, Clone)] also needs to work (where custom code runs in the middle that could change definitions).

My preferred course of action would be to just not emit these attributes if we detect custom derive. Even that, though, may not be trivial. For now a convention of "custom first and standard last" should suffice, but I do think we should fix this before stabilizing.

colin-kiegel commented 7 years ago

Disclaimer: I only have an outside view of the compiler - so there is a lot I don't know. ^^

But from what I understand, the current approach of macros 1.1 custom derive is more like a temporary workaround. Temporary workaround might translate to "quite a while". But in the long run (=macros 2.0) we won't do the string-parsing-round-trip anymore, which currently leads to the loss of span-information. I wonder if these hidden attributes in the AST were such a bad thing in a macros 2.0 world. Maybe someone with more inside-knowledge about the compiler can tell. If these hidden attributes actually make sense in that future world, I would argue to go for the workaround by conventionally putting custom derives up front. The whole thing already is a workaround anyway, isn't it?

alexcrichton commented 7 years ago

@colin-kiegel I believe you're right in the sense that what we're doing right now is not future-proof. Let's say you have, for example:

#[derive(Eq, Foo, PartialEq)]

Today we'd add the Eq implementation, then run the custom code for Foo, and then add an implementation of PartialEq. The structure could change between Eq and PartialEq, so the #[structural_match] added by Eq may not actually be correct after Foo runs.

In that sense I agree that they're not necessarily future-proof in any case!

colin-kiegel commented 7 years ago

My feeling is, that custom derives which rely on structural changes generally won't compose very well, independently of those hidden attributes. Will a v2.0 custom derive be able to change the structure of the item, or will it be somehow limited to decorating only?

alexcrichton commented 7 years ago

Yeah the ability will always be there I believe (inherent in the TokenStream -> TokenStream interface) although I suspect any reasonable implementation of #[derive] would retain the structure of the original structure.

mystor commented 7 years ago

I don't suppose we could require that the output tokenstream not contain the struct itself, to make sure that the structure doesn't get changed? The input struct TokenStream would be an immutable prefix? The big problem would be making sure to ignore the unrecognized attributes which the plugins are using after the build is complete. Perhaps each #[derive()] can have a prefix (say #[derive(Foo)] has the prefix Foo_) which the attributes which they understand must start with, and after processing each custom derive, we strip off those attributes?

alexcrichton commented 7 years ago

@mystor yeah the problem with that approach is unrecognized attributes, which is why we have the entire struct as input. That's generally more flexible than relying on a particular prefix/suffix/registration/etc.

colin-kiegel commented 7 years ago

If a v2.0 custom derive could mark custom attributes as used, it could be limited to read-only access to the rest of the items token stream. This way better composability of custom derives could be guaranteed IMO. If a v2.0 macro needs to change the structure of an item it would have to ue another api, but not custom derive. This way the problem with #[structural_mach] and ordering of (custom) derives would only be present in macros 1.1. Would that make sense?

sgrif commented 7 years ago

Another issue. If a struct has two different custom derives on it and the second one panics, the error span will point to the first one, not the one which panicked.

Reproduction Script In a crate called `demo_plugin` ``` rust #![feature(rustc_macro, rustc_macro_lib)] extern crate rustc_macro; use rustc_macro::TokenStream; #[rustc_macro_derive(Foo)] pub fn derive_foo(input: TokenStream) -> TokenStream { input } #[rustc_macro_derive(Bar)] pub fn derive_bar(input: TokenStream) -> TokenStream { panic!("lolnope"); } ``` In another crate ``` rust #![feature(rustc_macro)] #[macro_use] extern crate demo_plugin; #[derive(Foo, Bar)] struct Baz { a: i32, b: i32, } ``` The error will highlight `Foo` even though `Bar` panicked.
alexcrichton commented 7 years ago

Thanks for the report @sgrif! I've updated the description of this issue and I hope to keep track of all outstanding issues related to macros 1.1 there as well.

nikomatsakis commented 7 years ago

Hmm, the interaction of custom derive with #[structural_eq] is something I hadn't thought of before. It rather bothers me!

It seems to me that having a way to "append" text to a token stream might be a better interface at the end of the day...it would preserve span information and avoid this problem, no?

nrc commented 7 years ago

One advantage of the more general interface is that it allows packages to have attributes on fields, which can be stripped when the macro runs. This is the only real use-case I know of for allowing custom derive macros to alter the original item.

I think that the interface question comes down to whether we want custom derive to be 'just another macro', in which case it seems important to have the same interface as other procedural macros (where we want to modify the original item). Or whether it should be its own thing with a special interface, in which case the more restrictive (append) interface makes sense.

I'll note that syntax extensions long had a distinction between modifiers and decorators, and I think that everyone involved really hated that distinction. I'm therefore a bit reluctant to go down the path of having custom derive be a bit special (an alternative that has been discussed is some kind of very special custom derive, possibly even a declarative format).

nikomatsakis commented 7 years ago

@nrc well, it can still be tokenstream -> tokenstream, where we just don't expose a new method that starts from scratch, right?

colin-kiegel commented 7 years ago

I agree that it must be possible to have custom attributes on fields for a custom derive to really make sense. But I believe there may be many ways this could be done with the "append"-style - this should of course be discussed. I'd definitely prefer the append style, because I prefer a world where derive macros don't change the item and are composable, plus it solves the #[structural_eq] issue. This is just right amount of freedom IMO.

If people didn't like that destinction I would like to ask why, because obviously there was enough reason to make this distinction before, wasn't it?

nikomatsakis commented 7 years ago

(I guess what I suggested is just a temporary hack, doesn't really address the longer term composability problem.)

tomaka commented 7 years ago

My various libraries currently have macros that look like foo!(Bar, parameters...), which generate a struct Bar from the parameters.

During some discussion on IRC, we had an idea to write #[derive(Foo)] #[params] struct Bar; instead, and replace foo! with a #[derive(Foo)] that would generate the body of the struct.

It's obviously not a strong argument, but I really liked that idea, as it's clearer for the user that a struct is being built.

nikomatsakis commented 7 years ago

I wonder if we could rework the #[structural_match] to be placed on the generated impl instead. Would probably be fairly easy, actually.

(Doesn't really solve the problem)

jimmycuadra commented 7 years ago

Worth noting that both Serde and Diesel make a lot of use of custom attributes on fields, so there is a definite need for custom derive to allow for replacement.

nikomatsakis commented 7 years ago

So actually maybe I am just not thinking straight about the #[structural_match] issue. After all, what can a custom derive do?

nikomatsakis commented 7 years ago

Sorry for writing tons of small comments and thinking aloud, but there is one other concern. Although a custom derive may not be able to "falsify" a #[structural_match] annotation (because it would wind up without the "magic span"), it would probably wind up screwing up the span of any existing annotation, unless the derives are applied in the correct order, which is unfortunate. Basically an instance of the non-composability that @colin-kiegel has been talking about, but without any attempt to modify the struct in flight.

(In other words, since we rely on the span to judge whether stable stuff can be used, losing span information can cause some tricky problems there.)

EDIT: OK, reading back I see that I just rederived what @sgrif already reported. Sorry again. ;)

mystor commented 7 years ago

It's also kinda gross, because it means we're exposing unstable implementation details to stable code. Ideally stable code would never even know that the #[structural_match] annotation exists.

nrc commented 7 years ago

@nikomatsakis well, one way or another, we need to enforce different constraints depending on whether the macro is intended to be a custom derive or some other kind. That means some separate treatment (and different semantics), whatever the signature of the function.

@colin-kiegel

I'd definitely prefer the append style, because I prefer a world where derive macros don't change the item and are composable, plus it solves the #[structural_eq] issue. This is just right amount of freedom IMO.

I think that mutating macros can be composable, although of course the terms of composability are different. There must clearly be a set of pre- and post-conditions on the operation of derives, either enforced by the compiler or by convention. Non-mutation seems like one extreme on the spectrum of invariants we might choose here, and note that already we are discussing ways in which that can be softened (e.g., marking attributes used). I think in general, we would prefer the simplest conditions and to have them enforced by the compiler. However, this is subsumed somwhat by the question of how specially custom derive should be treated.

If people didn't like that destinction I would like to ask why, because obviously there was enough reason to make this distinction before, wasn't it?

I don't believe there was strong motivation at the time. I think it was easy to implement and 'seemed like a good idea'. It has been disliked since because it makes the implementation more complex, it adds a distinction for macro authors which is usually irrelevant, and it made macros less flexible by having to choose whether to modify or decorate.

nrc commented 7 years ago

I would very much like to consider the long-term design of custom derive, and ensure we are heading in the right direction. It seems to me that the constraints of the 1.1 solution and the desire to do as much as possible as soon as possible are muddying the waters here and we are losing sight of the larger vision.

alexcrichton commented 7 years ago

I agree with @jimmycuadra in that it seems like supporting custom attributes in one way or another is a hard requirement. @nikomatsakis is also right though in that the current treatment of #[derive(PartialEq, Eq)] is subpar and we shouldn't stabilize it. Finally, @mystor has a very good point that custom derive modes shouldn't even know about this magical attribute. We're bound to want to add more in the future and I don't want macros 1.1 to prevent us from doing that.

Also echoing @nrc's sentiment about the long term design of custom derive, I think a lot of this boils down to how #[derive] actually works. If and when we support arbitrary attributes I think @nrc has a good point about only having modifiers and not having decorators, but #[derive] is pretty special where a custom derive is not defining a new attribute but rather just tacking on to an existing one.

Right now the implementation has a strict left-to-right expansion of #[derive] modes. All internal derive modes are expanded in a loop and whenever a custom derive mode is hit we reserialize, expand, then go back to stage 1. This in turn means that the compiler may run the #[derive] attribute multiple times for one type definition. That leads to a bunch of hairiness.

One proposal I might have is to tweak the expansion order of #[derive]:

This has the surprising effect that you're not expanding left-to-right, but then again remember that this is only #[derive]. This gives the compiler maximal knowledge about the struct definition and when it's expanding builtin traits we know that the structure of the type will never change.

How does that sound? I believe it solves all the constraints here?

@nikomatsakis I'm not sure that the strategy of placing the attribute on the impl will work because other custom derive modes could in theory change the layout of the struct, even the types of the fields. This would violate the compiler's assumptions when it first expanded I think.

jimmycuadra commented 7 years ago

Has the order in which derives are processed ever been officially declared as being left-to-right, via the Rust reference or anything? More generally, does the order of attributes ever matter? It sounds like it's just incidental that it was implemented that way, and macro authors shouldn't have been relying on a left-to-right order. Alex's proposal of processing custom derives first so that they never see magical attributes added by the compiler makes a lot of sense.

mystor commented 7 years ago

I'd just like to add that I don't like the idea that custom derives can change the layout of the struct. I would want to be able to use this for something which is safety-sensitive. As an example, consider the #[derive(Trace)] implementation used by rust-gc.

#[derive(Trace)]
struct Foo {
    a: Gc<i32>,
}

Expanding to:

struct Foo {
    a: Gc<i32>,
}

unsafe impl Trace { // NOTE: Strawman impl
    unsafe fn trace(&self) { Trace::trace(&self.a) }
}

However, if we allow changing the fields in the struct, we can define an Evil custom derive:

#[derive(Evil)]
struct Foo {
    a: Gc<i32>,
}

Expanding to:

struct Foo {
    a: Gc<i32>,
    b: Gc<i32>,
}

Which, if we combine them:

#[derive(Trace, Evil)]
struct Foo {
    a: Gc<i32>,
}

Expanding to:

struct Foo {
    a: Gc<i32>,
    b: Gc<i32>,
}

unsafe impl Trace {
    unsafe fn trace(&self) { Trace::trace(&self.a) }
}

Which is an unsound implementation of Trace. When used with rust-gc, this allows for b to be a dangling reference, which is horribly unsafe and unsound. This means that Trace isn't a safe thing to #[derive] anymore on a type, which is highly unfortunate.