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

chore(allocator, data_structures, lexer, traverse): enable undocumented_unsafe_blocks clippy lint #9363

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading