Enabling the `nightly` feature can be a breaking change

[alexcrichton]

I've seen a number of crates now where when the nightly feature is enabled the crate's compilation breaks due to procedural macros. This typically happens around resolution in macro expansion and has to do primarily with quote! I believe.

If the nightly feature is disabled then everything is unhygienic and I think works with the equivalent of Span::call_site(), meaning that all the tokens produced by quote! ended up morally being used with Span::call_site(). When nightly is enabled, however, the quote! macro is actually under the hood using Span::def_site() everywhere (it was basically just ignored without the nightly feature).

tl;dr; tokens produced by quote! use Span::call_site() when nightly is not enabled, and Span::def_site() when nightly is enabled

@dtolnay do you have thoughts on this? Do you think we should, while Span is unstable, move the quote crate to using Span::call_site() by default?

[alexcrichton]

FWIW this is where quote! use Span::def_site() by default but on stable this is the same as call_site, whereas on nightly they are different

[kdy1]

How about Span::def_site().resolved_at(Span::call_site())?

[alexcrichton]

@kdy1 perhaps yeah, but this sort of largely an issue for the quote! crate where the incompatibility creeps up, so quote! could certainly change defaults but if it's doing that already isn't that equivalent to using Span::call_site() by default?

[kdy1]

I thought Span::def_site() is used to change span of error reporting, but both point call_site().

If the only difference between them is resolving, I think quote! should use call_site because using def_site shadows a bug unfixable by end user.

Span::def_site() allows

fn wrap_in_const<T: Into<TokenStream>>(const_name: &str, item: T) -> proc_macro::TokenStream {
    let item = Quote::new(Span::def_site()).quote_with(smart_quote!(
        Vars {
            item: item.into(),
            // CONST_NAME: Ident::new(const_name, call_site()),
        },
        {
            extern crate swc_common;
            extern crate std;
            item
        }
    ));
    item.into()
}

This does not work with Span::call_site() and on stable it's a compile error which cannot be fixed without forking macro crate.
Furthermore, macro authors typically use nightly while developing it mainly because pretty-printing isn't stable yet, so it's more likely for this bug to hide.

[dtolnay]

I can see how defaulting to call_site would reduce the number of cases in which a crate compiles without the nightly feature but not with nightly.

But I think that hewing as close as possible to macros 1.1 semantics is not the best way to build experience with our vision for macros 2.0. I have found def_site to be a better default (when you don't need the same generated code to work on stable as well). For now I am comfortable with there being two stages to making a macro work: get it working with the stable shim, then get it working with nightly.

[ngg]

I just had this exact issue but it seems not only the spans matter.
I have a project where I use https://github.com/danburkert/prost and my own https://github.com/ngg/futures-await-test.
Both these crates depend on proc_macro2 and I updated the futures-await-test to use the nightly version and try to be hygienic (0.1.2 version, later I reverted depending on nightly to temporarily solve this).
This made prost use the nightly feature as well, and it didn't work. After solving the issue with the def_site()/call_site() I still had some errors, I think this means there are further incompatibilities between the stable and the nightly versions.
I created a prost issue here with the details: https://github.com/danburkert/prost/issues/88
I don't know yet what exactly causes it but I get these errors:

error: int literal is too large
error: proc-macro derive produced unparseable token

These disappear if I switch back to the non-nightly version.

I think there are going to be lots of problems like this if anyone publishes a crate with the nightly feature as a dependency. For example if you would publish a new version of the futures-await crate based on the current master branch, it would break several other crates.

My suggestion is to remove this feature from this crate and publish it separately, that way some dependencies could use the stable and some the nightly version until every incompatibilities are solved.

[alexcrichton]

@ngg oh dear! Do you think you'd be able to minimize the example to see where the int literal is too large error is coming from? Independent of the hygiene issue here I think it'd be good to fix that as well!

[ngg]

I will have some time on the weekend, I'll try to minimize it.

[ngg]

I could reduce the int literal part to really simple. The other unparseable token error is probably independent from this as my repro case does not trigger that. I'll try to minimize that part as well.
I'm not familiar with the compiler internals enough to know if it is a compiler bug, a bug in proc-macro2 or a bug in quote (but my guess is that it's a compiler bug).
I have a small repro crate at https://github.com/ngg/proc_macro_nightly_bug.
Basically the problem is with negative integer literals.

The minimal dependency-free example looks like:

#[proc_macro]
pub fn buggy_macro(_: TokenStream) -> TokenStream {
    TokenNode::Literal(Literal::i32(-1)).into()
}

Something like this is generated by quote when using the nightly feature from proc-macro2:

#[proc_macro]
pub fn buggy_macro(_: TokenStream) -> TokenStream {
    let x = -1i32;
    quote!(#x).into()
}