Interoperability with proc-macro-error

[CreepySkeleton]

trafficstars

Author of proc-macro-error here.

It's a shame that "replace proc_macro with proc_macro2" approach doesn't work with my crate. I suspect this is because #[proc_macro_error] has some proc_macro::TokenStream in its expansion, and the expansion is done after the grand replacement.

#[proc_macro_error]
fn my_macro(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
    // code
}

// after replacement:
#[proc_macro_error]
fn my_macro(input: proc_macro2::TokenStream) -> proc_macro2::TokenStream {
    // code
}
// after expansion:
fn my_macro(input: proc_macro2::TokenStream) -> proc_macro::TokenStream {
    // a little mangled code
}

Come to think of it, PME doesn't really care about the types there. It's just that proc macro facility expects the TokenStream to be from proc_macro, not from elsewhere, and my crate has to accommodate. But it's no longer the case with your crate.

I have a little compact hack in mind:

You crate searches for #[proc_macro_error] and replaces it with #[proc_macro_error(__cargo_watt_compat)]. On the other side, my crate detects it and uses proc_macro2 instead of proc_macro.

Would you be open to accept such a PR?

[jakobhellermann]

sure, that would be nice

[jakobhellermann]

Actually, I think a tweak in the macro will probably not be enough. Since cargo watt patches syn to use proc_macro2 instead of proc_macro, other crates (like proc_macro_error) are likely to break.

For procedural macros that just depend on syn and quote this is not a big deal, because they too have proc_macro replaced. But for those depending on synstructure, proc_macro_error or similar it's not so easy.

Another solution would be to detect if the crate uses proc_macro_error and patch it (and proc_macro_error_attr) to a compatible version, like this. That has the disadvantage that it would only support the newest (or a compatible) version of proc_macro_error.

[CreepySkeleton]

First of all, I've managed to tweak my crate to work with both proc_macro and proc_macro2 without any changes on your side.

But yeah, I can't just stop depending on syn, the dep would have to be patched, too. Actually, I depend on syn-mid, another proc macro helper, which also depends on syn.

If you want to continue relying on the dumb replacement, you have only two options:

• Drop support for any macros that depend on something that depends (possibly transitively) on syn and quote. This may be feasible option, there are plenty of macros out there that don't depend on other helpers. On the other hand, it would be really pity because there are quite a few popular macros that depend on synstructure, proc_macro-error, darling or something else.
• Recursively traverse dependency graph, and, for every dep that depends on syn or quote, patch the dependencies to your own forks and apply the grand replacement to the crate itself. Should be doable.

And that's not to mention that some macros use unstable types from proc_macro on nigthly (proc-macro-error does) proc_macro2 lacks. Nightly builds will likely break.

[jakobhellermann]

I don't think it is possible to make any approach work flawlessly for every crate, at least not without wasm-macros being implemented as first-class citizens in rustc.

So I think I will just accept (and clarify in the README) that cargo watt will never work for all crates, because of its trial and error approach which comes to its limits pretty quickly.

Still, I think it may be feasible to either provide patches for popular macro helper libraries (like syn-mid, synstructure and proc_macro_error) or at least make it easy to add these patches yourself (--patch crate repo for example).

FWIW, I made proc_macro_error (not proc_macro_error_attr) work by running

sd 'proc_macro::' 'proc_macro2::' src/*.rs
# conflicting impl
sed -i '/impl DoubleSpanSingleSpan for proc_macro2::Span {/,+4d' src/lib.rs

so if the macro can be made to work with both TokenStreams, adding support for proc_macro_error should just be a matter of adding a patched git repo to a list of automatically applied patches.

[Freax13]

it's possible to get proc_macro working outside a macro on nightly by implementing proc_macro::bridge::server::Server and using proc_macro::bridge::client::Client. wouldn't that solve most of the compatibility issues?

[jakobhellermann]

I didn't know about proc_macro::bridge. Where can I find documentation about it?

[Freax13]

https://github.com/rust-lang/rust/blob/master/src/libproc_macro/bridge/mod.rs it's only meant to be used internally, but you can use it with #![feature(proc_macro_internals)]

[Freax13]

it's not documented anywhere afaict, so you kinda have to read the source code

[jakobhellermann]

I wonder whether this could be used in watt directly? If watt could handle pub extern "C" fn the_macro(input: proc_macro::TokenStream) -> proc_macro::TokenStream (not proc_macro2), then the translation from regular macro to watt macro would be very easy.

[Freax13]

probably

[pksunkara]

@CreepySkeleton, I was talking about this on Reddit but what about the approach of the watt feature in syn and proc_macro_error? I think that would solve all our issues.

We add the feature to syn first eliminating the need for the patch. Then, we add support for this feature to proc_macro_error, then we can simply enable these features and compile clap_derive with cargo watt build. I think the other creates synstructure etc.. can start propagating this feature and at the end all the crates should work with watt.

[jakobhellermann]

I looked into what it would take to support watt as a feature in syn, it doesn't seem to be that complicated: https://github.com/jakobhellermann/syn-watt/commits/watt-feature

[pksunkara]

Yup, I looked at your patch and did a little digging before suggesting this.

[CreepySkeleton]

A dedicated feature was actually the first thing I thought about but decided that it would be quite a lame solution.

You see, features are user facing, now more than ever since rustdoc has been working on explicit cfg tags. The first thing crossing user's mind when they see a feature is "How - and for what - can I use it?". In our case, the answer would be that they don't, it's just an implementation detail that was supposed to be hidden from users but wasn't. Also, such a feature in proc-macro-error, syn, whatever will become a marker of sorts which will divide proc macros to those that work with watt and those do not. This all sounds very unnecessary.

I like @Freax13 idea - orchestrate the macro expansion on our own, either by implementing Server or patching libproc_macro as whole and replacing thr default dynamic lib with our own via LD_PRELOAD or some other hack.

It looks like @Freax13 is working on it! https://github.com/dtolnay/watt/issues/42