unsafe extern { safe fn } support

[Nemo157]

Description

Declaring an external function as safe is a form of safety assertion, so it should be covered by undocumented_unsafe_blocks and require a // SAFETY: comment. (Or some kind of new group as that lint name is specific to blocks (containing #13316 too), but my expectation is that everyone enabling the existing lint actually wants the more general form).

I tried this code:

#![deny(clippy::undocumented_unsafe_blocks)]

unsafe extern "C" {
    pub safe fn foo();
}

I expected to see this happen: something like the error: unsafe block missing a safety comment message

Instead, this happened: no lint

Version

1.85.0-nightly (2024-12-03 c44b3d50fea96a3e0417)

Additional Labels

No response

[roberthree]

If I understand you correctly, this issue is not a duplicate of #13560, since it's about functions labelled safe inside unsafe extern-blocks, rather than unsafe extern itself. The desired result for your code example would be:

unsafe extern "C" {
    // SAFETY: ...
    pub safe fn foo();
}

However, in this case, I would not include it in undocumented_unsafe_blocks in its current form, as its name no longer reflects its scope (as pointed out in #13560 and #13316).
I think undocumented_unsafe_blocks and unnecessary_safety_comment need some refactoring in terms of their scope.

But focusing on safe-labelled functions, I fully agree that these should have safety documentation. As stated in Unsafe extern blocks, items marked as safe in unsafe extern-blocks can be used without an unsafe block. Therefore a proper argumentation is required similar to unsafe-blocks in safe functions.

#13316 and #13317 ask for requiring proper safety documentation for attributes, while #13560 asks for safety documentation for unsafe extern, all suggesting that one could extend undocumented_unsafe_blocks and unnecessary_safety_comment for this.
On the one hand, I could see merging undocumented_unsafe_blocks and unnecessary_safety_comment, because what's the point of having only one of them enabled? But on the other hand, with all these different structures there are two options:

1. Separate all these lints according to their respective structures, e.g. unsafe {}, unsafe impl, unsafe extern, safe fn, #[unsafe()], etc.
2. Replace these lints with a generic "proper_safety_comment" that can handle all structures for missing and unnecessary safety comments, because in the end it's about documenting the safe/unsafe barrier.

I think this should be addressed first before implementing the suggested lint.

[ojeda]

Declaring an external function as safe is a form of safety assertion, so it should be covered by undocumented_unsafe_blocks and require a // SAFETY: comment.

All unsafe extern blocks would need a // SAFETY comment, not just those containing a safe item -- supporting this is #13560 as @roberthree mentions.

But it is true that we may want to consider particular handling of individual items, especially for safe ones -- please see #13560 (comment) and rust-lang/rust-bindgen#3058 (comment).