RA should use the toolchain's proc_macro instead of copying library/proc-macro.

[eddyb]

_{I've brought this up elsewhere in the past, but it should've probably been a GH issue from the start. (I also regret not getting in touch with any RA devs when I heard they were working around the internal APIs of proc_macro, instead of with them - ideally we would streamline the aspects RA needs)}

Right now, RA contains (slightly modified IIUC) copies of rust-lang/rust's library/proc-macro for various versions, and when the proc-macro ABI breaks on nightly, RA's proc macro support starts crashing.

However, this is far from the intended usage of the unstable proc_macro::bridge implementation details.

Instead, RA's proc macro support should be built against specific toolchains, and there's two scenarios:

• RA build that supports multiple Rust versions
  • would have to embed sources (e.g. include_str!("proc-macro-srv/.../foo.rs")) to build on user's machine
  • to support stable, RUSTC_BOOTSTRAP=1 would need to be used (this is not IMO philosophically different from the second scenario, it's just that the build doesn't happen on rust-lang CI)
  • presumably this would be a binary that could be cached user-wide (when rustup is used, it could be cached in the toolchain itself, with the appropriate invalidation based on toolchain version etc.)
• RA build shipped with rustup
  • this can be more strongly tied to the same Rust version it ships with (i.e. refuse working with mismatched Cargo/rustc)
  • can include the proc macro support in the build process, just like rustc does

For stable versions, pre-building the proc macro support in RA CI is also possible, but I don't see that scaling up to individual nightlies.

[Veykril]

RA build that supports multiple Rust versions

I don't think I quite understand the points here so I'll ask some questions to clarify for myself.

• would have to embed sources (e.g. include_str!("proc-macro-srv/.../foo.rs")) to build on user's machine

Do you mean we should be building rust-analyzer with the rustc proc-macro server src on the users machine?

• to support stable, RUSTC_BOOTSTRAP=1 would need to be used

This is technically fine, we rely on this var for querying cargo on stable 🤐 Though it would be nice if we didn't have to resort to this.

RA build shipped with rustup
Ye, I agree, the RA build shipped with rustup should just ship with the proc-macro abi of that version and nothing else.

If I understand this right, this is solely talking about rustup shipped builds or custom builds by the user. But there are also the VSCode published builds which won't work with this approach.

[flodiebold]

Could we instead just build and distribute the rust-analyzer proc macro server together with rustc (maybe even in rustc)? This would require keeping the interface between rust-analyzer and the proc macro server somewhat stable, but I think that should be easier than adapting to ABI changes, considering it's just a JSON interface.

