feat: Proc macro multi apbi proof of concept

[alexjg]

#8925 was irritating me so I thought I would have a bash at fixing it. What I've done here is copy the crates/proc_macro_srv/src/proc_macro code (which is copied from <RUST>/library/proc_macro) to crates/proc_macro_srv/src/proc_macro_nightly and the modified the nightly version to include the changes from #9047 and yotamofek@aeb7b18

This gives us the code to support both stable and nightly ABIs. Then we use the proc_macro_api::version::read_dylib_info to determine which version of the ABI to load when creating a ProcMacroLibraryLibLoading (which is now an enum).

This seems to work for me. The code could be cleaned up but I wanted to see if the approach makes sense before I spend more time on it.

I've split the change into two commits, the first is just copying and modifying the proc_macro crate, the second contains most of the interesting work around figuring out which ABI to use.

[matklad]

I think it all makes sense, yeah!

One thing I worry is that, by copy-pasting from rustc more, it will be hard to maintain this in the long run, as few people understand how this thing works process wise.

I suggest merging this on Monday (after the release, to get more testing time), and then, if anyone wants to:

• automate the copy-pasting by adding cargo xtask update-syntax-bridge command or an ignored test a-la sourcegen_int_completion
• refeactor the rustc side of things, such that the syntax bridge is auto-published and is compatible with stable.

[bjorn3]

proc_macro should be autopublished as rustc-ap-proc_macro I think. It doesn't compile on stable though as it uses a lot of unstable features: https://github.com/rust-lang/rust/blob/master/library/proc_macro/src/lib.rs#L20-L33

[matklad]

Yeah, we def don't want to de-facto stabilize the whole of proc-macro. I guess, the pre-requisite here is extracting the bridge module to a stand-alone crate first.

[alexjg]

One thing that I am concerned about with this approach is that the ABI selection is done by this piece of code:

if info.version.0 < 1 {
    Err(LoadProcMacroDylibError::UnsupportedABI)
} else if info.version.1 < 47 {
    Err(LoadProcMacroDylibError::UnsupportedABI)
} else if info.version.1 < 54 {
    Ok(ProcMacroABI::Stable)
} else {
    Ok(ProcMacroABI::Nightly)
}

I think this means that every time there is a version bump in the stable rust compiler we will need to increment the version in the third branch of this expression right?

[flodiebold]

I think the problem is more the naming of the ABIs; what's currently the "nightly" ABI will become the "stable" one at some point (to be precise: when 1.54 releases). If we want to support the latest stable and nightly all the time, that's probably the best approach though; when the stable with the changed ABI is released, we'll remove "stable", copy "nightly" to "stable" and make < 54 unsupported as well. Or we could keep supporting older stable versions, but I'm not sure we want to do that.

Edit: Alternatively we could e.g. just call them A and B and just add new ones when there are more compatibility breaks. The version selection would still be correct though, we'd just add more else ifs (unless there are multiple ABI breaks within one nightly that we want to support -- which I doubt we would)

[lnicola]

Maybe we can name them by their release version?

[flodiebold]

You mean 1_47 and 1_54? That would make sense as well, I think.

[lnicola]

Yeah, and update 1_54 during the release cycle if it changes more than once.

[flodiebold]

Yeah, that might be the best variant. And if there's a new ABI break in the future, we can introduce e.g. 1_56, and then separately think about whether we want to limit the number of ABIs we support to two or keep all of them.

[alexjg]

I like it, alright, let me just whip something up.

[alexjg]

I've reorganised things a bit so that it should be easier to add ABIs in the future. Take a look.

[Dessix]

Yeah, and update 1_54 during the release cycle if it changes more than once.

If an ABI changes more than once in nightly (Has this ever happened before?), would it be better to make the tuple a triple, and have the third member be either the variant or the date-of-change?

Many repositories- my own included- pin a working nightly in rust-toolchain to avoid breakages in CI. If an ABI changes multiple times in the same target version, we could still end up with non-functioning rust-toolchain options despite having gone through the effort of making various ABIs work.

As for how many versions to support- what about limiting ourselves to two stabilized ABI versions and "all(ish) nightly ABI versions that haven't reached stable"?

[lnicola]

It happened during this release cycle, at least.

I think it's fine to support only one nightly (preferably the latest). Regarding projects that pin a certain nightly, that's more often than not an older one (from a previous release cycle), so this wouldn't necessarily help.

