`unsafe_code` lint fires for `unsafe fn` with safe body

[chbaker0]

#![forbid(unsafe_op_in_unsafe_fn)]
#![forbid(unsafe_code)]

trait Foo {
    unsafe fn dangerous();
}

struct SafeImpl;

impl Foo for SafeImpl {
    unsafe fn dangerous() {
        println!("this is safe!");
    }
}

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=3f3fbdb3494c6770487823f284c303f9

With forbid(unsafe_op_in_unsafe_fn), an unsafe fn's body in principle no longer has an implicit unsafe block. So, unsafe_code should not trigger on an unsafe fn alone: there is not necessarily any unsafe code inside!

This has practical effect. Consider one crate defining interfaces with unsafe methods (e.g. representing an OS API). Another crate wants to create mock implementations of these interfaces that are 100% safe. While any crate consuming these mocks will necessarily contain unsafe {...} blocks, it is nice for the test support crate to declare it itself has only safe code.

[chbaker0]

@rustbot label +A-diagnostics

[mu001999]

What about unsafe traits? They are also forbidden by lint unsafe_code.

[chbaker0]

Yes, unsafe trait items should be allowed too IMO, it's only unsafe impl that's actually unsafe.

Both unsafe trait and unsafe fn say "users must follow my contract", while unsafe { ... } blocks and unsafe impl say "I'm following the contracts defined by the interfaces I'm using". Only the latter are asserting things the compiler can't check.

[Ezrashaw]

@rustbot claim

[RalfJung]

I agree, the current behavior is quite confusing (but at least it is consistent between unsafe fn and unsafe trait).

[ia0]

I agree there seems to be a general confusion regarding unsafe, so I decided to write down my mental model to help education in that regard (the main idea being to make the proof type explicit as it would be in dependent type programming languages). This general confusion could be decomposed in the following core confusions:

• Confusion between the unsafe keyword in types and in code. In types, the unsafe keyword represents properties (things to prove). In code, the unsafe keyword represents proofs (proving the properties of the unsafe keyword in the associated type). This confusion is accentuated by the fact that it's the same unsafe keyword in both cases (versus say using unsafeprop in types and unsafeproof in code, e.g. unsafeprop trait and unsafeproof impl).
• Confusion between pre- and post-conditions, or more generally between requirements and guarantees. Requirements are things to prove. Guarantees are things you can rely on to prove requirements. This confusion is accentuated by the fact that there is currently no simple way to have post-conditions (there's only unsafe traits and it's quite a heavy encoding) and by the fact that "pre" and "post" may be interpreted as the code "before" and "after" the function call while it is actually about the properties to provide and receive from the function call. In particular, pre-conditions may refer to code after the function call (like Pin::get_unchecked_mut or String::as_mut_vec) and post-conditions may refer to data before the function call.

I am not recommending Rust to change anything. I am just trying to lift this general confusion regarding unsafe through educational materials. Whether Rust should change anything (and how) is up to the community as a whole. In particular, lifting the confusion should be enough to clarify issues like this one. Once the first core confusion is lifted it should be obvious that unsafe fns and unsafe traits are not unsafe code and should not lint. Only unsafe blocks and unsafe impls should lint.