From 2f05c546a35d00f8caf9806bbc2a61b91242bd2e Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Sat, 17 May 2025 10:16:00 +0000 Subject: [PATCH] refactor(allocator/vec): limit scope of `#[expect(clippy::cast_possible_truncation)]` (#11074) Follow-on after #10884. Pure refactor. Move the `#[expect(clippy::cast_possible_truncation)]` attributes so they only apply to a single statement, rather than the whole function. That way we can see clearly where there's a dangerous conversion happening. Annoyingly Rust doesn't allow putting attributes on expressions, so this requires statements that can then be attributed-up. But in my view, it's worth it. --- crates/oxc_allocator/src/vec2/mod.rs | 15 ++++++++------ crates/oxc_allocator/src/vec2/raw_vec.rs | 26 ++++++++++++++++-------- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/crates/oxc_allocator/src/vec2/mod.rs b/crates/oxc_allocator/src/vec2/mod.rs index a42ccf75cd649..5207f02e9c2e9 100644 --- a/crates/oxc_allocator/src/vec2/mod.rs +++ b/crates/oxc_allocator/src/vec2/mod.rs @@ -1169,10 +1169,11 @@ impl<'bump, T: 'bump> Vec<'bump, T> { /// assert_eq!(b"aaaa", &*vec); /// ``` #[inline] - #[expect(clippy::cast_possible_truncation)] pub unsafe fn set_len(&mut self, new_len: usize) { // Caller guarantees `new_len <= u32::MAX`, so `new_len as u32` cannot truncate `new_len` - self.buf.len = new_len as u32; + #[expect(clippy::cast_possible_truncation)] + let new_len = new_len as u32; + self.buf.len = new_len; } /// Removes an element from the vector and returns it. @@ -1641,7 +1642,6 @@ impl<'bump, T: 'bump> Vec<'bump, T> { /// Caller must ensure either that `T` is `Copy`, or the elements of `other` are not accessible /// except by the pointer `other`, and that they are not read after this call. #[inline] - #[expect(clippy::cast_possible_truncation)] unsafe fn append_elements(&mut self, other: *const [T]) { let count = (*other).len(); self.reserve(count); @@ -1651,7 +1651,9 @@ impl<'bump, T: 'bump> Vec<'bump, T> { // `self.buf.len + count` cannot be `> u32::MAX`. // If either of these conditions was violated, `self.reserve(count)` above would have panicked. // So this addition cannot wrap around. - self.buf.len += count as u32; + #[expect(clippy::cast_possible_truncation)] + let count = count as u32; + self.buf.len += count; } /// Creates a draining iterator that removes the specified range in the vector @@ -2834,14 +2836,15 @@ impl Drain<'_, '_, T> { } /// Make room for inserting more elements before the tail. - #[expect(clippy::cast_possible_truncation)] unsafe fn move_tail(&mut self, extra_capacity: usize) { let vec = self.vec.as_mut(); let used_capacity = self.tail_start + self.tail_len; // `used_capacity as u32` is safe because the [`Vec::drain`] method has ensured that // `self.tail_start` is less than or equal to `self.len()`, and `self.tail_len` calculated // from `self.len() - self.tail_start`, and `self.len()` is `u32`. - vec.buf.reserve(used_capacity as u32, extra_capacity); + #[expect(clippy::cast_possible_truncation)] + let used_capacity = used_capacity as u32; + vec.buf.reserve(used_capacity, extra_capacity); let new_tail_start = self.tail_start + extra_capacity; let src = vec.as_ptr().add(self.tail_start); diff --git a/crates/oxc_allocator/src/vec2/raw_vec.rs b/crates/oxc_allocator/src/vec2/raw_vec.rs index 8d90ca8e91c40..01034f13f7e76 100644 --- a/crates/oxc_allocator/src/vec2/raw_vec.rs +++ b/crates/oxc_allocator/src/vec2/raw_vec.rs @@ -93,7 +93,6 @@ impl<'a, T> RawVec<'a, T> { RawVec::allocate_in(cap, true, a) } - #[expect(clippy::cast_possible_truncation)] fn allocate_in(cap: usize, zeroed: bool, mut a: &'a Bump) -> Self { unsafe { let elem_size = mem::size_of::(); @@ -116,7 +115,9 @@ impl<'a, T> RawVec<'a, T> { // `cap as u32` is safe because `alloc_guard` ensures that `cap` // cannot exceed `u32::MAX`. - RawVec { ptr, a, cap: cap as u32, len: 0 } + #[expect(clippy::cast_possible_truncation)] + let cap = cap as u32; + RawVec { ptr, a, cap, len: 0 } } } } @@ -134,10 +135,17 @@ impl<'a, T> RawVec<'a, T> { /// /// If all these values came from a `Vec` created in allocator `a`, then these requirements /// are guaranteed to be fulfilled. - #[expect(clippy::cast_possible_truncation)] pub unsafe fn from_raw_parts_in(ptr: *mut T, a: &'a Bump, cap: usize, len: usize) -> Self { + // SAFETY: Caller guarantees `ptr` was allocated, which implies it's not null + let ptr = unsafe { NonNull::new_unchecked(ptr) }; + // Caller guarantees `cap` and `len` are `<= u32::MAX`, so `as u32` cannot truncate them - RawVec { ptr: NonNull::new_unchecked(ptr), a, cap: cap as u32, len: len as u32 } + #[expect(clippy::cast_possible_truncation)] + let len = len as u32; + #[expect(clippy::cast_possible_truncation)] + let cap = cap as u32; + + RawVec { ptr, len, cap, a } } } @@ -630,7 +638,6 @@ impl RawVec<'_, T> { /// Helper method to reserve additional space, reallocating the backing memory. /// The caller is responsible for confirming that there is not already enough space available. - #[expect(clippy::cast_possible_truncation)] fn grow_exact(&mut self, len: u32, additional: usize) -> Result<(), CollectionAllocErr> { unsafe { // NOTE: we don't early branch on ZSTs here because we want this @@ -647,7 +654,9 @@ impl RawVec<'_, T> { // `cap as u32` is safe because `finish_grow` called `alloc_guard`, and // `alloc_guard` ensures that `cap` cannot exceed `u32::MAX`. - self.cap = new_cap as u32; + #[expect(clippy::cast_possible_truncation)] + let new_cap = new_cap as u32; + self.cap = new_cap; Ok(()) } @@ -655,7 +664,6 @@ impl RawVec<'_, T> { /// Helper method to reserve additional space, reallocating the backing memory. /// The caller is responsible for confirming that there is not already enough space available. - #[expect(clippy::cast_possible_truncation)] fn grow_amortized(&mut self, len: u32, additional: usize) -> Result<(), CollectionAllocErr> { unsafe { // NOTE: we don't early branch on ZSTs here because we want this @@ -706,7 +714,9 @@ impl RawVec<'_, T> { // `cap as u32` is safe because `finish_grow` called `alloc_guard`, and // `alloc_guard` ensures that `cap` cannot exceed `u32::MAX`. - self.cap = cap as u32; + #[expect(clippy::cast_possible_truncation)] + let cap = cap as u32; + self.cap = cap; Ok(()) }