New lint to minimize the unsafe block scope.

[xFrednet]

What it does

Checks for unsafe blocks that contain safe code, which can be extracted to the outside safe code.

Categories (optional)

• Kind: pedantic

Rust doesn't enforce memory safety guarantees in unsafe blocks. Having more code inside the unsafe block than necessary, might result in bugs which could be avoided by having the code in the save-code-scope. It also clearly indicates which part of an operation is unsafe.

Drawbacks

Using the smallest unsafe code block possible might be less readable.

Example

unsafe {
    let mut vec: Vec<u8> = Vec::with_capacity(128);
    vec.set_len(100);
    println!("{:#?}", vec);
}

Could be written as:

let mut vec: Vec<u8> = Vec::with_capacity(128);
unsafe {
     vec.set_len(100);
}
println!("{:#?}", vec);

[afetisov]

Disagree. There is no reason to artificially make the scope smaller, the exact unsafe operations are already highlighted by every IDE, and there is nothing gained from duplicating that information in the block structure. On the other hand, unsafe code correctness depends on some invariants which are unknown to the compiler, and it makes sense, when possible, to enclose in unsafe { } the entire subset of code which requires such external invariants.

Basically, a good practice is to have SAFETY comment next to each unsafe block, with a clear documentation of the required invariants and the proof for the safety of that block. If you shrink or split your unsafe block into smaller ones, would you be able to write such documentation? Or maybe you would be repeating the same arguments and appealing to something outside of the unsafe block? In the latter case introducing smaller blocks made the code harder to reason about, rather than more clear.

For example, the Vec::set_len function from your example is unsafe, but doesn't have any unsafe operations inside, since Rust has no concept of unsafe field access. Let's say we write a hand-rolled implementation of Vec, and within that implementation we directly access the len field rather than call set_len. I insist that accesses to len should be within unsafe blocks because their correctness relies on extra invariants, but the lint would make me put it outside of unsafe blocks. Changing len could be a single unsafe thing within a safe function! A function with no unsafe inside could have the possibility of wrecking memory safety invariants. Yes, technically the actual memory-unsafe operation would be element access, but it would be hard to find where its assumptions were violated.

Since the semantic requirements cannot be checked by Clippy, I don't think this lint should exist, even as pedantic. People which want to improve their code at the cost of extra effort would be misled into actually making the code worse.

[xFrednet]

For the given reasons, the lint should, if ever created, be restriction rather than pedantic as some projects warn about pedantic. I'll leave this open for discussion and also think about it a bit more for now. Thank you very much for the comment, you mentioned some good reasons that I haven't thought about 🙃