"extern blocks must be unsafe" for 2024 edition

[zroug]

Describe the Bug

wasm_bindgen attribute doesn't work for extern blocks in 2024 edition.

Steps to Reproduce

1. Use rustc 1.84.0-nightly and configure project to use 2024 edition.
2. Try to compile

   use wasm_bindgen::prelude::*;
   
   #[wasm_bindgen]
   extern {
       pub fn test();
   }

3. Playground link (but it can't be reproduced on the playground because it only affects the WASM target).

Expected Behavior

The example should compile.

Actual Behavior

error: extern blocks must be unsafe
 --> src/test.rs:5:9
  |
5 |     pub fn test();
  |            ^^^^

Note: Adding the unsafe keyword to the extern block doesn't help, same error.

[daxpedda]

My first impression is that this is actually a Rustc bug.
Proc-macros carry on the edition from the crate that its generated from, to avoid new editions breaking all proc-macros from older editions.

I will get back to this as soon as I know more.
While we can't update to the 2024 edition, because of our MSRV, we can easily adjust the generated output to include the unsafe keyword, as its supported with our MSRV. So this shouldn't be a problem.

While familiarizing myself with the 2024 editions, I also noticed that the attributes export_name, link_section and no_mangle now require wrapping them in an #[unsafe] attribute, which isn't supported in our MSRV.
Though luckily, I checked and it doesn't trigger any errors.

[daxpedda]

See rust-lang/rust#132425.

[ehuss]

After some investigation, I don't believe there is anything that rustc can do to avoid this. It uses hygiene data of the spans to determine which edition something is from, and because of the behavior with respan mentioned in rust-lang/rust#132425, that edition information is getting overridden.

@daxpedda Would you be willing to try to find some way to adjust respan (or the way it is called) so that it doesn't change the hygiene data? I think the only token that matters is the extern keyword that starts the extern block, and we need to avoid changing the hygiene on that token to the caller's edition.

One option is to avoid calling respan() or set_span() on that token.

Another option, and I'm not certain this is correct, is to use located_at() which will use the edition from the proc-macro, but with the position data from the caller. So, something like this:

diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs
index e5f3cfb9..d5d14743 100644
--- a/crates/backend/src/codegen.rs
+++ b/crates/backend/src/codegen.rs
@@ -1894,9 +1894,9 @@ fn respan(input: TokenStream, span: &dyn ToTokens) -> TokenStream {

     for (i, token) in spans.into_iter().enumerate() {
         if i == 0 {
-            first_span = token.span();
+            first_span = Span::call_site().located_at(token.span());
         }
-        last_span = token.span();
+        last_span = Span::call_site().located_at(token.span());
     }

     let mut new_tokens = Vec::new();

I'm not familiar with wasm-bindgen, so I don't know how to test that this still gives the kinds of error messages that you want.

[daxpedda]

Thank you for looking this deeply into it and even suggesting a solution @ehuss.

I'm going to take the lazy route here and simply add unsafe to all extern "C" { ... } blocks, as this is still compatible with our MSRV. This should avoid any changes to error messages.

[ehuss]

I'm going to take the lazy route here and simply add unsafe to all extern "C" { ... } blocks, as this is still compatible with our MSRV. This should avoid any changes to error messages.

Ah! I was going to suggest this, but I saw rust-version = "1.57" in your Cargo.toml, which seemed far too old to support it (since unsafe extern was added in 1.82). I assumed you are not willing to bump it that high?

[daxpedda]

Indeed you are right, I was reading the reference from v1.57 and assumed that it supports unsafe extern, but missed this paragraph:

The unsafe keyword is syntactically allowed to appear before the extern keyword, but it is rejected at a semantic level. This allows macros to consume the syntax and make use of the unsafe keyword, before removing it from the token stream.

[zroug]

Does #4259 actually do what it says it does? I tried out the PR and I still get the same error, and there is no mention of unsafe in the diff.

[daxpedda]

Does #4259 actually do what it says it does?

No, I changed the OP in #4259 to reflect the actual change. It applies the diff from #4218 (comment).

I tried out the PR and I still get the same error, and there is no mention of unsafe in the diff.

How did you try it? It works for me locally.

[zroug]

Okay, while trying to build a minimal example, the error disappeared, and then it disappeared in the larger project aswell.

Sorry, I was trying to use the patch in a larger project and had to work with a lot of patches to dependencies and tooling. But I must have missed something, and with the discrepancy in the PR description and my lack of proc macro knowledge, I was a little too eager to jump to conclusions. Sorry for the inconvenience.