Skip to content

Commit

Permalink
library: Normalize safety-for-unsafe-block comments
Browse files Browse the repository at this point in the history
Almost all safety comments are of the form `// SAFETY:`,
so normalize the rest and fix a few of them that should
have been a `/// # Safety` section instead.

Furthermore, make `tidy` only allow the uppercase form. While
currently `tidy` only checks `core`, it is a good idea to prevent
`core` from drifting to non-uppercase comments, so that later
we can start checking `alloc` etc. too.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
  • Loading branch information
ojeda committed Feb 24, 2021
1 parent fe1bf8e commit eefec8a
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 19 deletions.
4 changes: 2 additions & 2 deletions library/alloc/src/collections/btree/map/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,14 +278,14 @@ impl<'a, K: Ord, V> VacantEntry<'a, K, V> {
pub fn insert(self, value: V) -> &'a mut V {
let out_ptr = match self.handle.insert_recursing(self.key, value) {
(Fit(_), val_ptr) => {
// Safety: We have consumed self.handle and the handle returned.
// SAFETY: We have consumed self.handle and the handle returned.
let map = unsafe { self.dormant_map.awaken() };
map.length += 1;
val_ptr
}
(Split(ins), val_ptr) => {
drop(ins.left);
// Safety: We have consumed self.handle and the reference returned.
// SAFETY: We have consumed self.handle and the reference returned.
let map = unsafe { self.dormant_map.awaken() };
let root = map.root.as_mut().unwrap();
root.push_internal_level().push(ins.kv.0, ins.kv.1, ins.right);
Expand Down
17 changes: 9 additions & 8 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1938,13 +1938,13 @@ impl<T, A: Allocator> Vec<T, A> {
pub fn split_at_spare_mut(&mut self) -> (&mut [T], &mut [MaybeUninit<T>]) {
let ptr = self.as_mut_ptr();

// Safety:
// SAFETY:
// - `ptr` is guaranteed to be in bounds for `capacity` elements
// - `len` is guaranteed to less or equal to `capacity`
// - `MaybeUninit<T>` has the same layout as `T`
let spare_ptr = unsafe { ptr.cast::<MaybeUninit<T>>().add(self.len) };

// Safety:
// SAFETY:
// - `ptr` is guaranteed to be valid for `len` elements
// - `spare_ptr` is offseted from `ptr` by `len`, so it doesn't overlap `initialized` slice
unsafe {
Expand Down Expand Up @@ -2154,7 +2154,8 @@ pub fn from_elem_in<T: Clone, A: Allocator>(elem: T, n: usize, alloc: A) -> Vec<
}

trait ExtendFromWithinSpec {
/// Safety:
/// # Safety
///
/// - `src` needs to be valid index
/// - `self.capacity() - self.len()` must be `>= src.len()`
unsafe fn spec_extend_from_within(&mut self, src: Range<usize>);
Expand All @@ -2165,14 +2166,14 @@ impl<T: Clone, A: Allocator> ExtendFromWithinSpec for Vec<T, A> {
let initialized = {
let (this, spare) = self.split_at_spare_mut();

// Safety:
// SAFETY:
// - caller guaratees that src is a valid index
let to_clone = unsafe { this.get_unchecked(src) };

to_clone.iter().cloned().zip(spare.iter_mut()).map(|(e, s)| s.write(e)).count()
};

// Safety:
// SAFETY:
// - elements were just initialized
unsafe {
let new_len = self.len() + initialized;
Expand All @@ -2187,11 +2188,11 @@ impl<T: Copy, A: Allocator> ExtendFromWithinSpec for Vec<T, A> {
{
let (init, spare) = self.split_at_spare_mut();

// Safety:
// SAFETY:
// - caller guaratees that `src` is a valid index
let source = unsafe { init.get_unchecked(src) };

// Safety:
// SAFETY:
// - Both pointers are created from unique slice references (`&mut [_]`)
// so they are valid and do not overlap.
// - Elements are :Copy so it's OK to to copy them, without doing
Expand All @@ -2203,7 +2204,7 @@ impl<T: Copy, A: Allocator> ExtendFromWithinSpec for Vec<T, A> {
unsafe { ptr::copy_nonoverlapping(source.as_ptr(), spare.as_mut_ptr() as _, count) };
}

// Safety:
// SAFETY:
// - The elements were just initialized by `copy_nonoverlapping`
self.len += count;
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/io/copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl<I: Write> BufferedCopySpec for BufWriter<I> {
Ok(0) => return Ok(len), // EOF reached
Ok(bytes_read) => {
assert!(bytes_read <= spare_cap.len());
// Safety: The initializer contract guarantees that either it or `read`
// SAFETY: The initializer contract guarantees that either it or `read`
// will have initialized these bytes. And we just checked that the number
// of bytes is within the buffer capacity.
unsafe { buf.set_len(buf.len() + bytes_read) };
Expand Down
10 changes: 7 additions & 3 deletions library/std/src/lazy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,13 +440,17 @@ impl<T> SyncOnceCell<T> {
res
}

/// Safety: The value must be initialized
/// # Safety
///
/// The value must be initialized
unsafe fn get_unchecked(&self) -> &T {
debug_assert!(self.is_initialized());
(&*self.value.get()).assume_init_ref()
}

/// Safety: The value must be initialized
/// # Safety
///
/// The value must be initialized
unsafe fn get_unchecked_mut(&mut self) -> &mut T {
debug_assert!(self.is_initialized());
(&mut *self.value.get()).assume_init_mut()
Expand All @@ -456,7 +460,7 @@ impl<T> SyncOnceCell<T> {
unsafe impl<#[may_dangle] T> Drop for SyncOnceCell<T> {
fn drop(&mut self) {
if self.is_initialized() {
// Safety: The cell is initialized and being dropped, so it can't
// SAFETY: The cell is initialized and being dropped, so it can't
// be accessed again. We also don't touch the `T` other than
// dropping it, which validates our usage of #[may_dangle].
unsafe { (&mut *self.value.get()).assume_init_drop() };
Expand Down
6 changes: 4 additions & 2 deletions library/std/src/sys/windows/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ mod tests;
pub const MAIN_SEP_STR: &str = "\\";
pub const MAIN_SEP: char = '\\';

// Safety: `bytes` must be a valid wtf8 encoded slice
/// # Safety
///
/// `bytes` must be a valid wtf8 encoded slice
#[inline]
unsafe fn bytes_as_os_str(bytes: &[u8]) -> &OsStr {
// &OsStr is layout compatible with &Slice, which is compatible with &Wtf8,
Expand Down Expand Up @@ -130,7 +132,7 @@ fn parse_next_component(path: &OsStr, verbatim: bool) -> (&OsStr, &OsStr) {
// The max `separator_end` is `bytes.len()` and `bytes[bytes.len()..]` is a valid index.
let path = &path.bytes()[separator_end..];

// Safety: `path` is a valid wtf8 encoded slice and each of the separators ('/', '\')
// SAFETY: `path` is a valid wtf8 encoded slice and each of the separators ('/', '\')
// is encoded in a single byte, therefore `bytes[separator_start]` and
// `bytes[separator_end]` must be code point boundaries and thus
// `bytes[..separator_start]` and `bytes[separator_end..]` are valid wtf8 slices.
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys_common/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl StaticRWLock {
/// The lock is automatically unlocked when the returned guard is dropped.
#[inline]
pub fn read_with_guard(&'static self) -> RWLockReadGuard {
// Safety: All methods require static references, therefore self
// SAFETY: All methods require static references, therefore self
// cannot be moved between invocations.
unsafe {
self.0.read();
Expand All @@ -117,7 +117,7 @@ impl StaticRWLock {
/// The lock is automatically unlocked when the returned guard is dropped.
#[inline]
pub fn write_with_guard(&'static self) -> RWLockWriteGuard {
// Safety: All methods require static references, therefore self
// SAFETY: All methods require static references, therefore self
// cannot be moved between invocations.
unsafe {
self.0.write();
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ pub fn check(path: &Path, bad: &mut bool) {
suppressible_tidy_err!(err, skip_undocumented_unsafe, "undocumented unsafe");
}
}
if line.contains("// SAFETY:") || line.contains("// Safety:") {
if line.contains("// SAFETY:") {
last_safety_comment = true;
} else if line.trim().starts_with("//") || line.trim().is_empty() {
// keep previous value
Expand Down

0 comments on commit eefec8a

Please sign in to comment.