Skip to content

False positive unnecessary 'unsafe' block warning? #9576

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

Closed
Vurv78 opened this issue Jul 12, 2021 · 15 comments
Closed

False positive unnecessary 'unsafe' block warning? #9576

Vurv78 opened this issue Jul 12, 2021 · 15 comments
Labels
A-macro macro expansion A-ty type system / type inference / traits / method resolution S-actionable Someone could pick this issue up and work on it right now

Comments

@Vurv78
Copy link

Vurv78 commented Jul 12, 2021

I'm not sure if this is a part of GenericDetour::call being generated by a macro, I thought this was fixed with expanding macros though. Removing the unsafe block just causes a missing-unsafe error

use detour::GenericDetour;

type Add5 = extern "Rust" fn(i32) -> i32;
extern "Rust" fn add_5(n: i32) -> i32 { n + 5 }
extern "Rust" fn hookfn(n: i32) -> i32 { n + 50 }

fn main() -> Result<(), detour::Error> {
	let hook = unsafe { GenericDetour::<Add5>::new(add_5, hookfn)? };
	
	// This unsafe is needed (afaik) yet causes a ``unnecessary `unsafe` block`` diagnostic?
	let _res = unsafe { hook.call(50) }; 

	Ok(())
}
@flodiebold
Copy link
Member

missing-unsafe is our diagnostic, "unnecessary unsafe block" comes from rustc. When in doubt, rustc is right ;) Apparently the call method is generated by a macro; depending on the macro (I didn't spend much time looking for the call), it might be necessary to enable rust-analyzer.experimental.procAttrMacros, or it might be another problem.

@bjorn3
Copy link
Member

bjorn3 commented Jul 12, 2021

@flodiebold
Copy link
Member

Oof. There are a lot of unsafe and safe versions of the method there, so RA might be getting the method selection wrong.

@Vurv78 Vurv78 changed the title False positive `unnecessary unsafe block` warning? False positive `unnecessary \unsafe\ block` warning? Jul 12, 2021
@Vurv78 Vurv78 changed the title False positive `unnecessary \unsafe\ block` warning? False positive unnecessary 'unsafe' block warning? Jul 12, 2021
@Vurv78
Copy link
Author

Vurv78 commented Jul 12, 2021

Oops, I didn't see the missing-unsafe error is from ra, but the warning is from rustc... So it should be safe but it is seen as unsafe by rust-analyzer

@lnicola lnicola added A-ty type system / type inference / traits / method resolution S-actionable Someone could pick this issue up and work on it right now labels Jul 14, 2021
@rossmacarthur
Copy link

Encountered this when using wasm-bindgen

#[wasm_bindgen(module = "/index.js")]
extern "C" {
    #[wasm_bindgen]
    pub fn my_function() -> JsValue
}

my_function is safe here but ra thinks it is unsafe.

@lnicola
Copy link
Member

lnicola commented Aug 20, 2021

@rossmacarthur try enabling rust-analyzer.experimental.procAttrMacros.

@nbars
Copy link

nbars commented Aug 28, 2021

I believe I've got the same issue when using the log crates logging functions:
Screenshot_2021-08-28_14-47-13

However, cargo build and cargo check work just fine. I'm using rust-analyzer 996300f4a 2021-08-23 stable and also tried the previous release.

Is there any way for me to get more details about why the rust-analyzer suspects the logging functions to be unsafe?

Keep up the good work :)

@nbars
Copy link

nbars commented Aug 28, 2021

It appears that using the nightly channel via "rust-analyzer.updates.channel": "nightly" solves the problem.

@lnicola
Copy link
Member

lnicola commented Aug 28, 2021

@nbars see #10022

@yatesco
Copy link

yatesco commented Nov 9, 2021

It appears that using the nightly channel via "rust-analyzer.updates.channel": "nightly" solves the problem.

I'm still getting this error, even with the nightly rust-analyzer (with stable rust and 2021 profile)
nevermind this might be an error with my vim config. VScode seems to work fine.

@ModProg
Copy link
Contributor

ModProg commented Dec 8, 2021

nevermind this might be an error with my vim config. VScode
seems to work fine.

@yatesco if you still have the problem with vim, idk if you use lspconfig, but this is working for me with rust-tools (a wrapper around lspconfig)

My Config
            require('rust-tools').setup {
                tools = {
                    autoSetHints = true,
                    hover_with_actions = true,
                    runnables = {use_telescope = true},
                    inlay_hints = {
                        show_parameter_hints = false,
                        parameter_hints_prefix = "",
                        other_hints_prefix = ""
                    }
                },
                -- see https://github.com/neovim/nvim-lspconfig/blob/master/CONFIG.md#rust_analyzer
                server = {
                    settings = {
                        -- https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/user/generated_config.adoc
                        ["rust-analyzer"] = {
                            checkOnSave = {command = "clippy"},
                            experimental = {procAttrMacros = true},
                            updates = {channel = "nightly"}
                        }
                    }
                }
            }

@yatesco
Copy link

yatesco commented Dec 8, 2021

Thanks @ModProg - it's working on the latest nightly now.

@Veykril
Copy link
Member

Veykril commented Dec 17, 2021

triage: this still happens

@Veykril Veykril added the A-macro macro expansion label Dec 17, 2021
@zicklag
Copy link

zicklag commented Aug 16, 2022

For anybody else still running into this, disabling the missing-unsafe diagnostic might help:

  "rust-analyzer.diagnostics.disabled": [
    "missing-unsafe"
  ],

I was having issues with wasm_bindgen even on the latest stable rust and latest RA, util I disabled that.

@Veykril
Copy link
Member

Veykril commented Jan 19, 2023

This doesn't happen for me anymore

@Veykril Veykril closed this as completed Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro macro expansion A-ty type system / type inference / traits / method resolution S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests

10 participants