Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wasm-bindgen 0.2.92 triggers clippy::mem_forget #3944

Closed
Conaclos opened this issue May 1, 2024 · 2 comments · Fixed by #3985
Closed

wasm-bindgen 0.2.92 triggers clippy::mem_forget #3944

Conaclos opened this issue May 1, 2024 · 2 comments · Fixed by #3985
Labels

Comments

@Conaclos
Copy link

Conaclos commented May 1, 2024

Summary

wasm-bindgen 0.2.92 triggers the Clippy rule mem_forget. Is it expected?

error: usage of `mem::forget` on `Drop` type
  --> crates/biome_wasm/src/lib.rs:24:1
   |
24 | #[wasm_bindgen]
   | ^^^^^^^^^^^^^^^
   |
   = note: argument has type `wasm_bindgen::JsValue`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#mem_forget
   = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info)
@Conaclos Conaclos changed the title wasm-bindgen 0.2.92 and later triggers clippy::mem_forget wasm-bindgen 0.2.92 triggers clippy::mem_forget May 1, 2024
@Liamolucko
Copy link
Collaborator

I think this is the mem::forget it's picking up on:

#wasm_bindgen::__rt::std::mem::forget(value);

There's an #[allow(clippy::all)] right above it that's supposed to prevent issues like this, but I'm now realising that I've misunderstood how that works a bit: clippy::all isn't actually all of clippy's lints, it's clippy's default set of lints, which doesn't include mem_forget.

What we should really be using is #[automatically_derived], which does disable all lints - I'll make a PR switching it to use that.

@Liamolucko
Copy link
Collaborator

What we should really be using is #[automatically_derived], which does disable all lints - I'll make a PR switching it to use that.

Hm, seems like I was wrong: Clippy still throws a warning on this code even though it uses #[automatically_derived]:

#![warn(clippy::mem_forget)]
struct Foo;
#[automatically_derived]
impl Foo {
    fn test() {
        std::mem::forget(String::new());
    }
}

Searching for automatically_derived in rustc/clippy's source code, and looking back at the original PR where we started using it (#2719), it seems like specific lints manually disable themselves if they see it, wherever it makes sense.

So I'm not really sure what the best solution is then, other than just spamming #[allow(clippy::all, clippy::nursery, clippy::pedantic, clippy::restriction)] everywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants