From ccf28965113c4d385cb466ffffb889f1b90b53c1 Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Fri, 19 Sep 2025 11:18:24 +0200 Subject: [PATCH 1/3] Remove implementing Deref for HistoryBuf There is already `as_slice()` which gives access to the underlying backing array where the elements are not in the order of writing. With automatic dereferencing, functions like `iter()` or `windows()` are automatically available for `HistoryBuf` and are returning items in an order which does not reflect the write order and will likely surprise users which did not carefully read all the documentation on for `HistoryBuf` like me. --- CHANGELOG.md | 1 + src/history_buf.rs | 18 ++++-------------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddb1d69bad..61648238b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Implement `defmt::Format` for `CapacityError`. - Implement `TryFrom` for `Deque` from array. - Switch from `serde` to `serde_core` for enabling faster compilations. +- Removed `impl Deref` for `HistoryBuf` to make accessing the raw backing array explicit (use `as_slice`). ## [v0.9.1] - 2025-08-19 diff --git a/src/history_buf.rs b/src/history_buf.rs index b06e4648b9..1249b5e41c 100644 --- a/src/history_buf.rs +++ b/src/history_buf.rs @@ -34,7 +34,6 @@ use core::fmt; use core::marker::PhantomData; use core::mem::MaybeUninit; -use core::ops::Deref; use core::ptr; use core::slice; @@ -572,18 +571,10 @@ impl + ?Sized> Drop for HistoryBufInner { } } -impl + ?Sized> Deref for HistoryBufInner { - type Target = [T]; - - fn deref(&self) -> &[T] { - self.as_slice() - } -} - impl + ?Sized> AsRef<[T]> for HistoryBufInner { #[inline] fn as_ref(&self) -> &[T] { - self + self.as_slice() } } @@ -592,7 +583,7 @@ where T: fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - <[T] as fmt::Debug>::fmt(self, f) + <[T] as fmt::Debug>::fmt(self.as_slice(), f) } } @@ -660,7 +651,6 @@ mod tests { let x: HistoryBuf = HistoryBuf::new_with(1); assert_eq!(x.len(), 4); assert_eq!(x.as_slice(), [1; 4]); - assert_eq!(*x, [1; 4]); assert!(x.is_full()); let x: HistoryBuf = HistoryBuf::new(); @@ -910,8 +900,8 @@ mod tests { x, y, "{:?} {:?}", - x.iter().collect::>(), - y.iter().collect::>() + x.as_slice().iter().collect::>(), + y.as_slice().iter().collect::>() ); } } From b2c7e262443175409c972d9c5f9bbc4198cad60e Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Fri, 19 Sep 2025 11:58:52 +0200 Subject: [PATCH 2/3] Replace HistoryBuf::as_slice() to as_unordered_slice() Some method names already contain statements on ordering like `oldest_ordered()`. In this context a method returning an unordered slice should state this clearly. --- CHANGELOG.md | 1 + src/history_buf.rs | 84 ++++++++++++++++++++++++++++++---------------- 2 files changed, 56 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61648238b8..64741ed2fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Implement `TryFrom` for `Deque` from array. - Switch from `serde` to `serde_core` for enabling faster compilations. - Removed `impl Deref` for `HistoryBuf` to make accessing the raw backing array explicit (use `as_slice`). +- Replace `HistoryBuf::as_slice` with `HistoryBuf::as_unordered_slice` to clarify ordering. ## [v0.9.1] - 2025-08-19 diff --git a/src/history_buf.rs b/src/history_buf.rs index 1249b5e41c..23ea7cb0c0 100644 --- a/src/history_buf.rs +++ b/src/history_buf.rs @@ -21,13 +21,13 @@ //! // The most recent written element is a four. //! assert_eq!(buf.recent(), Some(&4)); //! -//! // To access all elements in an unspecified order, use `as_slice()`. -//! for el in buf.as_slice() { +//! // To access all elements in an unspecified order, use `as_unordered_slice()`. +//! for el in buf.as_unordered_slice() { //! println!("{:?}", el); //! } //! //! // Now we can prepare an average of all values, which comes out to 4. -//! let avg = buf.as_slice().iter().sum::() / buf.len(); +//! let avg = buf.as_unordered_slice().iter().sum::() / buf.len(); //! assert_eq!(avg, 4); //! ``` @@ -178,13 +178,13 @@ pub struct HistoryBufInner + ?Sized> { /// // The most recent written element is a four. /// assert_eq!(buf.recent(), Some(&4)); /// -/// // To access all elements in an unspecified order, use `as_slice()`. -/// for el in buf.as_slice() { +/// // To access all elements in an unspecified order, use `as_unordered_slice()`. +/// for el in buf.as_unordered_slice() { /// println!("{:?}", el); /// } /// /// // Now we can prepare an average of all values, which comes out to 4. -/// let avg = buf.as_slice().iter().sum::() / buf.len(); +/// let avg = buf.as_unordered_slice().iter().sum::() / buf.len(); /// assert_eq!(avg, 4); /// ``` pub type HistoryBuf = HistoryBufInner>; @@ -211,13 +211,13 @@ pub type HistoryBuf = HistoryBufInner() / buf.len(); +/// let avg = buf.as_unordered_slice().iter().sum::() / buf.len(); /// assert_eq!(avg, 4); /// ``` pub type HistoryBufView = HistoryBufInner>; @@ -269,7 +269,7 @@ where /// // Allocate a 16-element buffer on the stack /// let mut x: HistoryBuf = HistoryBuf::new_with(4); /// // All elements are four - /// assert_eq!(x.as_slice(), [4; 16]); + /// assert_eq!(x.as_unordered_slice(), [4; 16]); /// ``` #[inline] pub fn new_with(t: T) -> Self { @@ -478,7 +478,16 @@ impl + ?Sized> HistoryBufInner { /// Returns the array slice backing the buffer, without keeping track /// of the write position. Therefore, the element order is unspecified. + #[deprecated( + note = "as_slice's name did not explicitly state unspecified ordering of elements. Use as_unordered_slice instead." + )] pub fn as_slice(&self) -> &[T] { + self.as_unordered_slice() + } + + /// Returns the array slice backing the buffer, without keeping track + /// of the write position. Therefore, the element order is unspecified. + pub fn as_unordered_slice(&self) -> &[T] { unsafe { slice::from_raw_parts(self.data.borrow().as_ptr().cast(), self.len()) } } @@ -495,7 +504,7 @@ impl + ?Sized> HistoryBufInner { /// assert_eq!(buffer.as_slices(), (&[1, 2, 3][..], &[4, 5, 6][..])); /// ``` pub fn as_slices(&self) -> (&[T], &[T]) { - let buffer = self.as_slice(); + let buffer = self.as_unordered_slice(); if self.filled { (&buffer[self.write_at..], &buffer[..self.write_at]) @@ -556,7 +565,12 @@ where { fn clone(&self) -> Self { let mut ret = Self::new(); - for (new, old) in ret.data.borrow_mut().iter_mut().zip(self.as_slice()) { + for (new, old) in ret + .data + .borrow_mut() + .iter_mut() + .zip(self.as_unordered_slice()) + { new.write(old.clone()); } ret.filled = self.filled; @@ -574,7 +588,7 @@ impl + ?Sized> Drop for HistoryBufInner { impl + ?Sized> AsRef<[T]> for HistoryBufInner { #[inline] fn as_ref(&self) -> &[T] { - self.as_slice() + self.as_unordered_slice() } } @@ -583,7 +597,7 @@ where T: fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - <[T] as fmt::Debug>::fmt(self.as_slice(), f) + <[T] as fmt::Debug>::fmt(self.as_unordered_slice(), f) } } @@ -650,11 +664,11 @@ mod tests { fn new() { let x: HistoryBuf = HistoryBuf::new_with(1); assert_eq!(x.len(), 4); - assert_eq!(x.as_slice(), [1; 4]); + assert_eq!(x.as_unordered_slice(), [1; 4]); assert!(x.is_full()); let x: HistoryBuf = HistoryBuf::new(); - assert_eq!(x.as_slice(), []); + assert_eq!(x.as_unordered_slice(), []); assert!(!x.is_full()); } @@ -663,33 +677,33 @@ mod tests { let mut x: HistoryBuf = HistoryBuf::new(); x.write(1); x.write(4); - assert_eq!(x.as_slice(), [1, 4]); + assert_eq!(x.as_unordered_slice(), [1, 4]); x.write(5); x.write(6); x.write(10); - assert_eq!(x.as_slice(), [10, 4, 5, 6]); + assert_eq!(x.as_unordered_slice(), [10, 4, 5, 6]); x.extend([11, 12].iter()); - assert_eq!(x.as_slice(), [10, 11, 12, 6]); + assert_eq!(x.as_unordered_slice(), [10, 11, 12, 6]); } #[test] fn clear() { let mut x: HistoryBuf = HistoryBuf::new_with(1); x.clear(); - assert_eq!(x.as_slice(), []); + assert_eq!(x.as_unordered_slice(), []); let mut x: HistoryBuf = HistoryBuf::new(); x.clear_with(1); - assert_eq!(x.as_slice(), [1; 4]); + assert_eq!(x.as_unordered_slice(), [1; 4]); } #[test] fn clone() { let mut x: HistoryBuf = HistoryBuf::new(); for i in 0..10 { - assert_eq!(x.as_slice(), x.clone().as_slice()); + assert_eq!(x.as_unordered_slice(), x.clone().as_unordered_slice()); x.write(i); } @@ -710,17 +724,17 @@ mod tests { assert_eq!(GLOBAL.load(Ordering::Relaxed), 0); y.write(InstrumentedClone(0)); assert_eq!(GLOBAL.load(Ordering::Relaxed), 0); - assert_eq!(y.clone().as_slice(), [InstrumentedClone(1)]); + assert_eq!(y.clone().as_unordered_slice(), [InstrumentedClone(1)]); assert_eq!(GLOBAL.load(Ordering::Relaxed), 1); y.write(InstrumentedClone(0)); assert_eq!(GLOBAL.load(Ordering::Relaxed), 1); assert_eq!( - y.clone().as_slice(), + y.clone().as_unordered_slice(), [InstrumentedClone(1), InstrumentedClone(1)] ); assert_eq!(GLOBAL.load(Ordering::Relaxed), 3); assert_eq!( - y.clone().clone().clone().as_slice(), + y.clone().clone().clone().as_unordered_slice(), [InstrumentedClone(3), InstrumentedClone(3)] ); assert_eq!(GLOBAL.load(Ordering::Relaxed), 9); @@ -762,15 +776,27 @@ mod tests { assert_eq!(x.oldest(), Some(&4)); } + #[allow(deprecated)] #[test] fn as_slice() { let mut x: HistoryBuf = HistoryBuf::new(); - assert_eq!(x.as_slice(), []); + assert_eq!(x.as_slice(), x.as_unordered_slice()); + + x.extend([1, 2, 3, 4, 5].iter()); + + assert_eq!(x.as_slice(), x.as_unordered_slice()); + } + + #[test] + fn as_unordered_slice() { + let mut x: HistoryBuf = HistoryBuf::new(); + + assert_eq!(x.as_unordered_slice(), []); x.extend([1, 2, 3, 4, 5].iter()); - assert_eq!(x.as_slice(), [5, 2, 3, 4]); + assert_eq!(x.as_unordered_slice(), [5, 2, 3, 4]); } /// Test whether `.as_slices()` behaves as expected. @@ -900,8 +926,8 @@ mod tests { x, y, "{:?} {:?}", - x.as_slice().iter().collect::>(), - y.as_slice().iter().collect::>() + x.as_unordered_slice().iter().collect::>(), + y.as_unordered_slice().iter().collect::>() ); } } From 05a15b13cdb6c5d72ec998d4354c9151292b2705 Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Fri, 19 Sep 2025 14:28:33 +0200 Subject: [PATCH 3/3] Replace HistoryBuf::write with push HistoryBuf already uses extend in method names for ingesting multiple items at once and other heapless containers use push for methods ingesting a single item. Let's be more consistent with this naming scheme by switching from write to push here as well. --- CHANGELOG.md | 1 + src/de.rs | 2 +- src/history_buf.rs | 121 ++++++++++++++++++++++++++++----------------- 3 files changed, 78 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64741ed2fd..ceb81970b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Switch from `serde` to `serde_core` for enabling faster compilations. - Removed `impl Deref` for `HistoryBuf` to make accessing the raw backing array explicit (use `as_slice`). - Replace `HistoryBuf::as_slice` with `HistoryBuf::as_unordered_slice` to clarify ordering. +- Replace `HistoryBuf::write` with `HistoryBuf::push` for a more consistent naming scheme. ## [v0.9.1] - 2025-08-19 diff --git a/src/de.rs b/src/de.rs index bf1d1ecc48..bf62ec24bf 100644 --- a/src/de.rs +++ b/src/de.rs @@ -201,7 +201,7 @@ where let mut values = HistoryBuf::new(); while let Some(value) = seq.next_element()? { - values.write(value); + values.push(value); } Ok(values) diff --git a/src/history_buf.rs b/src/history_buf.rs index 23ea7cb0c0..466cfc4b22 100644 --- a/src/history_buf.rs +++ b/src/history_buf.rs @@ -1,6 +1,6 @@ //! A "history buffer", similar to a write-only ring buffer of fixed length. //! -//! This buffer keeps a fixed number of elements. On write, the oldest element +//! This buffer keeps a fixed number of elements. On push, the oldest element //! is overwritten. Thus, the buffer is useful to keep a history of values with //! some desired depth, and for example calculate a rolling average. //! @@ -14,8 +14,8 @@ //! // Starts with no data //! assert_eq!(buf.recent(), None); //! -//! buf.write(3); -//! buf.write(5); +//! buf.push(3); +//! buf.push(5); //! buf.extend(&[4, 4]); //! //! // The most recent written element is a four. @@ -157,7 +157,7 @@ pub struct HistoryBufInner + ?Sized> { /// A "history buffer", similar to a write-only ring buffer of fixed length. /// -/// This buffer keeps a fixed number of elements. On write, the oldest element +/// This buffer keeps a fixed number of elements. On push, the oldest element /// is overwritten. Thus, the buffer is useful to keep a history of values with /// some desired depth, and for example calculate a rolling average. /// @@ -171,8 +171,8 @@ pub struct HistoryBufInner + ?Sized> { /// // Starts with no data /// assert_eq!(buf.recent(), None); /// -/// buf.write(3); -/// buf.write(5); +/// buf.push(3); +/// buf.push(5); /// buf.extend(&[4, 4]); /// /// // The most recent written element is a four. @@ -204,8 +204,8 @@ pub type HistoryBuf = HistoryBufInner + ?Sized> HistoryBufInner { } /// Writes an element to the buffer, overwriting the oldest value. + #[deprecated(note = "Superseeded by push for harmonizing naming with other containers.")] pub fn write(&mut self, t: T) { + self.push(t); + } + + /// Pushes an element to the buffer, overwriting the oldest value. + pub fn push(&mut self, t: T) { if self.filled { // Drop the old before we overwrite it. unsafe { ptr::drop_in_place(self.data.borrow_mut()[self.write_at].as_mut_ptr()) } @@ -392,7 +398,7 @@ impl + ?Sized> HistoryBufInner { T: Clone, { for item in other { - self.write(item.clone()); + self.push(item.clone()); } } @@ -404,8 +410,8 @@ impl + ?Sized> HistoryBufInner { /// use heapless::HistoryBuf; /// /// let mut x: HistoryBuf = HistoryBuf::new(); - /// x.write(4); - /// x.write(10); + /// x.push(4); + /// x.push(10); /// assert_eq!(x.recent(), Some(&10)); /// ``` pub fn recent(&self) -> Option<&T> { @@ -421,8 +427,8 @@ impl + ?Sized> HistoryBufInner { /// use heapless::HistoryBuf; /// /// let mut x: HistoryBuf = HistoryBuf::new(); - /// x.write(4); - /// x.write(10); + /// x.push(4); + /// x.push(10); /// assert_eq!(x.recent_index(), Some(1)); /// ``` pub fn recent_index(&self) -> Option { @@ -445,8 +451,8 @@ impl + ?Sized> HistoryBufInner { /// use heapless::HistoryBuf; /// /// let mut x: HistoryBuf = HistoryBuf::new(); - /// x.write(4); - /// x.write(10); + /// x.push(4); + /// x.push(10); /// assert_eq!(x.oldest(), Some(&4)); /// ``` pub fn oldest(&self) -> Option<&T> { @@ -462,8 +468,8 @@ impl + ?Sized> HistoryBufInner { /// use heapless::HistoryBuf; /// /// let mut x: HistoryBuf = HistoryBuf::new(); - /// x.write(4); - /// x.write(10); + /// x.push(4); + /// x.push(10); /// assert_eq!(x.oldest_index(), Some(0)); /// ``` pub fn oldest_index(&self) -> Option { @@ -542,7 +548,7 @@ impl + ?Sized> Extend for HistoryBufInner { I: IntoIterator, { for item in iter.into_iter() { - self.write(item); + self.push(item); } } } @@ -672,16 +678,41 @@ mod tests { assert!(!x.is_full()); } + #[allow(deprecated)] #[test] fn write() { + let mut write: HistoryBuf = HistoryBuf::new(); + let mut push: HistoryBuf = HistoryBuf::new(); + + write.write(1); + write.write(4); + push.push(1); + push.push(4); + assert_eq!(write, push); + + write.push(5); + write.push(6); + write.push(10); + push.push(5); + push.push(6); + push.push(10); + assert_eq!(write, push); + + write.extend([11, 12].iter()); + push.extend([11, 12].iter()); + assert_eq!(write, push); + } + + #[test] + fn push() { let mut x: HistoryBuf = HistoryBuf::new(); - x.write(1); - x.write(4); + x.push(1); + x.push(4); assert_eq!(x.as_unordered_slice(), [1, 4]); - x.write(5); - x.write(6); - x.write(10); + x.push(5); + x.push(6); + x.push(10); assert_eq!(x.as_unordered_slice(), [10, 4, 5, 6]); x.extend([11, 12].iter()); @@ -704,7 +735,7 @@ mod tests { let mut x: HistoryBuf = HistoryBuf::new(); for i in 0..10 { assert_eq!(x.as_unordered_slice(), x.clone().as_unordered_slice()); - x.write(i); + x.push(i); } // Records number of clones locally and globally. @@ -722,11 +753,11 @@ mod tests { let mut y: HistoryBuf = HistoryBuf::new(); let _ = y.clone(); assert_eq!(GLOBAL.load(Ordering::Relaxed), 0); - y.write(InstrumentedClone(0)); + y.push(InstrumentedClone(0)); assert_eq!(GLOBAL.load(Ordering::Relaxed), 0); assert_eq!(y.clone().as_unordered_slice(), [InstrumentedClone(1)]); assert_eq!(GLOBAL.load(Ordering::Relaxed), 1); - y.write(InstrumentedClone(0)); + y.push(InstrumentedClone(0)); assert_eq!(GLOBAL.load(Ordering::Relaxed), 1); assert_eq!( y.clone().as_unordered_slice(), @@ -746,14 +777,14 @@ mod tests { assert_eq!(x.recent_index(), None); assert_eq!(x.recent(), None); - x.write(1); - x.write(4); + x.push(1); + x.push(4); assert_eq!(x.recent_index(), Some(1)); assert_eq!(x.recent(), Some(&4)); - x.write(5); - x.write(6); - x.write(10); + x.push(5); + x.push(6); + x.push(10); assert_eq!(x.recent_index(), Some(0)); assert_eq!(x.recent(), Some(&10)); } @@ -764,14 +795,14 @@ mod tests { assert_eq!(x.oldest_index(), None); assert_eq!(x.oldest(), None); - x.write(1); - x.write(4); + x.push(1); + x.push(4); assert_eq!(x.oldest_index(), Some(0)); assert_eq!(x.oldest(), Some(&1)); - x.write(5); - x.write(6); - x.write(10); + x.push(5); + x.push(6); + x.push(10); assert_eq!(x.oldest_index(), Some(1)); assert_eq!(x.oldest(), Some(&4)); } @@ -821,7 +852,7 @@ mod tests { let mut buffer: HistoryBuf = HistoryBuf::new(); for n in 0..20 { - buffer.write(n); + buffer.push(n); let (head, tail) = buffer.as_slices(); assert_eq_iter( [head, tail].iter().copied().flatten(), @@ -911,16 +942,16 @@ mod tests { let mut x: HistoryBuf = HistoryBuf::new(); let mut y: HistoryBuf = HistoryBuf::new(); assert_eq!(x, y); - x.write(1); + x.push(1); assert_ne!(x, y); - y.write(1); + y.push(1); assert_eq!(x, y); for _ in 0..4 { - x.write(2); + x.push(2); assert_ne!(x, y); for i in 0..5 { - x.write(i); - y.write(i); + x.push(i); + y.push(i); } assert_eq!( x, @@ -945,9 +976,9 @@ mod tests { } let mut x: HistoryBuf = HistoryBuf::new(); - x.write(DropCheck {}); - x.write(DropCheck {}); - x.write(DropCheck {}); + x.push(DropCheck {}); + x.push(DropCheck {}); + x.push(DropCheck {}); assert_eq!(DROP_COUNT.load(Ordering::SeqCst), 0); x.clear();