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

unsafe extern support (undocumented_unsafe_blocks?) #13560

Open
ojeda opened this issue Oct 18, 2024 · 2 comments
Open

unsafe extern support (undocumented_unsafe_blocks?) #13560

ojeda opened this issue Oct 18, 2024 · 2 comments
Labels
A-lint Area: New lints

Comments

@ojeda
Copy link
Contributor

ojeda commented Oct 18, 2024

What it does

Rust 1.80.0 allows writing unsafe extern under feature(unsafe_extern_blocks), Rust 1.82.0 stabilizes it, and Edition 2024 will require it. Thus it would be nice to have a lint that ensures // SAFETY comments are in place.

undocumented_unsafe_blocks covers not just unsafe blocks (at least currently), so it could make sense to put it there, but it may make more sense to avoid adding more things into that one.

Advantage

No response

Drawbacks

No response

Example

unsafe extern {}

Should be written as:

// SAFETY: ...
unsafe extern {}
@roberthree
Copy link

In light of #13777, which addresses safe-labelled functions in unsafe extern-blocks, I'm curious what a safety comment for unsafe extern would look like? As stated in Unsafe external blocks (unsafe extern) and Unsafe extern blocks, the unsafe label for extern-blocks indicates that the developer has ensured that the signatures of all items are correct. In this sense, it may be more appropriate to require safety documentation for each item in an unsafe extern-block rather than a general comment for the entire block. It feels similar to a safety comment on top of an unsafe-block that covers multiple unsafe operations (which can be restricted with multiple_unsafe_ops_per_block).

@ojeda
Copy link
Contributor Author

ojeda commented Dec 19, 2024

unsafe extern already covers the full "signatures are correct", which includes safe, but it is true it may be convenient to give particular comments per-item inside.

When writing the blocks manually, then one may for instance give a general statement in the // SAFETY on top of unsafe extern about the signatures, but defer the explanation of safe ones to safety comments inside.

In such a case, it would be important to have a way to tell Clippy to not lint about unnecessary safety comments inside.

For instance:

// SAFETY: (... why the signatures are correct ...). In addition,
// for `safe` items, please see the safety comments inside.
unsafe extern {
    // SAFETY: (.. why `safe` is fine here ...).
    pub safe fn f();
}

Other projects may want to explain individually why particular signatures are correct though, e.g. if reasons differ. Though they could also use multiple blocks.

Now, when using a tool like bindgen, one is likely relying on the generated code to be correct (assuming the tool is setup correctly and doesn't have bugs etc.), and thus the tool should probably fill the comment at the top of the block.

However, the tool may also provide a way to understand which items can be marked safe in the generated code. For instance, it may recognize something like a [[safe]] attribute in the C header and/or the user may provide a list, etc. The tool could then fill the reasons accordingly.

This came up in rust-lang/rust-bindgen#3058 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

2 participants