Don't emit unexpected cfgs in proc-macros

[daxpedda]

This issue has been caused by recent changes in nightly: rust-lang/rust#132577. unexpected_cfg is now emitted from proc-macros as well.

This PR simply avoids this by checking the cfg check in the macro itself, which is required anyway to enable the corresponding unstable features.

This changes when the #[coverage] attribute is emitted by adding an internal coverage crate feature to the proc-macro and their utility crates. This crate feature is enabled by wasm-bindgen and wasm-bindgen-test when cfg(wasm_bindgen_unstable_test_coverage) is enabled.

[taiki-e]

AFAIK, this is the kind of cfg that is set globally on the user side, so the warning itself is not something we need to be concerned about except that it will break the build if we deny the warning.

Therefore, I wonder if it would be fine to simply allow unexpected_cfg in the generated code.

[daxpedda]

Indeed, however this workaround isn't required. In #4306 I simply used something like this:

fn coverage() -> Option<TokenStream> {
    #[cfg(wasm_bindgen_unstable_test_coverage)]
    return Some(quote! { #[coverage(off)] });
    #[cfg(not(wasm_bindgen_unstable_test_coverage))]
    None
}

The problem is not this PR, but #4277.

wasm-bindgen/crates/macro/src/lib.rs

Lines 27 to 34 in 94b2dc6

	#[cfg_attr(
	all(
	not(feature = "xxx_resolver_1"),
	not(feature = "std"),
	feature = "atomics"
	),
	allow_internal_unstable(thread_local)
	)]

Because there is no no_std version of thread_local! we have to use the unstable #[thread_local] feature. This is okay because it only concerns targeting target_feature = "atomics", which requires nightly to compile to begin with.

There are various possible workarounds:

• Require users to enable this unstable feature in their code. This doesn't really work because it has to be done in dependencies as well.
• Drop support for no_std with target_feature = "atomics".
• Use a custom thread-local implementation. This would require some sort of thread-safe implementation of a "list", unless we make a custom implementation of Mutex, which is an option as well. The performance would obviously be horrendous.

I don't see a way to make this happen without demoting target_feature = "atomics" from a second class citizen to not really working for most or at least some people depending on the solution picked from above.

While I do understand that this is annoying and has affected various projects (surprisingly less than I would have expected), I haven't really seen anybody actually arguing for feature resolver version 1 support. So my impression so far is that this is a CI issue and not a real-world-usage one. Which is still an issue, don't get me wrong! But I decided that so far this seems like a good price to pay.

If you do have some input on feature resolver version 1 support, please leave it in #4304.

[taiki-e]

Thanks for the clarification.

I don't see a way to make this happen without demoting target_feature = "atomics" from a second class citizen to not really working for most or at least some people depending on the solution picked from above.

The problem here is that the proc-macro side does not know which cfg is set on the library side, right?

In that case, I think we could use an approach like naked-function, where defining macros on the library side to handle the differences that depends on the cfg, and then call them from the proc-macro side.

https://github.com/Amanieu/naked-function/blob/5091474ca4e4984d683ad816b120e7d088197c6c/src/lib.rs#L7-L11

[daxpedda]

I don't see a way to make this happen without demoting target_feature = "atomics" from a second class citizen to not really working for most or at least some people depending on the solution picked from above.

The problem here is that the proc-macro side does not know which cfg is set on the library side, right?

In that case, I think we could use an approach like naked-function, where defining macros on the library side to handle the differences that depends on the cfg, and then call them from the proc-macro side.

https://github.com/Amanieu/naked-function/blob/5091474ca4e4984d683ad816b120e7d088197c6c/src/lib.rs#L7-L11

The problem is not the proc-macro output, the problem is setting those unstable features that enable the proc-macro output to compile. These have to be set on the proc-macro itself and at top-level inside the proc-macro crate. These #[feature(...)] statements, which are part of the crate and not of the proc-macro output, have to be disabled for the crate to compile on stable.

Moving the proc-macro output that requires these unstable features to a macro doesn't help either. The whole workaround is allow_internal_unstable, unless macro_rules! has an equivalent, this would lead to the problem I have outlined above: dependencies won't compile anymore with target_feature = "atomics".

[taiki-e]

Moving the proc-macro output that requires these unstable features to a macro doesn't help either. The whole workaround is allow_internal_unstable, unless macro_rules! has an equivalent, this would lead to the problem I have outlined above: dependencies won't compile anymore with target_feature = "atomics".

allow_internal_unstable is also supported on macro_rules! .
e.g., vec! uses it: https://github.com/rust-lang/rust/blob/acf48426b64d24f372d534f634072de1f4c7e588/library/alloc/src/macros.rs#L41

[daxpedda]

Moving the proc-macro output that requires these unstable features to a macro doesn't help either. The whole workaround is allow_internal_unstable, unless macro_rules! has an equivalent, this would lead to the problem I have outlined above: dependencies won't compile anymore with target_feature = "atomics".

allow_internal_unstable is also supported on macro_rules! . e.g., vec! uses it: https://github.com/rust-lang/rust/blob/acf48426b64d24f372d534f634072de1f4c7e588/library/alloc/src/macros.rs#L41

Oh, interesting! I will look into that then.

[daxpedda]

Thanks @taiki-e, its works and I'm working on a PR now!