("somewhat stable" would mean being backwards- and forwards-compatible over a few rustc versions, since rust-analyzer doesn't guarantee compatibility with more than that.)

(That would incidentally also fix the M1 Mac architecture confusion problems that have shown up recently.)

[bjorn3]

to support stable, RUSTC_BOOTSTRAP=1 would need to be used

If the proc macro bridge is put in a crate separate from the user facing proc_macro crate, I think it wouldn't be too hard to make the proc macro bridge compile on stable. I believe I already removed almost all uses of unstable features from it. The proc_macro crate still needs unstable features for making api's unstable and for performance, but this shouldn't be the case for the bridge.

[eddyb]

Do you mean we should be building rust-analyzer with the rustc proc-macro server src on the users machine?

@Veykril Sorry, I should've been more specific, I'm saying RA would be built and shipped as normal, but proc-macro-srv would be built against the libproc_macro-*.rlib that the toolchain on the user's machine contains (the source code of proc_macro isn't necessary, just like it isn't to build a proc macro).

---

@flodiebold The rustup-provided rust-analyzer component would include the binary you're talking about, are you suggesting that it should be present outside of that component? Should it get its own component or do you want it to be in the rustc component itself?

(I don't think it should be in rustc itself, or at least I've avoided considering something like that, as it introduces a lot more conundrums, e.g. at that point it could read rmeta, and the separation between that and the functionality available through the proc macro bridge would get fuzzy)

---

@bjorn3 The bridge is inherently an unstable API, I don't really understand this point. The reason to use RUSTC_BOOTSTRAP=1 isn't to be able to compile proc_macro, it's to use an existing one.
The bridge isn't intended to be separated or treated as source code, only x.py ever compiles it more than once. Once a toolchain contains a libproc_macro-*.rlib, the sound way to use that bridge is to compile against proc_macro::bridge with RUSTC_BOOTSTRAP=1.

[flodiebold]

The rustup-provided rust-analyzer component would include the binary you're talking about, are you suggesting that it should be present outside of that component? Should it get its own component or do you want it to be in the rustc component itself?

Yes, I'm suggesting that it would be a separate binary that is always built with rustc. That would solve the problem for all rust-analyzer builds, not just the rustup ones. So I'd want it to be in the rustc component, so that we can rely on it always being there.

(I don't think it should be in rustc itself, or at least I've avoided considering something like that, as it introduces a lot more conundrums, e.g. at that point it could read rmeta, and the separation between that and the functionality available through the proc macro bridge would get fuzzy)

I don't quite understand what you mean here, why would it read rmeta and why would that be a problem?

[Veykril]

Sorry, I should've been more specific, I'm saying RA would be built and shipped as normal, but proc-macro-srv would be built against the libproc_macro-*.rlib that the toolchain on the user's machine contains (the source code of proc_macro isn't necessary, just like it isn't to build a proc macro).

We'd need to store the binary somewhere then though since we don't want to rebuild this per project.

I like the idea of shipping the proc-macro server as a rustup component tbh, that way we could also get around having to build it ourselves. (IntelliJ could just use it as a rustup component as well then instead of copying our thing)

[eddyb]

So I'd want it to be in the rustc component, so that we can rely on it always being there.

Assuming it's nestled in the same lib/rustlib/$HOST/bin "private area" (that contains e.g. rust-lld), and named appropriately (e.g. rust-analyzer-...), I don't see that big of an issue with this.

Presumably someone would have to open a compiler-team MCP to get this going.
(unless this falls under @rust-lang/infra? not 100% sure on that aspect)

However, I should note that experimenting with external RA using the rust-analyzer component of rustup for proc-macro-srv (and ignoring the full RA build in there), could be done with less hassle and should still work just as well (at least for nightlies).

---

I don't quite understand what you mean here, why would it read rmeta and why would that be a problem?

Currently, when rustc loads a proc macro crate, it combines information from both the embedded rmeta (i.e. as if it were an rlib or dylib crate, though very little is encoded for proc macro crates) and the &[proc_macro::bridge::ProcMacro]-typed static it can read through dlsym.

If proc-macro-srv was part of rustc, it would be very easy to decode the embedded rmeta and get some information (e.g. the names of the proc macros) from there, without the proc_macro::bridge interface needing to carry that same information (this is, coincidentally, the kind of concern that reminded me to open this issue in the first place).

IOW, with access to rustc-internal knowledge, the design space expands a lot more, on top of also introducing the specter of this being a quasi-stable interface to rustc.

[jyn514]

Presumably someone would have to open a compiler-team MCP to get this going.
(unless this falls under https://github.com/orgs/rust-lang/teams/infra? not 100% sure on that aspect)

bootstrap itself falls under T-infra, but libproc_macro is in kind of a weird place because it integrates libs, compiler, and also has to deal with bootstrapping ... I think bringing it up in T-compiler is reasonable since people there know more about proc-macros.

If you want to ship more files in the rustc dist component, that definitely falls under T-infra.

I haven't read through this whole issue, but I'm one of the current maintainers of bootstrap, happy to answer questions.

[jyn514]

(just skimmed the issue - +1 to not exposing any rustc_private functionality to the proc macro server, it should only be able to access stuff in bridge)

[flodiebold]

IOW, with access to rustc-internal knowledge, the design space expands a lot more, on top of also introducing the specter of this being a quasi-stable interface to rustc.

OK, I can see that.

However, I should note that experimenting with external RA using the rust-analyzer component of rustup for proc-macro-srv (and ignoring the full RA build in there), could be done with less hassle and should still work just as well (at least for nightlies).

It would require people to suddenly install the rust-analyzer component even though they get RA through e.g. VSCode. Also, I don't think there's any reason for the proc-macro server to be tied to rust-analyzer at all in this setup (except for saying "this is intended for rust-analyzer and we don't provide any stability guarantees for the protocol outside of that"). We could even put the code into the rust repo? The only reason against that I see is that we'd have the definitions of the JSON protocol in two places.

[jyn514]

We could even put the code into the rust repo? The only reason against that I see is that we'd have the definitions of the JSON protocol in two places.

doesn't rust-lang/rust already have rust-analyzer as a submodule? I don't see why we'd need to also duplicate proc-macro-srv in-tree.

[flodiebold]

We wouldn't duplicate it; it would live in rust-lang/rust because it's independent (code-wise) from rust-analyzer.

The reason why I'm slightly against keeping it in rust-analyzer is that it might give the impression that the rust-analyzer binary and the proc-macro-server binary would be matched (built from the same revision), which would not be the case -- they'd have to be compatible with older versions in both directions. The only code that would have to be duplicated would be the struct definitions for the JSON protocol, and I feel having these duplicated would actually reflect the fact that changing the protocol definition in the server doesn't affect what code runs in rust-analyzer. (That's still a big improvement over the current situation, where we have duplicated the proc-macro bridge multiple times!)

(As an analogy, consider the cargo metadata format.)

Also, if the server is part of the rustc component, it makes sense to me that the code would live in the rustc repo, but 🤷‍♂️

Anyway, we could also start with the code in the rust-analyzer repo and move it later if we want, so I don't think this question is blocking 🤷‍♂️ We should make the server an actual separate binary and not a subcommand of rust-analyzer though, since otherwise we'll have to build the full rust-analyzer as part of the rustc component...

[flodiebold]

(I guess there is an alternative future where rust-analyzer is always distributed over rustup, and the extension runs the RA version matching the compiler version configured for each project, e.g. in rust-toolchain.toml. In that future, we could indeed rely on the RA and proc-macro server always matching. The problem with that though is that a VSCode workspace can potentially contain multiple Cargo workspaces with differing rust-toolchain.tomls. Also, people will want to use newer rust-analyzer versions with stable rustc.)

[jyn514]

(As an analogy, consider the cargo metadata format.)

hmm, that gives me an idea - can we make a rust crate that parses the JSON and have both rust-analyzer and proc-macro-srv depend on that? that avoids duplication but still allows you to have different versions in RA and rust-lang/rust.

[lf-]

I don't think that the JSON interface being unstable is problematic as long as it's versioned; the previous breakages of r-a were generally because of abi breaks and us not updating our side yet. As in, a versioned-but-can-change-arbitrarily API is basically what we have today, just with more unfortunate copying of code.

[flodiebold]

Also, if the code for the server lives in the rust-analyzer repo, it could be broken on nightly by changes in the proc macro bridge, right? In which case either there's no nightly, or there is a nightly but no server, which kind of defeats the purpose.

[eddyb]

@flodiebold the "standard" way we've been fixing this genre of issue in rust-lang/lang is using git subtree to sync between a rust-lang/rust directory and another repo, while allowing local changes - e.g. if rustc_hir is changed and clippy stops building, merging such a PR is blocked on fixing the rust-lang/rust copy of clippy (which someone may later sync with the separate clippy repo).

I believe miri is the last major holdout from this process - cc @oli-obk (I'm not sure if there's a problem there or if they haven't gotten around to it).

[oli-obk]

(I'm not sure if there's a problem there or if they haven't gotten around to it).

the latter.

[fasterthanlime]

#12835 (partially) closes this.

The steps after that are:

• Re-add rust-analyzer as a subtree to rust-lang/rust
• Teach rust-analyzer to use the "rust-analyzer rustup component's proc macro server"
• Profit!

[eddyb]

So I'm not sure how to share this, but I was wondering about what it would take to long-term not need certain usecases to even touch proc_macro::bridge at all (e.g. with proc_macro working outside of an active session, a bit like proc-macro2 - @mystor's been pushing for that) _{and I got carried away until I got this really cursed attempt "working" (faux-standalone proc_macro via rustc expanding a trojan horse + reaching private symbols via offset from some exported symbol).}

---

EDIT (after sleeping on it): to be perfectly clear, this is an useless demo:

• the symbol doesn't even relate to the name of the proc macro
  • also it's private, which accounts for 66.6% of the cursedness budget
  • you couldn't even find it by reading the (exported) "decls" static
  • long-term we could maybe semi-stabilize something in this area ^{_{(v0 mangling could theoretically help)}}, but I suspect there would be pushback either way
• Spans (neither for diagnostic nor hygiene reasons) can't really be expressed
  • this accounts for most of the lossiness of stringification
  • while you ^{_{can technically abuse macro_rules! (or TokenStream::from_str) to generate unique hygiene}}, none of that efficiently allows mapping to some other representation and they're broken wrt diagnostics (e.g. wouldn't allow slicing up a literal span)
  • likely needs an RFC on a "semantically stratified" design of Span, so years out
    ^{_{(i.e. properly decomposing diagnostics/hygiene across original-source/expansion-output/synthetic-tokens/etc.)}}

The bridge does remain the only way to reliably provide all of the proc_macro API.
^{_{(and unlike a Rust frontend w/ miri/backends, RA also can't interpret/compile proc macros on its own to bypass interop w/ rustc-compiled proc macros)}}

[fasterthanlime]

This PR just landed to rust-lang/rust:

• Sync rust-analyzer, add rust-analyzer-proc-macro-srv binary to Rustc component rust#99756

Here's what it means: #12803 (comment)

I think this issue can be closed — although we can wait for monday, too :)

[eddyb]

Thank you so much for working on this!

Sounds like we got what was being discussed back in e.g. the first half of #12579 (comment), which I would've expected to take much longer, so that's a welcome surprise 🚀