Macros wrapping expressions in `unsafe` blocks when using `unsafe_op_in_unsafe_fn`

[ojeda]

What it does

For projects using unsafe_op_in_unsafe_fn (and specially those requiring // SAFETY comments), it would be nice to detect cases where a macro is wrapping an expression passed as a parameter in a "hidden" unsafe block, and thus bypassing the intention of unsafe_op_in_unsafe_fn.

The logic could be something like:

• If inside an unsafe fn...
• And there is a macro call outside an unsafe block...
• And unsafe_op_in_unsafe_fn is enabled in this context...
• And if the node/AST/tree of a parameter appears as-is...
• Then check that node/AST/tree is not inside an introduced unsafe block.

Callers of the macro would need to wrap the entire macro call in an unsafe block to avoid hitting the lint (plus perhaps playing with #[allow(unused_unsafe)] inside the macro to avoid the unneeded nested unsafe warning from rustc).

Categories (optional)

• Kind: clippy::pedantic?

Drawbacks

None.

Example

Assume #![deny(unsafe_op_in_unsafe_fn)], and that container_of! wraps the first parameter in an unsafe block. Then:

let reg = container_of!((*file).private_data, Self, mdev);

Could be written as:

let reg = unsafe { container_of!((*file).private_data, Self, mdev) };

[llogiq]

I have a case in compact_arena where a macro ensures the safe call of an unsafe function.

I would consider that a false positive. I would not see a problem with allowing the lint in this situation, but the lint docs should consider this possibility.

[camsteffen]

I am reviewing #7469 and want to raise a question before merging. @flip1995

I think these two things are orthogonal:

1. I want to be warned when an unsafe function lacks an unsafe block. (unsafe_op_in_unsafe_fn)
2. I want to be warned when a macro hides an unsafe block. (new lint)

The fact that you are in an unsafe function does not make the hidden unsafe block any more egregious IMO. So I don't think this lint should be dependent on #[warn(unsafe_op_in_unsafe_fn)] unsafe fn. Am I making sense?

#[warn(unsafe_op_in_unsafe_fn)]
unsafe fn f() {
    unsafe_macro!(); // this is bad
}

fn g() {
    unsafe_macro!(); // so isn't this just as bad?
}

[flip1995]

I think it should depend on the unsafe_op_in_unsafe_fn lint if the macro is in an unsafe function. Without enabling the unsafe_op_in_unsafe_fn your saying that it is ok to mark a function as unsafe and then everything that happens in this function may or may not be unsafe, which includes macro calls that may hide unsafe ops.

For your example in fn g() I think you're right. This is also problematic and the lint should probably trigger here regardless whether the unsafe_op_in_unsafe_fn lint is enabled or not.

[camsteffen]

I see what you mean.

#[allow(unsafe_op_in_unsafe_fn)]
unsafe fn h() {
    unsafe_macro!(); // this is ok
}

[ojeda]

I guess this ended up being macro_metavars_in_unsafe, right?

[ojeda]

Cc'ing @y21 here as well, in case he wasn't aware of this one, since he implemented macro_metavars_in_unsafe.

[blyxyas]

@ojeda Does macro_metavars_in_unsafe fix the issue for Rust-For-Linux or do you still need a specialized lint?

[ojeda]

I think so -- at least it does for the example above, i.e. if I create a "bad" container_of! and use it, then macro_metavars_in_unsafe does lint for me.

So closing!