Open joshlf opened 1 year ago
Hi, is this issue still open?
Yes! We have ~20 instances of #[allow(clippy::undocumented_unsafe_blocks)]
in our codebase. If you're interested in driving that number down, I recommend you tackle them (at first) one-at-a-time. We have a high bar for safety comments, and it can take some practice to get the hang of. See the comments in ptr.rs
for examples of the sort of safety comments we're looking for. Comments should, ideally:
Yes, I am interested in this, thank you!
Can I work on this if this is still open?
As part of #61, we need to make sure that all
unsafe
code is proven to be sound. Currently, allunsafe
code is either documented with a safety comment (// SAFETY: ...
), or is marked with a TODO that references this issue. Our goal is to reach 100% safety comment coverage, and to not regress once we've reached 100%. To that end, we enforce theclippy::undocumented_unsafe_blocks
lint to prevent regressions.A note on linting: It'd be nice to be able to replace the top-level
#![deny(clippy::undocumented_unsafe_blocks)]
with aforbid
once all TODOs are burned down, but unfortunately oursafety_comment!
macro relies on the ability to use#[allow(clippy::undocumented_unsafe_blocks)]
, so we have to settle for a deny.In order to ensure that our soundness is forwards-compatible, safety comments must satisfy the following criteria:
Mentoring instructions
TODO(#429)
in a comment; leave a GitHub comment on this issue to claim that instance to avoid duplicated work. Write a safety comment that abides by the requirements listed above.Feel free to ask for help here if you're stuck or have questions!
List of suggested safety comments
List
This list contains safety comments which are good starter safety comments if you're not already familiar with writing them. - [ ] https://github.com/google/zerocopy/blob/f001cf24c0dba5c396c691236a5d347270ea6066/src/lib.rs#L757-L767 - [ ] https://github.com/google/zerocopy/blob/f001cf24c0dba5c396c691236a5d347270ea6066/src/lib.rs#L757-L767