From 1c24955c607c285dff8091225ba5a2656756e51e Mon Sep 17 00:00:00 2001 From: Pedro Nobre Date: Thu, 3 Sep 2026 21:23:04 +0100 Subject: [PATCH] ci/refactor: added clippy to rust.yml; made minor code improvements (strongly based on clippy warnings and standards) --- .github/workflows/rust.yml | 4 ++++ src/borsh.rs | 11 ++++++----- src/lib.rs | 4 ++-- src/rawsmallvec.rs | 7 +++++++ src/taggedlen.rs | 22 +++++++++++++++------- tests/borsh.rs | 4 ++-- tests/main.rs | 27 +++++++++------------------ 7 files changed, 45 insertions(+), 34 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 6f8af7f..72614fa 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -50,6 +50,10 @@ jobs: if: matrix.nightly run: rustup component add rustfmt && cargo fmt --all --check + - name: Clippy check + if: matrix.nightly + run: rustup component add clippy && cargo clippy --all-features --all-targets -- -D warnings + - name: Build run: cargo build --verbose diff --git a/src/borsh.rs b/src/borsh.rs index 81d3833..139588a 100644 --- a/src/borsh.rs +++ b/src/borsh.rs @@ -28,28 +28,29 @@ impl BorshSerialize for SmallVec BorshDeserialize for SmallVec { fn deserialize_reader(reader: &mut R) -> Serial { let length = u64::deserialize_reader(reader)?; - return repeat_with(|| Type::deserialize_reader(reader)) + repeat_with(|| Type::deserialize_reader(reader)) .take(length.try_into().map_err(|_| Error::new( ErrorKind::OutOfMemory, "Cannot deserialize a sequence with more than usize::MAX elements in this machine" ))?) - .collect(); + .collect() } } impl BorshSchema for SmallVec { fn declaration() -> Declaration { - return format!("Vec<{}>", Type::declaration()); + format!("Vec<{}>", Type::declaration()) } - fn add_definitions_recursively(definitions: &mut Map) -> () { + fn add_definitions_recursively(definitions: &mut Map) { let declaration = Self::declaration(); if definitions.contains_key(&declaration) { return; diff --git a/src/lib.rs b/src/lib.rs index ba2302a..0aba626 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1132,7 +1132,7 @@ impl SmallVec { // We have to do this so that Miri doesn't report a "Stacked // Borrows" rule violation. See PR/406 - debug_assert!(len + 1 <= self.capacity()); + debug_assert!(len < self.capacity()); // SAFETY: we have wrote the value to the address already unsafe { self.len.increment(); @@ -1473,7 +1473,7 @@ impl SmallVec { // We have to do this so that Miri doesn't report a "Stacked // Borrows" rule violation. See PR/406 - debug_assert!(len + 1 <= self.capacity()); + debug_assert!(len < self.capacity()); // SAFETY: we have wrote the value to the address already unsafe { self.len.increment(); diff --git a/src/rawsmallvec.rs b/src/rawsmallvec.rs index 764f0c1..977c601 100644 --- a/src/rawsmallvec.rs +++ b/src/rawsmallvec.rs @@ -27,6 +27,13 @@ pub union RawSmallVec { pub heap: (NonNull, usize) } +impl Default for RawSmallVec { + #[inline] + fn default() -> Self { + Self::new() + } +} + impl RawSmallVec { const IS_ZST: bool = size_of::() == 0; diff --git a/src/taggedlen.rs b/src/taggedlen.rs index bc17d80..b89934b 100644 --- a/src/taggedlen.rs +++ b/src/taggedlen.rs @@ -3,7 +3,7 @@ use core::marker::PhantomData; /// Vec guarantees that its length is always less than [`isize::MAX`] in /// *bytes*. /// -/// For a non ZST, this means that the length is less than `isize::MAX` objects, +/// For a non-ZST, this means that the length is less than `isize::MAX` objects, /// which implies we have at least one free bit we can use. We use the least /// significant bit for the tag. And store the length in the `usize::BITS - 1` /// most significant bits. @@ -12,15 +12,12 @@ use core::marker::PhantomData; #[repr(transparent)] pub struct TaggedLen(usize, PhantomData); +// We don't use `#[derive(Clone, Copy)]` instead because `T` doesn't need to be +// `Copy` or `Clone`. impl Clone for TaggedLen { #[inline] fn clone(&self) -> Self { - Self(self.0, PhantomData) - } - - #[inline] - fn clone_from(&mut self, source: &Self) { - self.0 = source.0; + *self } } @@ -58,6 +55,12 @@ impl TaggedLen { /// Returns the same tag with the length increased by one. /// /// This increases the length without rereading the `on heap` flag. + /// + /// # Safety + /// + /// The caller must ensure that after incrementing, the length would still + /// be less than [`isize::MAX`] in bytes. For non-ZSTs this means the + /// length must be less than `isize::MAX - 1` before the call. #[inline] pub const unsafe fn increment(&mut self) { self.0 += if Self::IS_ZST { @@ -71,6 +74,11 @@ impl TaggedLen { /// Returns the same tag with the length decreased by one. /// /// This decreases the length without rereading the `on heap` flag. + /// + /// # Safety + /// + /// The caller must ensure that the length is greater than zero before the + /// call. #[inline] pub const unsafe fn decrement(&mut self) { debug_assert!(self.value() > 0); diff --git a/tests/borsh.rs b/tests/borsh.rs index 61dfee7..1c99798 100644 --- a/tests/borsh.rs +++ b/tests/borsh.rs @@ -7,7 +7,7 @@ use { }; #[test] -fn round_trip() -> () { +fn round_trip() { let smallvec = SmallVec::::from([1, 2, 3]); let bytes = to_vec(&smallvec).unwrap(); let new = SmallVec::::deserialize(&mut bytes.as_ref()).unwrap(); @@ -15,7 +15,7 @@ fn round_trip() -> () { } #[test] -fn round_trip_zst() -> () { +fn round_trip_zst() { let smallvec = SmallVec::<(), 5>::from([(); 0x100000]); let bytes = to_vec(&smallvec).unwrap(); let new = SmallVec::<(), 100>::deserialize(&mut bytes.as_ref()).unwrap(); diff --git a/tests/main.rs b/tests/main.rs index b74fd4e..c21ccd3 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -396,10 +396,7 @@ fn append() { assert_eq!(v.len(), 6); assert_eq!(n.len(), 0); - assert_eq!( - &v.iter().map(|v| *v).collect::>(), - &[0, 1, 2, 3, 5, 6] - ); + assert_eq!(v.iter().copied().collect::>(), [0, 1, 2, 3, 5, 6]); } #[test] @@ -425,20 +422,14 @@ fn extend_from_slice() { } assert_eq!(v.len(), 4); v.extend_from_slice(&[5, 6]); - assert_eq!( - &v.iter().map(|v| *v).collect::>(), - &[0, 1, 2, 3, 5, 6] - ); + assert_eq!(v.iter().copied().collect::>(), [0, 1, 2, 3, 5, 6]); } #[test] fn extend_from_within() { let mut v: SmallVec = SmallVec::from([0, 1, 2, 3]); v.extend_from_within(1..3); - assert_eq!( - &v.iter().map(|v| *v).collect::>(), - &[0, 1, 2, 3, 1, 2], - ); + assert_eq!(v.iter().copied().collect::>(), [0, 1, 2, 3, 1, 2],); } #[test] @@ -653,9 +644,9 @@ fn into_iter_as_slice() { fn into_iter_clone() { // Test that the cloned iterator yields identical elements and that it owns // its own copy (i.e. no use after move errors). - let mut iter = SmallVec::::from_iter(0..3).into_iter(); + let iter = SmallVec::::from_iter(0..3).into_iter(); let mut clone_iter = iter.clone(); - while let Some(x) = iter.next() { + for x in iter { assert_eq!(x, clone_iter.next().unwrap()); } assert_eq!(clone_iter.next(), None); @@ -665,9 +656,9 @@ fn into_iter_clone() { fn into_iter_clone_partially_consumed_iterator() { // Test that the cloned iterator only contains the remaining elements of the // original iterator. - let mut iter = SmallVec::::from_iter(0..3).into_iter().skip(1); + let iter = SmallVec::::from_iter(0..3).into_iter().skip(1); let mut clone_iter = iter.clone(); - while let Some(x) = iter.next() { + for x in iter { assert_eq!(x, clone_iter.next().unwrap()); } assert_eq!(clone_iter.next(), None); @@ -1012,7 +1003,7 @@ fn collect_from_iter() { const ELEMENTS: usize = 1000; #[cfg(not(miri))] const ELEMENTS: usize = 1_000_000; - let iter = IterNoHint(std::iter::repeat(1u8).take(ELEMENTS)); + let iter = IterNoHint(std::iter::repeat_n(1u8, ELEMENTS)); let _y: SmallVec = SmallVec::from_iter(iter); } @@ -1047,6 +1038,6 @@ fn spare_capacity_mut() { v.push(3); assert!(v.spilled()); let spare = v.spare_capacity_mut(); - assert!(spare.len() >= 1); + assert!(!spare.is_empty()); assert_eq!(spare.as_ptr().cast::(), unsafe { v.as_ptr().add(3) }); }