rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.23k stars 12.7k forks source link

[edition 2024] `missing_unsafe_on_extern` in macros with older editions #132425

Open daxpedda opened 4 days ago

daxpedda commented 4 days ago

I tried this code:

use wasm_bindgen::prelude::*;

#[wasm_bindgen]
unsafe extern {
    pub fn test();
}

I expected to see this happen: it should compile. Instead, this happened: it fails to compile.

Meta

rustc --version --verbose:

rustc 1.84.0-nightly (1e4f10ba6 2024-10-29)
binary: rustc
commit-hash: 1e4f10ba6476e48a42a79b9f846a2d9366525b9e
commit-date: 2024-10-29
host: x86_64-unknown-linux-gnu
release: 1.84.0-nightly
LLVM version: 19.1.1
Compile Error

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


Basically the problem is that the #[wasm-bindgen] proc-macro generates extern "C" blocks which aren't prefixed with the unsafe keyword. However, my understanding is that proc-macros should carry the edition from the crate they are coming from, which in this case is edition 2021. The rules of edition 2024 should not be applied to the generated code from this edition 2021 proc-macro.

That said, this is easily fixable in wasm-bindgen, but the bug will probably break other libraries as well.

Cc https://github.com/rustwasm/wasm-bindgen/issues/4218.

ehuss commented 4 days ago

cc @spastorino Do you by chance know what is happening? Is it possible that wasm-bindgen is using the wrong hygiene?

Or maybe the validation is looking at a span injected from the user's code?

spastorino commented 5 hours ago

Just gave it a very quick look ... this error is emitted in https://github.com/spastorino/rust/blob/2dece5bb62f234f5622a08289c5a3d1555cd7843/compiler/rustc_ast_passes/src/ast_validation.rs#L964-L977. The if part is using span.at_least_rust_2024 https://github.com/spastorino/rust/blob/2dece5bb62f234f5622a08289c5a3d1555cd7843/compiler/rustc_ast_passes/src/ast_validation.rs#L965-L966 so it shouldn't be the problem. What is probably being executed is https://github.com/spastorino/rust/blob/2dece5bb62f234f5622a08289c5a3d1555cd7843/compiler/rustc_ast_passes/src/ast_validation.rs#L968-L975 as a forward incompatible lint (allow by default). I'm not sure why this would be failing though. Would need to try things out and debug it to get a better understanding of what's going out. Unless I'm missing something obvious.

ehuss commented 2 hours ago

I believe the problem is here.

wasm-bindgen is modifying the tokens of the generated extern block to use the span of the identifier from the input. That identifier will have the 2024 hygiene. Thus the entire extern block will have 2024 hygiene, and thus the validation will think it is a 2024 extern block.

I think this might need to be fixed on the wasm-bindgen side. I don't know how the edition hygiene of the item span is computed if the item is built from a mix of tokens, but I'm wondering if wasm-bindgen could not modify the span of the extern "C" { tokens. That is, only respan this line and these lines, and not the surrounding stuff like extern "C".

ehuss commented 2 hours ago

Another alternative solution is for wasm-bindgen to include the unsafe keyword if the input includes it. That way, the caller can update their extern block to include unsafe. However, cargo fix probably won't work with that.