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

Lints about unsafe {} blocks propagate inside macros #74838

Open
poliorcetics opened this issue Jul 27, 2020 · 5 comments
Open

Lints about unsafe {} blocks propagate inside macros #74838

poliorcetics opened this issue Jul 27, 2020 · 5 comments
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@poliorcetics
Copy link
Contributor

Coming from #74200.

I used #![deny(unsafe_op_in_unsafe_fn)] in std/panicking.rs. This raised an error about a missing unsafe {} block in std/thread/local.rs. Adding it raised a warning about extraneous unsafe {} in other parts of the code. This forced me to use #[allow(unused_unsafe)].

It was noted that lints like those (whatever those is) should maybe not propagate inside macros.

cc @Mark-Simulacrum who raised the point in the first place

@LeSeulArtichaut LeSeulArtichaut added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 28, 2020
@RalfJung
Copy link
Member

It is expected behavior that unsafe blocks in a macro need #[allow(unused_unsafe)] to not get a warning when the macro is itself used inside an unsafe block. Sounds like that is what happened here?

@RalfJung
Copy link
Member

I think the fix for this would be something like "unsafety hygiene". I am not sure if that is on the roadmap, but it is probably not backwards compatible so it could be done only for macro macros (not macro_rules! macros).

Cc @petrochenkov

@Mark-Simulacrum
Copy link
Member

Unsafety hygiene would be the "next step", but to start I would just want to not warn about unsafe inside macros if they're already in an unsafe context -- that seems reasonable. I imagine that would be backwards compatible? We could also just silence "unused_unsafe" entirely inside macros, I guess, which would be easier.

@jyn514
Copy link
Member

jyn514 commented Jun 16, 2023

i think this was fixed by #100081

mj10021 added a commit to mj10021/rust that referenced this issue Jul 24, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 24, 2023
delete [allow(unused_unsafe)] from issue rust-lang#74838

While looking into issue rust-lang#111288 I noticed the following `#[allow(...)]` with a `FIXME` asking for it to be removed.  Deleting the `#[allow(...)]` does not seem to break anything, it seems like the lint has been updated for unsafe blocks in macros?
@kadiwa4
Copy link
Contributor

kadiwa4 commented Sep 27, 2023

current output (nightly 1.74.0 2023-09-26):

macro_rules! spooky_macro {
    () => {
        unsafe { core::mem::zeroed::<u32>() }
    };
}

fn main() {
    // case 1: warning: unnecessary `unsafe` block
    unsafe {
        let _: [u8; 4] = core::mem::transmute(spooky_macro!());
    }
}

pub unsafe fn spooky_fn() {
    // case 2: Fixed! No warning.
    let _: [u8; 4] = core::mem::transmute(spooky_macro!());
}

So yes, since this issue was specifically filed about case 2, you could consider it fixed.

There are two other issues about the same kind of problem: #49112 and #94912

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants