From d7306dbbd2d25d9d924d17d9c37a12a020a351f0 Mon Sep 17 00:00:00 2001 From: Alex Martens Date: Wed, 15 Nov 2023 13:20:54 -0800 Subject: [PATCH] Fix clippy lints --- .github/workflows/build.yml | 12 +++++++++ CHANGELOG.md | 8 +++++- src/binary_heap.rs | 4 ++- src/deque.rs | 4 +-- src/histbuf.rs | 17 ++++++++++++- src/indexmap.rs | 12 ++++----- src/linear_map.rs | 1 + src/mpmc.rs | 5 +++- src/pool/arc.rs | 2 +- src/sealed.rs | 16 +++++------- src/sorted_linked_list.rs | 4 ++- src/spsc.rs | 51 +++++++++++++++++++++++++++++++------ src/string.rs | 32 +++++------------------ src/vec.rs | 14 +++++----- tests/tsan.rs | 4 +-- 15 files changed, 120 insertions(+), 66 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1e3b0ca07c..186a8a4a35 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -101,6 +101,18 @@ jobs: - name: cargo fmt --check run: cargo fmt --all -- --check + clippy: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + with: + components: clippy + targets: i686-unknown-linux-gnu + - run: cargo clippy --all --target i686-unknown-linux-gnu -- --deny warnings + # Compilation check check: name: check diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eee9068b9..accf16bf82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] -- configure `stable_deref_trait` as a platform-dependent dependency +### Changed + +- Changed `stable_deref_trait` to a platform-dependent dependency. + +### Fixed + +- Fixed clippy lints. ## [v0.8.0] - 2023-11-07 diff --git a/src/binary_heap.rs b/src/binary_heap.rs index ace9863e2d..b0602f0075 100644 --- a/src/binary_heap.rs +++ b/src/binary_heap.rs @@ -233,7 +233,7 @@ where /// assert_eq!(heap.peek(), Some(&5)); /// ``` pub fn peek(&self) -> Option<&T> { - self.data.as_slice().get(0) + self.data.as_slice().first() } /// Returns a mutable reference to the greatest item in the binary heap, or @@ -297,6 +297,7 @@ where /// Removes the *top* (greatest if max-heap, smallest if min-heap) item from the binary heap and /// returns it, without checking if the binary heap is empty. + #[allow(clippy::missing_safety_doc)] // TODO pub unsafe fn pop_unchecked(&mut self) -> T { let mut item = self.data.pop_unchecked(); @@ -330,6 +331,7 @@ where } /// Pushes an item onto the binary heap without first checking if it's full. + #[allow(clippy::missing_safety_doc)] // TODO pub unsafe fn push_unchecked(&mut self, item: T) { let old_len = self.len(); self.data.push_unchecked(item); diff --git a/src/deque.rs b/src/deque.rs index 1d3987080d..1206d66dfb 100644 --- a/src/deque.rs +++ b/src/deque.rs @@ -283,7 +283,7 @@ impl Deque { let index = self.front; self.full = false; self.front = Self::increment(self.front); - (self.buffer.get_unchecked_mut(index).as_ptr() as *const T).read() + self.buffer.get_unchecked_mut(index).as_ptr().read() } /// Removes an item from the back of the deque and returns it, without checking that the deque @@ -297,7 +297,7 @@ impl Deque { self.full = false; self.back = Self::decrement(self.back); - (self.buffer.get_unchecked_mut(self.back).as_ptr() as *const T).read() + self.buffer.get_unchecked_mut(self.back).as_ptr().read() } /// Appends an `item` to the front of the deque diff --git a/src/histbuf.rs b/src/histbuf.rs index 4ccd83b774..9a30f6fd06 100644 --- a/src/histbuf.rs +++ b/src/histbuf.rs @@ -117,6 +117,21 @@ impl HistoryBuffer { } } + /// Returns true if the buffer is empty. + /// + /// # Examples + /// + /// ``` + /// use heapless::HistoryBuffer; + /// + /// let x: HistoryBuffer = HistoryBuffer::new(); + /// assert!(x.is_empty()); + /// ``` + #[inline] + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + /// Returns the capacity of the buffer, which is the length of the /// underlying backing array. #[inline] @@ -219,7 +234,7 @@ impl HistoryBuffer { /// } /// /// ``` - pub fn oldest_ordered<'a>(&'a self) -> OldestOrdered<'a, T, N> { + pub fn oldest_ordered(&self) -> OldestOrdered<'_, T, N> { if self.filled { OldestOrdered { buf: self, diff --git a/src/indexmap.rs b/src/indexmap.rs index dffb10a452..c61624199f 100644 --- a/src/indexmap.rs +++ b/src/indexmap.rs @@ -1,7 +1,7 @@ use core::{ borrow::Borrow, fmt, - hash::{BuildHasher, Hash, Hasher as _}, + hash::{BuildHasher, Hash}, iter::FromIterator, mem, num::NonZeroU32, @@ -64,7 +64,7 @@ impl HashValue { } fn probe_distance(&self, mask: usize, current: usize) -> usize { - current.wrapping_sub(self.desired_pos(mask) as usize) & mask + current.wrapping_sub(self.desired_pos(mask)) & mask } } @@ -364,7 +364,7 @@ where fn clone(&self) -> Self { Self { entries: self.entries.clone(), - indices: self.indices.clone(), + indices: self.indices, } } } @@ -961,7 +961,7 @@ where K: Borrow, Q: ?Sized + Hash + Eq, { - if self.len() == 0 { + if self.is_empty() { return None; } let h = hash_with(key, &self.build_hasher); @@ -1238,9 +1238,7 @@ where K: ?Sized + Hash, S: BuildHasher, { - let mut h = build_hasher.build_hasher(); - key.hash(&mut h); - HashValue(h.finish() as u16) + HashValue(build_hasher.hash_one(key) as u16) } #[cfg(test)] diff --git a/src/linear_map.rs b/src/linear_map.rs index 0a3b8a3ef5..68079dce15 100644 --- a/src/linear_map.rs +++ b/src/linear_map.rs @@ -448,6 +448,7 @@ pub struct Iter<'a, K, V> { impl<'a, K, V> Iterator for Iter<'a, K, V> { type Item = (&'a K, &'a V); + #[allow(clippy::needless_borrowed_reference)] fn next(&mut self) -> Option { self.iter.next().map(|&(ref k, ref v)| (k, v)) } diff --git a/src/mpmc.rs b/src/mpmc.rs index db38934b66..cf221acbee 100644 --- a/src/mpmc.rs +++ b/src/mpmc.rs @@ -145,7 +145,8 @@ impl MpMcQueue { crate::sealed::power_of_two::(); // Const assert on size. - Self::ASSERT[!(N < (IntSize::MAX as usize)) as usize]; + #[allow(clippy::no_effect)] + Self::ASSERT[(N >= (IntSize::MAX as usize)) as usize]; let mut cell_count = 0; @@ -204,6 +205,7 @@ impl Cell { } } +#[allow(clippy::comparison_chain)] unsafe fn dequeue( buffer: *mut Cell, dequeue_pos: &AtomicTargetSize, @@ -243,6 +245,7 @@ unsafe fn dequeue( Some(data) } +#[allow(clippy::comparison_chain)] unsafe fn enqueue( buffer: *mut Cell, enqueue_pos: &AtomicTargetSize, diff --git a/src/pool/arc.rs b/src/pool/arc.rs index a5c5f2cef9..218c4d2dd6 100644 --- a/src/pool/arc.rs +++ b/src/pool/arc.rs @@ -221,7 +221,7 @@ where P: ArcPool, { fn as_ref(&self) -> &P::Data { - &**self + self } } diff --git a/src/sealed.rs b/src/sealed.rs index cb5ad840ad..674be808d0 100644 --- a/src/sealed.rs +++ b/src/sealed.rs @@ -1,29 +1,24 @@ -#[allow(dead_code)] -#[allow(path_statements)] +#[allow(dead_code, path_statements, clippy::no_effect)] pub(crate) const fn smaller_than() { Assert::::LESS; } -#[allow(dead_code)] -#[allow(path_statements)] +#[allow(dead_code, path_statements, clippy::no_effect)] pub(crate) const fn greater_than_eq_0() { Assert::::GREATER_EQ; } -#[allow(dead_code)] -#[allow(path_statements)] +#[allow(dead_code, path_statements, clippy::no_effect)] pub(crate) const fn greater_than_0() { Assert::::GREATER; } -#[allow(dead_code)] -#[allow(path_statements)] +#[allow(dead_code, path_statements, clippy::no_effect)] pub(crate) const fn greater_than_1() { Assert::::GREATER; } -#[allow(dead_code)] -#[allow(path_statements)] +#[allow(dead_code, path_statements, clippy::no_effect)] pub(crate) const fn power_of_two() { Assert::::GREATER; Assert::::POWER_OF_TWO; @@ -42,6 +37,7 @@ impl Assert { pub const LESS_EQ: usize = R - L; /// Const assert hack + #[allow(clippy::erasing_op)] pub const NOT_EQ: isize = 0 / (R as isize - L as isize); /// Const assert hack diff --git a/src/sorted_linked_list.rs b/src/sorted_linked_list.rs index 041895cd3e..9adbafcc4b 100644 --- a/src/sorted_linked_list.rs +++ b/src/sorted_linked_list.rs @@ -310,7 +310,8 @@ where /// ``` pub fn push(&mut self, value: T) -> Result<(), T> { if !self.is_full() { - Ok(unsafe { self.push_unchecked(value) }) + unsafe { self.push_unchecked(value) } + Ok(()) } else { Err(value) } @@ -462,6 +463,7 @@ where /// assert_eq!(ll.pop(), Ok(1)); /// assert_eq!(ll.pop(), Err(())); /// ``` + #[allow(clippy::result_unit_err)] pub fn pop(&mut self) -> Result { if !self.is_empty() { Ok(unsafe { self.pop_unchecked() }) diff --git a/src/spsc.rs b/src/spsc.rs index c8085720e7..0222d31d26 100644 --- a/src/spsc.rs +++ b/src/spsc.rs @@ -251,7 +251,7 @@ impl Queue { /// Adds an `item` to the end of the queue, without checking if it's full /// - /// # Unsafety + /// # Safety /// /// If the queue is full this operation will leak a value (T's destructor won't run on /// the value that got overwritten by `item`), *and* will allow the `dequeue` operation @@ -295,7 +295,7 @@ impl Queue { /// Returns the item in the front of the queue, without checking if there is something in the /// queue /// - /// # Unsafety + /// # Safety /// /// If the queue is empty this operation will return uninitialized memory. pub unsafe fn dequeue_unchecked(&mut self) -> T { @@ -507,7 +507,9 @@ impl<'a, T, const N: usize> Consumer<'a, T, N> { /// Returns the item in the front of the queue, without checking if there are elements in the /// queue /// - /// See [`Queue::dequeue_unchecked`] for safety + /// # Safety + /// + /// See [`Queue::dequeue_unchecked`] #[inline] pub unsafe fn dequeue_unchecked(&mut self) -> T { self.rb.inner_dequeue_unchecked() @@ -526,6 +528,22 @@ impl<'a, T, const N: usize> Consumer<'a, T, N> { self.rb.len() } + /// Returns true if the queue is empty + /// + /// # Examples + /// + /// ``` + /// use heapless::spsc::Queue; + /// + /// let mut queue: Queue = Queue::new(); + /// let (mut producer, mut consumer) = queue.split(); + /// assert!(consumer.is_empty()); + /// ``` + #[inline] + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + /// Returns the maximum number of elements the queue can hold #[inline] pub fn capacity(&self) -> usize { @@ -536,6 +554,7 @@ impl<'a, T, const N: usize> Consumer<'a, T, N> { /// empty /// /// # Examples + /// /// ``` /// use heapless::spsc::Queue; /// @@ -562,7 +581,9 @@ impl<'a, T, const N: usize> Producer<'a, T, N> { /// Adds an `item` to the end of the queue, without checking if the queue is full /// - /// See [`Queue::enqueue_unchecked`] for safety + /// # Safety + /// + /// See [`Queue::enqueue_unchecked`] #[inline] pub unsafe fn enqueue_unchecked(&mut self, val: T) { self.rb.inner_enqueue_unchecked(val) @@ -581,6 +602,22 @@ impl<'a, T, const N: usize> Producer<'a, T, N> { self.rb.len() } + /// Returns true if the queue is empty + /// + /// # Examples + /// + /// ``` + /// use heapless::spsc::Queue; + /// + /// let mut queue: Queue = Queue::new(); + /// let (mut producer, mut consumer) = queue.split(); + /// assert!(producer.is_empty()); + /// ``` + #[inline] + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + /// Returns the maximum number of elements the queue can hold #[inline] pub fn capacity(&self) -> usize { @@ -894,14 +931,12 @@ mod tests { let hash1 = { let mut hasher1 = hash32::FnvHasher::default(); rb1.hash(&mut hasher1); - let hash1 = hasher1.finish(); - hash1 + hasher1.finish() }; let hash2 = { let mut hasher2 = hash32::FnvHasher::default(); rb2.hash(&mut hasher2); - let hash2 = hasher2.finish(); - hash2 + hasher2.finish() }; assert_eq!(hash1, hash2); } diff --git a/src/string.rs b/src/string.rs index 086f21e80d..be3456348e 100644 --- a/src/string.rs +++ b/src/string.rs @@ -209,6 +209,7 @@ impl String { /// # Ok::<(), ()>(()) /// ``` #[inline] + #[allow(clippy::result_unit_err)] pub fn push_str(&mut self, string: &str) -> Result<(), ()> { self.vec.extend_from_slice(string.as_bytes()) } @@ -251,6 +252,7 @@ impl String { /// # Ok::<(), ()>(()) /// ``` #[inline] + #[allow(clippy::result_unit_err)] pub fn push(&mut self, c: char) -> Result<(), ()> { match c.len_utf8() { 1 => self.vec.push(c as u8).map_err(|_| {}), @@ -315,7 +317,7 @@ impl String { /// Ok::<(), ()>(()) /// ``` pub fn pop(&mut self) -> Option { - let ch = self.chars().rev().next()?; + let ch = self.chars().next_back()?; // pop bytes that correspond to `ch` for _ in 0..ch.len_utf8() { @@ -518,21 +520,13 @@ impl PartialEq> for String { fn eq(&self, rhs: &String) -> bool { str::eq(&**self, &**rhs) } - - fn ne(&self, rhs: &String) -> bool { - str::ne(&**self, &**rhs) - } } // String == str impl PartialEq for String { #[inline] fn eq(&self, other: &str) -> bool { - str::eq(&self[..], &other[..]) - } - #[inline] - fn ne(&self, other: &str) -> bool { - str::ne(&self[..], &other[..]) + str::eq(self, other) } } @@ -540,11 +534,7 @@ impl PartialEq for String { impl PartialEq<&str> for String { #[inline] fn eq(&self, other: &&str) -> bool { - str::eq(&self[..], &other[..]) - } - #[inline] - fn ne(&self, other: &&str) -> bool { - str::ne(&self[..], &other[..]) + str::eq(self, &other[..]) } } @@ -552,11 +542,7 @@ impl PartialEq<&str> for String { impl PartialEq> for str { #[inline] fn eq(&self, other: &String) -> bool { - str::eq(&self[..], &other[..]) - } - #[inline] - fn ne(&self, other: &String) -> bool { - str::ne(&self[..], &other[..]) + str::eq(self, &other[..]) } } @@ -564,11 +550,7 @@ impl PartialEq> for str { impl PartialEq> for &str { #[inline] fn eq(&self, other: &String) -> bool { - str::eq(&self[..], &other[..]) - } - #[inline] - fn ne(&self, other: &String) -> bool { - str::ne(&self[..], &other[..]) + str::eq(self, &other[..]) } } diff --git a/src/vec.rs b/src/vec.rs index a669e90f81..6fb27cc57d 100644 --- a/src/vec.rs +++ b/src/vec.rs @@ -76,6 +76,7 @@ impl Vec { /// v.extend_from_slice(&[1, 2, 3]).unwrap(); /// ``` #[inline] + #[allow(clippy::result_unit_err)] pub fn from_slice(other: &[T]) -> Result where T: Clone, @@ -210,6 +211,7 @@ impl Vec { /// vec.extend_from_slice(&[2, 3, 4]).unwrap(); /// assert_eq!(*vec, [1, 2, 3, 4]); /// ``` + #[allow(clippy::result_unit_err)] pub fn extend_from_slice(&mut self, other: &[T]) -> Result<(), ()> where T: Clone, @@ -257,7 +259,7 @@ impl Vec { debug_assert!(!self.is_empty()); self.len -= 1; - (self.buffer.get_unchecked_mut(self.len).as_ptr() as *const T).read() + self.buffer.get_unchecked_mut(self.len).as_ptr().read() } /// Appends an `item` to the back of the collection @@ -305,6 +307,7 @@ impl Vec { /// new_len is less than len, the Vec is simply truncated. /// /// See also [`resize_default`](Self::resize_default). + #[allow(clippy::result_unit_err)] pub fn resize(&mut self, new_len: usize, value: T) -> Result<(), ()> where T: Clone, @@ -331,6 +334,7 @@ impl Vec { /// If `new_len` is less than `len`, the `Vec` is simply truncated. /// /// See also [`resize`](Self::resize). + #[allow(clippy::result_unit_err)] pub fn resize_default(&mut self, new_len: usize) -> Result<(), ()> where T: Clone + Default, @@ -934,9 +938,7 @@ impl Iterator for IntoIter { type Item = T; fn next(&mut self) -> Option { if self.next < self.vec.len() { - let item = unsafe { - (self.vec.buffer.get_unchecked_mut(self.next).as_ptr() as *const T).read() - }; + let item = unsafe { self.vec.buffer.get_unchecked_mut(self.next).as_ptr().read() }; self.next += 1; Some(item) } else { @@ -1001,7 +1003,7 @@ where A: PartialEq, { fn eq(&self, other: &[B]) -> bool { - <[A]>::eq(self, &other[..]) + <[A]>::eq(self, other) } } @@ -1011,7 +1013,7 @@ where A: PartialEq, { fn eq(&self, other: &Vec) -> bool { - <[A]>::eq(other, &self[..]) + <[A]>::eq(other, self) } } diff --git a/tests/tsan.rs b/tests/tsan.rs index 74629ead2c..043c24578e 100644 --- a/tests/tsan.rs +++ b/tests/tsan.rs @@ -88,7 +88,7 @@ fn contention() { for i in 0..(2 * N) { sum = sum.wrapping_add(i as u32); - while let Err(_) = p.enqueue(i as u8) {} + while p.enqueue(i as u8).is_err() {} } println!("producer: {}", sum); @@ -137,7 +137,7 @@ fn mpmc_contention() { for i in 0..(16 * N) { sum = sum.wrapping_add(i); println!("enqueue {}", i); - while let Err(_) = Q.enqueue(i) {} + while Q.enqueue(i).is_err() {} } s1.send(sum).unwrap();