Decouple proc_macro from the rest of the compiler. by eddyb · Pull Request #49219 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation270 Commits16 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This PR removes all dependencies of proc_macro on compiler crates and allows multiple copies of proc_macro, built even by different compilers (but from the same source), to interoperate.

Practically, it allows:

running proc macro tests at stage1 (I moved most from -fulldeps to the regular suites)
using proc macros in the compiler itself (may require some rustbuild trickery)

On the server (i.e. compiler front-end) side:

server::* traits are implemented to provide the concrete types and methods
- the concrete types are completely separated from the proc_macro public API
- the only use of the type implementing Server is to be passed to Client::run

On the client (i.e. proc macro) side (potentially using a different proc_macro instance!):

client::Client wraps around client-side (expansion) function pointers
- it encapsulates the proc_macro instance used by the client
- its run method can be called by a server, to execute the client-side function
  * the client instance is bridged to the provided server, while it runs
  * ~~currently a thread is spawned, could use process isolation in the future~~
  (not the case anymore, see Run proc macro invocations in separate threads. #56058)
proc macro crates get a generated static holding a &[ProcMacro]
- this describes all derives/attr/bang proc macros, replacing the "registrar" function
- each variant of ProcMacro contains an appropriately typed Client<fn(...) -> ...>

proc_macro public APIs call into the server via an internal "bridge":

only a currently running proc macro Client can interact with those APIs
- server code might not be able to (if it uses a different proc_macro instance)
  * however, it can always create and run its own Client, but that may be inefficient
the bridge uses serialization, C ABI and integer handles to avoid Rust ABI instability
each invocation of a proc macro results in disjoint integers in its proc_macro handles
- this prevents using values of those types across invocations (if they even can be kept)

r? @alexcrichton cc @jseyfried @nikomatsakis @Zoxc @thepowersgang

stepancheg reacted with thumbs up emoji kennytm, hanna-kruppe, eternaleye, LifeIsStrange, Mark-Simulacrum, sinkuu, Manishearth, Zoxc, jseyfried, wesleywiser, and 31 more reacted with hooray emoji ebkalderon and lambda-fairy reacted with heart emoji

bors added a commit that referenced this pull request

Mar 20, 2018

Decouple proc_macro from the rest of the compiler.

This PR removes all dependencies of proc_macro on compiler crates and allows multiple copies of proc_macro, built even by different compilers (but from the same source), to interoperate.

On the compiler (aka "frontend") side:

Registry is implemented (as until now)
- instead of function pointers, the Expand{1,2} wrapper types are received
FrontendInterface is implemented to provide the concrete type and methods
- the concrete types are completely separated from the proc_macro public API
- the only use of the implementer is to be passed to Expand{1,2}::run

On the proc macro side (potentially using a different proc_macro instance):

&mut Registry is passed to the registrar (as until now)
Expand{1,2} wrappers are created to be passed to the Registry
- they encapsulate the proc_macro instance used by the macro crate
- their run method will set up the "current frontend" of that instance

proc_macro public APIs call into the "current frontend" via the internal bridge:

only a currently running proc macro can interact with those APIs
- the frontend itself might not be able to (if it uses a different proc_macro instance)
the bridge uses C ABI to avoid Rust ABI instability and "generation tagging" for safety
each invocation of a proc macro results in a different "generation"
- specifically, opaque proc_macro types wrapping concrete frontend types are tagged
- this prevents using values of those types across invocations (even if they can be kept)
an ABI mismatch between two versions of proc_macro is only possible during stage1
- specifically, rustc compiled by stage0 but proc macros compiled by stage1
- can only affect running tests at stage1 or the compiler using proc macros
- defficiencies in the bridge can be addressed without waiting for a release cycle

r? @alexcrichton cc @jseyfried @nikomatsakis @Zoxc @thepowersgang

cc @rust-lang/infra We shouldn't merge this without a crater run, although you might want to wait on a review as well (then again, the results could inform the review, so whichever comes first).


#[unstable(feature = "proc_macro_internals", issue = "27812")]
#[doc(hidden)]
pub contents: Term,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to commit to using interning for literals? I'm not sure interning it terrible useful for that.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, I can make these fields private again. Definitely not considering stabilizing any of this.

@@ -11,3 +11,6 @@ crate-type = ["dylib", "rlib"]
[dependencies]
getopts = "0.2"
term = { path = "../libterm" }

# not actually used but needed to always have proc_macro in the sysroot
proc_macro = { path = "../libproc_macro" }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be a rustbuild change.

Could you use something like bincode for data bridging, instead of 800 lines of own code?

@bjorn3 I'm not serializing the data (but rather using handles), and bincode (like any other serialization) is annoying to use without derived impls (which you can't have because this is proc_macro itself - or at least I don't want to try to make that work, for now).

I am going to look into simplifying the bridge code, removing most unsafe, and making it more amenable to message-passing, however.

This comment has been minimized.

eddyb changed the title~~Decouple proc_macro from the rest of the compiler.~~ [WIP] Decouple proc_macro from the rest of the compiler.

Mar 22, 2018

This comment has been minimized.

@Mark-Simulacrum It doesn't look bad, that's for sure, thanks! If we switch to spawning a thread (or even a process) per invocation it'd warrant another perf run.

Phlosioneer added a commit to Phlosioneer/rust that referenced this pull request

Mar 25, 2018

A few tests related to proc-macros fail when performing stage-1 testing. This PR adds // ignore-stage1 to them, along with a FIXME.

Original issue: rust-lang#49352

This should (hopefully) be fixed when rust-lang#49219 is merged.

eddyb mentioned this pull request

Mar 30, 2018

Hi @eddyb (crater requester), @Zoxc (PR reviewer)! Crater results are at: http://cargobomb-reports.s3.amazonaws.com/pr-49219/index.html. 'Blacklisted' crates (spurious failures etc) can be found here. If you see any spurious failures not on the list, please make a PR against that file.

(interested observers: Crater is a tool for testing the impact of changes on the crates.io ecosystem. You can find out more at the repo if you're curious)

This was referenced

Nov 30, 2018

I can't believe you've done this!

eddyb, oli-obk, kennytm, fedochet, RalfJung, matthewjasper, euclio, ljedrz, mark-i-m, Centril, and 9 more reacted with laugh emoji

eddyb deleted the proc-macro-decouple branch

November 30, 2018 11:13

hcpl mentioned this pull request

Dec 9, 2018

sfackler added a commit to sfackler/proc-macro2 that referenced this pull request

Dec 18, 2018