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

New Lint: Unsafe block in safe functions without SAFETY comment #7238

Closed
fee1-dead opened this issue May 18, 2021 · 4 comments
Closed

New Lint: Unsafe block in safe functions without SAFETY comment #7238

fee1-dead opened this issue May 18, 2021 · 4 comments
Labels
A-lint Area: New lints

Comments

@fee1-dead
Copy link
Member

What it does

Detects unsafe blocks used in safe functions missing a comment explaining/justifying the use of unsafe.

Categories (optional)

  • Kind: clippy::pedantic

What is the advantage of the recommended code over the original code

  • Explains why the use of unsafe in the function is okay
  • Makes it easier to maintain

Drawbacks

None.

Example

fn get(self, slice: &[T]) -> Option<T> {
  if self < slice.len() { unsafe { Some(&*self.get_unchecked(slice)) } } else { None }
}

Could be written as:

fn get(self, slice: &[T]) -> Option<T> {
  // SAFETY: `self` is checked to be in bounds.
  if self < slice.len() { unsafe { Some(&*self.get_unchecked(slice)) } } else { None }
}
@fee1-dead fee1-dead added the A-lint Area: New lints label May 18, 2021
@giraffate
Copy link
Contributor

giraffate commented May 20, 2021

From API guidelines, unsafe functions should be documented with a "Safety" section, so why do you want it with unsafe blocks?

FYI

  • Clippy has the missing_safety_doc lint.
  • Clippy can't find just comment. Doc comments is ok.

@fee1-dead
Copy link
Member Author

There are some requirements for a call to an unsafe function to be safe. A comment with unsafe blocks can explain how the safety contract is respected. As shown in the example, it could be easier to maintain if the developer knows that unsafe block has been checked. This new lint can help developers to make sure that all usage of unsafe is documented.

For checking comments, I don't know if there is a solution. The Rust repository does document their usage of unsafe so I thought "why not a lint?"

@PatchMixolydic
Copy link
Contributor

An implementation for this seems to be in the works: #7557

bors added a commit that referenced this issue Oct 8, 2021
Add undocumented_unsafe_blocks lint

changelog: Added a new lint [`undocumented_unsafe_blocks`]

Fixes #7464, #7238 (?)
@fee1-dead
Copy link
Member Author

Since #7748 was merged, I am closing this.

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

3 participants