Plus, there might be other workarounds like using an older RA version, possibly the one from rustup (which gets updated weekly-ish).

all(ish) nightly ABI versions that haven't reached stable

Did you mean the nightly versions in the current release cycle, or all the nightly versions ever?

[Dessix]

Did you mean the nightly versions in the current release cycle, or all the nightly versions ever?

I meant all nightly ABI versions that haven't reached stable yet. As for the "ish", I mean that any that are deemed too broken or too short-lived could be skipped.

[lnicola]

I meant all nightly ABI versions that haven't reached stable yet.

In the current cycle (1.55) or also the nightlies from the previous release cycles?

[Dessix]

I was advocating for the current release cycle. I wasn't aware old release cycles were a thing people used, but I've never used stable for longer than the first few rustup commands. If a nightly ABI variant is never going to reach stable, there's little sense in maintaining support for it once stable passes it up.

[lnicola]

Many repositories- my own included- pin a working nightly in rust-toolchain to avoid breakages in CI.

Which nightly are you using now? Pretty much every project I've seen doing this was using an older nightly.

If a nightly ABI variant is never going to reach stable, there's little sense in maintaining support for it once stable passes it up.

Yeah, but a nightly ABI is even more temporary than a stable one (stable is forever, nightlies are only relevant for a couple of days in the model above). A workaround would be to use rust-analyzer-preview from that specific nightly, which is still not great, but presumably temporary (at the latest, we'd drop it when the release cycle ends, as you proposed).

[Dessix]

Which nightly are you using now? Pretty much every project I've seen doing this was using an older nightly.

2021-05-17, the newest I found that worked with Rust-Analyzer still supporting proc-macros. Normally, we'd adjust the pin forward every other week or two.

Supporting every nightly is one thing- but the ABI from 5-17 remains functional with rust-analyzer builds from 2 months later- so I suspect they either don't change often enough to cause problems, or something else is going on that I don't understand, but it doesn't sound - to me - like the cutoff date needs to be anywhere in the realm of "days".

[lnicola]

So the two changes in the current cycle were merged on May 20 and 30. Assuming we can summon alexjg to update this right after a breaking change is merged, I'm not sure it's worth supporting those nightlies now in July.

[Dessix]

Fair- I'm not suggesting we backdate, but with only a few breaks every few months, it seems like it should be viable to support future changes without immediately dumping the older variants, since the code will already be written if they were in any way long-lived.

[alexjg]

I'm happy to keep an eye on this if other changes need making. I should stress though that I don't really understand the actual changes that were made to the ABIs, I just packaged up work other people had done and organised the code a bit so that it's possible for us to use multiple ABIs at all.

The biggest downside of not dropping ABIs as we go forward is probably compilation times no? Regardless, we can make each decision as and when, we're not locked in to any particular schedule here.

[lnicola]

The biggest downside of not dropping ABIs as we go forward is probably compilation times no?

Yes, and some interference with the CI metrics (memory, build time, type inference statistics). Of course, those aren't perfect anyway, and they change from one rustc release to another.

with only a few breaks every few months, it seems like it should be viable to support future changes without immediately dumping the older variants

It's a matter of policy: we already only support the latest stable compiler and last two VS Code releases. Keep in mind that technically older nightlies are unsupported in the context of the larger Rust project.

It's one thing to say "we support the latest stable, beta, and some recent nightly version" and something else to say "we support stable, beta, and every nightly in the last three months" (and yes, somebody will probably file an issue for any version we skip). What if we don't get to implement one ABI version before a second breaking change gets merged?

Again, this happens seldom enough that it should only rarely be an inconvenience. And you might as well use the rustup version, which is "guaranteed" to work, but still gets updated often enough.

[lnicola]

@matklad any opinion on the nightly thing?

Unrelated, I'm inclined to r+ this today, in order to test it this week.

[matklad]

I think a reasonable stance is to initially support only the latest nightly, and see how it goes. If there's a huge demand for supporting more than one nightly, and volontaires to do the supporting work, we can add more! But starting small let's us to better estimate the maintenance/benefit tradeoff.

bors r+

[bors]

Build succeeded:

• Rust (macos-latest)
• Rust (ubuntu-latest)
• Rust (windows-latest)
• TypeScript (ubuntu-latest)
• TypeScript (windows-latest)

[flodiebold]

If we support more ABIs, it also increases the effort when we need to change non-ABI-related duplicated code, like the expand and list_macros functions. Maybe we can reduce that duplication though.