Skip to content

Commit

Permalink
chore(allocator, data_structures, lexer, traverse): enable `undocumen…
Browse files Browse the repository at this point in the history
…ted_unsafe_blocks` clippy lint (#9363)

Enable `undocumented_unsafe_blocks` clippy lint rule, and fix warnings it raises.
  • Loading branch information
overlookmotel committed Feb 26, 2025
1 parent 3c35485 commit 4f2fc39
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 8 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ rc_buffer = "warn"
rc_mutex = "warn"
rest_pat_in_fully_bound_structs = "warn"
unnecessary_safety_comment = "warn"
undocumented_unsafe_blocks = "allow" # FIXME
undocumented_unsafe_blocks = "warn"
infinite_loop = "warn"
map_with_unused_argument_over_ranges = "warn"
unused_result_ok = "warn"
Expand Down
5 changes: 3 additions & 2 deletions crates/oxc_allocator/src/allocator_api2.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// All methods just delegate to `bumpalo`, so all marked `#[inline(always)]`
#![expect(clippy::inline_always)]
// All methods just delegate to `bumpalo`, so all marked `#[inline(always)]`.
// All have same safety preconditions of `bumpalo` methods of the same name.
#![expect(clippy::inline_always, clippy::undocumented_unsafe_blocks)]

use std::{alloc::Layout, ptr::NonNull};

Expand Down
2 changes: 2 additions & 0 deletions crates/oxc_data_structures/src/stack/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ pub trait StackCommon<T>: StackCapacity<T> {
let old_layout = unsafe { Self::layout_for(self.capacity_bytes()) };

// Grow allocation.
// SAFETY:
// `start` and `end` are boundaries of the allocation (`alloc` and `grow` ensure that).
// So `old_start_ptr` and `old_layout` accurately describe the current allocation.
// `grow` creates new allocation with byte size double what it currently is, or caps it
Expand Down Expand Up @@ -270,6 +271,7 @@ unsafe fn grow(
// `layout_for` produces a layout with `T`'s alignment, so `new_ptr` is aligned for `T`.
let new_ptr = unsafe { alloc::realloc(old_start.as_ptr(), old_layout, new_capacity_bytes) };
let Some(new_start) = NonNull::new(new_ptr) else {
// SAFETY: See above
let new_layout =
unsafe { Layout::from_size_align_unchecked(new_capacity_bytes, old_layout.align()) };
alloc::handle_alloc_error(new_layout);
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_parser/src/lexer/byte_handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ impl Lexer<'_> {
/// * `byte` must be next byte of source code, corresponding to current position of `lexer.source`.
/// * Only `BYTE_HANDLERS` for ASCII characters may use the `ascii_byte_handler!()` macro.
pub(super) unsafe fn handle_byte(&mut self, byte: u8) -> Kind {
// SAFETY: Caller guarantees to uphold safety invariants
unsafe { BYTE_HANDLERS[byte as usize](self) }
}
}
Expand Down
17 changes: 12 additions & 5 deletions crates/oxc_parser/src/lexer/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,10 @@ impl<'a> Source<'a> {
debug_assert!(end.ptr <= self.end);
// Check `start` and `end` are on UTF-8 character boundaries.
// SAFETY: Above assertions ensure `start` and `end` are valid to read from if not at EOF.
debug_assert!(start.ptr == self.end || !is_utf8_cont_byte(unsafe { start.read() }));
debug_assert!(end.ptr == self.end || !is_utf8_cont_byte(unsafe { end.read() }));
unsafe {
debug_assert!(start.ptr == self.end || !is_utf8_cont_byte(start.read()));
debug_assert!(end.ptr == self.end || !is_utf8_cont_byte(end.read()));
}

// SAFETY: Caller guarantees `start` is not after `end`.
// `SourcePosition`s can only be created from a `Source`.
Expand Down Expand Up @@ -541,12 +543,13 @@ impl<'a> Source<'a> {
/// Caller must ensure `Source` is not at end of file.
#[inline]
pub(super) unsafe fn peek_byte_unchecked(&self) -> u8 {
debug_assert!(self.ptr >= self.start && self.ptr < self.end);

// SAFETY: Caller guarantees `ptr` is before `end` (i.e. not at end of file).
// Methods of this type provide no way to allow `ptr` to be before `start`.
// `Source`'s invariants guarantee that `self.start` - `self.end` contains allocated memory.
// `Source::new` takes an immutable ref `&str`, guaranteeing that the memory `self.ptr`
// addresses cannot be aliased by a `&mut` ref as long as `Source` exists.
debug_assert!(self.ptr >= self.start && self.ptr < self.end);
unsafe { self.position().read() }
}
}
Expand Down Expand Up @@ -592,6 +595,7 @@ impl SourcePosition<'_> {
/// just not past it.
#[inline]
pub(super) unsafe fn add(self, n: usize) -> Self {
// SAFETY: Caller guarantees that `add` will not go out of bounds
unsafe { Self::new(self.ptr.add(n)) }
}

Expand All @@ -603,6 +607,7 @@ impl SourcePosition<'_> {
/// of `Source` this `SourcePosition` was created from.
#[inline]
pub(super) unsafe fn sub(self, n: usize) -> Self {
// SAFETY: Caller guarantees that `sub` will not go out of bounds
unsafe { Self::new(self.ptr.sub(n)) }
}

Expand All @@ -623,14 +628,15 @@ impl SourcePosition<'_> {
/// because if a `&mut` reference existed, that would violate Rust's aliasing rules.
#[inline]
pub(super) unsafe fn read(self) -> u8 {
debug_assert!(!self.ptr.is_null());

// SAFETY:
// Caller guarantees `self` is not at end of source text.
// `Source` is created from a valid `&str`, so points to allocated, initialized memory.
// `Source` conceptually holds the source text `&str`, which guarantees no mutable references
// to the same memory can exist, as that would violate Rust's aliasing rules.
// Pointer is "dereferenceable" by definition as a `u8` is 1 byte and cannot span multiple objects.
// Alignment is not relevant as `u8` is aligned on 1 (i.e. no alignment requirements).
debug_assert!(!self.ptr.is_null());
unsafe { *self.ptr.as_ref().unwrap_unchecked() }
}

Expand All @@ -641,14 +647,15 @@ impl SourcePosition<'_> {
/// i.e. if source length is 10, `self` must be on position 8 max.
#[inline]
pub(super) unsafe fn read2(self) -> [u8; 2] {
debug_assert!(!self.ptr.is_null());

// SAFETY:
// Caller guarantees `self` is not at no later than 2 bytes before end of source text.
// `Source` is created from a valid `&str`, so points to allocated, initialized memory.
// `Source` conceptually holds the source text `&str`, which guarantees no mutable references
// to the same memory can exist, as that would violate Rust's aliasing rules.
// Pointer is "dereferenceable" by definition as a `u8` is 1 byte and cannot span multiple objects.
// Alignment is not relevant as `u8` is aligned on 1 (i.e. no alignment requirements).
debug_assert!(!self.ptr.is_null());
#[expect(clippy::ptr_as_ptr)]
unsafe {
let p = self.ptr as *const [u8; 2];
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_traverse/src/context/ancestry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ impl<'a> TraverseAncestry<'a> {
#[expect(clippy::ptr_as_ptr, clippy::ref_as_ptr)]
pub(crate) unsafe fn retag_stack(&mut self, ty: AncestorType) {
debug_assert!(self.stack.len() >= 2);
// SAFETY: Caller must uphold the safety invariants
unsafe { *(self.stack.last_mut() as *mut _ as *mut AncestorType) = ty };
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_traverse/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,7 @@ impl<'a> TraverseCtx<'a> {
/// This method must not be public outside this crate, or consumer could break safety invariants.
#[inline]
pub(crate) unsafe fn retag_stack(&mut self, ty: AncestorType) {
// SAFETY: Caller muct uphold safety constraints
unsafe { self.ancestry.retag_stack(ty) };
}

Expand Down

0 comments on commit 4f2fc39

Please sign in to comment.