From 7fca90cfc437b8d6826c0d0fde1a3434f2f2492e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 11 Apr 2025 00:17:37 +0000 Subject: [PATCH 1/5] Pre-allocate bytes in `Features::to_context_internal` This avoids vec doubling while doing `Features` conversions. --- lightning-types/src/features.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lightning-types/src/features.rs b/lightning-types/src/features.rs index 1b7f880314f..c750b17493d 100644 --- a/lightning-types/src/features.rs +++ b/lightning-types/src/features.rs @@ -903,15 +903,17 @@ impl Features { /// Converts `Features` to `Features`. Only known `T` features relevant to context `C` are /// included in the result. fn to_context_internal(&self) -> Features { - let from_byte_count = T::KNOWN_FEATURE_MASK.len(); - let to_byte_count = C::KNOWN_FEATURE_MASK.len(); - let mut flags = Vec::new(); - for (i, byte) in self.flags.iter().enumerate() { - if i < from_byte_count && i < to_byte_count { - let from_known_features = T::KNOWN_FEATURE_MASK[i]; - let to_known_features = C::KNOWN_FEATURE_MASK[i]; - flags.push(byte & from_known_features & to_known_features); + let flag_iter = self.flags.iter().enumerate().filter_map(|(i, byte)| { + if i < T::KNOWN_FEATURE_MASK.len() && i < C::KNOWN_FEATURE_MASK.len() { + Some((i, *byte & T::KNOWN_FEATURE_MASK[i] & C::KNOWN_FEATURE_MASK[i])) + } else { + None } + }); + let mut flags = Vec::new(); + flags.resize(flag_iter.clone().count(), 0); + for (i, byte) in flag_iter { + flags[i] = byte; } Features:: { flags, mark: PhantomData } } From 552cdf49dc2f6f05e45f862e2549d33aa9cd889f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 10 Apr 2025 21:57:29 +0000 Subject: [PATCH 2/5] Store `Feature` flags in line rather than on the heap by default In a running LDK instance we generally have a ton of `Feature`s objects flying around, including ones per channel/peer in the `NetworkGraph`, ones per channel/peer in `Channel`/`ChannelManager`, and ones inside of BOLT 11/12 Invoices. Thus, its useful to avoid unecessary allocations in them to reduce heap fragmentation. Luckily, Features generally don't have more than 15 bytes worth of flags, and because `Vec` has a `NonNull` pointer we can actually wrap a vec in a two-variant enum with zero additional space penalty. While this patch leaves actually deserializing `Features` without allocating as a future exercise, immediately releasing the allocation is much better than holding it, and `Features` constructed through repeated `set_*` calls benefit from avoiding the Vec entirely. --- lightning-types/src/features.rs | 149 +++++++++++++++++++++++++++++--- 1 file changed, 135 insertions(+), 14 deletions(-) diff --git a/lightning-types/src/features.rs b/lightning-types/src/features.rs index c750b17493d..63143298901 100644 --- a/lightning-types/src/features.rs +++ b/lightning-types/src/features.rs @@ -90,14 +90,13 @@ use core::borrow::Borrow; use core::hash::{Hash, Hasher}; use core::marker::PhantomData; +use core::ops::{Deref, DerefMut}; use core::{cmp, fmt}; use alloc::vec::Vec; mod sealed { - use super::Features; - - use alloc::vec::Vec; + use super::{FeatureFlags, Features}; /// The context in which [`Features`] are applicable. Defines which features are known to the /// implementation, though specification of them as required or optional is up to the code @@ -297,21 +296,21 @@ mod sealed { /// Returns whether the feature is required by the given flags. #[inline] - fn requires_feature(flags: &Vec) -> bool { + fn requires_feature(flags: &[u8]) -> bool { flags.len() > Self::BYTE_OFFSET && (flags[Self::BYTE_OFFSET] & Self::REQUIRED_MASK) != 0 } /// Returns whether the feature is supported by the given flags. #[inline] - fn supports_feature(flags: &Vec) -> bool { + fn supports_feature(flags: &[u8]) -> bool { flags.len() > Self::BYTE_OFFSET && (flags[Self::BYTE_OFFSET] & (Self::REQUIRED_MASK | Self::OPTIONAL_MASK)) != 0 } /// Sets the feature's required (even) bit in the given flags. #[inline] - fn set_required_bit(flags: &mut Vec) { + fn set_required_bit(flags: &mut FeatureFlags) { if flags.len() <= Self::BYTE_OFFSET { flags.resize(Self::BYTE_OFFSET + 1, 0u8); } @@ -322,7 +321,7 @@ mod sealed { /// Sets the feature's optional (odd) bit in the given flags. #[inline] - fn set_optional_bit(flags: &mut Vec) { + fn set_optional_bit(flags: &mut FeatureFlags) { if flags.len() <= Self::BYTE_OFFSET { flags.resize(Self::BYTE_OFFSET + 1, 0u8); } @@ -333,7 +332,7 @@ mod sealed { /// Clears the feature's required (even) and optional (odd) bits from the given /// flags. #[inline] - fn clear_bits(flags: &mut Vec) { + fn clear_bits(flags: &mut FeatureFlags) { if flags.len() > Self::BYTE_OFFSET { flags[Self::BYTE_OFFSET] &= !Self::REQUIRED_MASK; flags[Self::BYTE_OFFSET] &= !Self::OPTIONAL_MASK; @@ -661,6 +660,9 @@ mod sealed { supports_trampoline_routing, requires_trampoline_routing ); + // By default, allocate enough bytes to cover up to Trampoline. Update this as new features are + // added which we expect to appear commonly across contexts. + pub(super) const MIN_FEATURES_ALLOCATION_BYTES: usize = (57 + 7) / 8; define_feature!( 259, DnsResolver, @@ -700,6 +702,125 @@ mod sealed { const ANY_REQUIRED_FEATURES_MASK: u8 = 0b01_01_01_01; const ANY_OPTIONAL_FEATURES_MASK: u8 = 0b10_10_10_10; +// Vecs are always 3 pointers long, so `FeatureFlags` is never shorter than 24 bytes on 64-bit +// platforms no matter what we do. +// +// Luckily, because `Vec` uses a `NonNull` pointer to its buffer, the two-variant enum is free +// space-wise, but we only get the remaining 2 usizes in length available for our own stuff (as any +// other value is interpreted as the `Heap` variant). +// +// Thus, as long as we never use more than 16 bytes (15 bytes for the data and one byte for the +// length) for our Held variant `FeatureFlags` is the same length as a `Vec` in memory. +const DIRECT_ALLOC_BYTES: usize = if sealed::MIN_FEATURES_ALLOCATION_BYTES > 8 * 2 - 1 { + sealed::MIN_FEATURES_ALLOCATION_BYTES +} else { + 8 * 2 - 1 +}; +const _ASSERT: () = assert!(DIRECT_ALLOC_BYTES <= u8::MAX as usize); + +/// The internal storage for feature flags. Public only for the [`sealed`] feature flag traits but +/// generally should never be used directly. +#[derive(Clone, PartialEq, Eq)] +#[doc(hidden)] +pub enum FeatureFlags { + Held { bytes: [u8; DIRECT_ALLOC_BYTES], len: u8 }, + Heap(Vec), +} + +impl FeatureFlags { + fn empty() -> Self { + Self::Held { bytes: [0; DIRECT_ALLOC_BYTES], len: 0 } + } + + fn from(vec: Vec) -> Self { + if vec.len() <= DIRECT_ALLOC_BYTES { + let mut bytes = [0; DIRECT_ALLOC_BYTES]; + bytes[..vec.len()].copy_from_slice(&vec); + Self::Held { bytes, len: vec.len() as u8 } + } else { + Self::Heap(vec) + } + } + + fn resize(&mut self, new_len: usize, default: u8) { + match self { + Self::Held { bytes, len } => { + let start_len = *len as usize; + if new_len <= DIRECT_ALLOC_BYTES { + bytes[start_len..].copy_from_slice(&[default; DIRECT_ALLOC_BYTES][start_len..]); + *len = new_len as u8; + } else { + let mut vec = Vec::new(); + vec.resize(new_len, default); + vec[..start_len].copy_from_slice(&bytes[..start_len]); + *self = Self::Heap(vec); + } + }, + Self::Heap(vec) => { + vec.resize(new_len, default); + if new_len <= DIRECT_ALLOC_BYTES { + let mut bytes = [0; DIRECT_ALLOC_BYTES]; + bytes[..new_len].copy_from_slice(&vec[..new_len]); + *self = Self::Held { bytes, len: new_len as u8 }; + } + }, + } + } + + fn len(&self) -> usize { + self.deref().len() + } + + fn iter( + &self, + ) -> (impl Clone + ExactSizeIterator + DoubleEndedIterator) { + let slice = self.deref(); + slice.iter() + } + + fn iter_mut( + &mut self, + ) -> (impl ExactSizeIterator + DoubleEndedIterator) { + let slice = self.deref_mut(); + slice.iter_mut() + } +} + +impl Deref for FeatureFlags { + type Target = [u8]; + fn deref(&self) -> &[u8] { + match self { + FeatureFlags::Held { bytes, len } => &bytes[..*len as usize], + FeatureFlags::Heap(vec) => &vec, + } + } +} + +impl DerefMut for FeatureFlags { + fn deref_mut(&mut self) -> &mut [u8] { + match self { + FeatureFlags::Held { bytes, len } => &mut bytes[..*len as usize], + FeatureFlags::Heap(vec) => &mut vec[..], + } + } +} + +impl PartialOrd for FeatureFlags { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} +impl Ord for FeatureFlags { + fn cmp(&self, other: &Self) -> cmp::Ordering { + self.deref().cmp(other.deref()) + } +} +impl fmt::Debug for FeatureFlags { + fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> { + self.deref().fmt(fmt) + } +} + /// Tracks the set of features which a node implements, templated by the context in which it /// appears. /// @@ -707,7 +828,7 @@ const ANY_OPTIONAL_FEATURES_MASK: u8 = 0b10_10_10_10; #[derive(Eq)] pub struct Features { /// Note that, for convenience, flags is LITTLE endian (despite being big-endian on the wire) - flags: Vec, + flags: FeatureFlags, mark: PhantomData, } @@ -897,7 +1018,7 @@ impl ChannelTypeFeatures { impl Features { /// Create a blank Features with no features set pub fn empty() -> Self { - Features { flags: Vec::new(), mark: PhantomData } + Features { flags: FeatureFlags::empty(), mark: PhantomData } } /// Converts `Features` to `Features`. Only known `T` features relevant to context `C` are @@ -910,7 +1031,7 @@ impl Features { None } }); - let mut flags = Vec::new(); + let mut flags = FeatureFlags::empty(); flags.resize(flag_iter.clone().count(), 0); for (i, byte) in flag_iter { flags[i] = byte; @@ -923,7 +1044,7 @@ impl Features { /// /// This is not exported to bindings users as we don't support export across multiple T pub fn from_le_bytes(flags: Vec) -> Features { - Features { flags, mark: PhantomData } + Features { flags: FeatureFlags::from(flags), mark: PhantomData } } /// Returns the feature set as a list of bytes, in little-endian. This is in reverse byte order @@ -938,7 +1059,7 @@ impl Features { /// This is not exported to bindings users as we don't support export across multiple T pub fn from_be_bytes(mut flags: Vec) -> Features { flags.reverse(); // Swap to little-endian - Self { flags, mark: PhantomData } + Self { flags: FeatureFlags::from(flags), mark: PhantomData } } /// Returns true if this `Features` has any optional flags set @@ -1331,7 +1452,7 @@ mod tests { use std::hash::{Hash, Hasher}; let mut zerod_features = InitFeatures::empty(); - zerod_features.flags = vec![0]; + zerod_features.flags = FeatureFlags::Heap(vec![0]); let empty_features = InitFeatures::empty(); assert!(empty_features.flags.is_empty()); From 936276606204b0ced82da602e7bbcf915f5009f9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 5 May 2025 21:38:25 +0000 Subject: [PATCH 3/5] f test feature flags transitions --- lightning-types/src/features.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lightning-types/src/features.rs b/lightning-types/src/features.rs index 63143298901..2bb713e946f 100644 --- a/lightning-types/src/features.rs +++ b/lightning-types/src/features.rs @@ -1464,4 +1464,27 @@ mod tests { empty_features.hash(&mut empty_hash); assert_eq!(zerod_hash.finish(), empty_hash.finish()); } + + #[test] + fn test_feature_flags_transitions() { + // Tests transitions from stack to heap and back in `FeatureFlags` + let mut flags = FeatureFlags::empty(); + assert!(matches!(flags, FeatureFlags::Held { .. })); + + flags.resize(DIRECT_ALLOC_BYTES, 42); + assert_eq!(flags.len(), DIRECT_ALLOC_BYTES); + assert!(flags.iter().take(DIRECT_ALLOC_BYTES).all(|b| *b == 42)); + assert!(matches!(flags, FeatureFlags::Held { .. })); + + flags.resize(DIRECT_ALLOC_BYTES * 2, 43); + assert_eq!(flags.len(), DIRECT_ALLOC_BYTES * 2); + assert!(flags.iter().take(DIRECT_ALLOC_BYTES).all(|b| *b == 42)); + assert!(flags.iter().skip(DIRECT_ALLOC_BYTES).all(|b| *b == 43)); + assert!(matches!(flags, FeatureFlags::Heap(_))); + + flags.resize(DIRECT_ALLOC_BYTES, 0); + assert_eq!(flags.len(), DIRECT_ALLOC_BYTES); + assert!(flags.iter().take(DIRECT_ALLOC_BYTES).all(|b| *b == 42)); + assert!(matches!(flags, FeatureFlags::Held { .. })); + } } From d52760f7b050f8d8abffe3ddfae48150ecc43632 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 10 Apr 2025 22:35:23 +0000 Subject: [PATCH 4/5] Fuzz the new `FeatureFlags` storage No new datastructure would be complete without a dedicated fuzzer, so we add one here. --- fuzz/src/bin/feature_flags_target.rs | 120 +++++++++++++++++++++++++++ fuzz/src/bin/gen_target.sh | 1 + fuzz/src/feature_flags.rs | 95 +++++++++++++++++++++ fuzz/src/lib.rs | 1 + fuzz/targets.h | 1 + lightning-types/src/features.rs | 20 +++-- 6 files changed, 232 insertions(+), 6 deletions(-) create mode 100644 fuzz/src/bin/feature_flags_target.rs create mode 100644 fuzz/src/feature_flags.rs diff --git a/fuzz/src/bin/feature_flags_target.rs b/fuzz/src/bin/feature_flags_target.rs new file mode 100644 index 00000000000..1be8fd12e8c --- /dev/null +++ b/fuzz/src/bin/feature_flags_target.rs @@ -0,0 +1,120 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +// This file is auto-generated by gen_target.sh based on target_template.txt +// To modify it, modify target_template.txt and run gen_target.sh instead. + +#![cfg_attr(feature = "libfuzzer_fuzz", no_main)] +#![cfg_attr(rustfmt, rustfmt_skip)] + +#[cfg(not(fuzzing))] +compile_error!("Fuzz targets need cfg=fuzzing"); + +#[cfg(not(hashes_fuzz))] +compile_error!("Fuzz targets need cfg=hashes_fuzz"); + +#[cfg(not(secp256k1_fuzz))] +compile_error!("Fuzz targets need cfg=secp256k1_fuzz"); + +extern crate lightning_fuzz; +use lightning_fuzz::feature_flags::*; + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + feature_flags_run(data.as_ptr(), data.len()); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + feature_flags_run(data.as_ptr(), data.len()); + }); + } +} + +#[cfg(feature = "libfuzzer_fuzz")] +#[macro_use] extern crate libfuzzer_sys; +#[cfg(feature = "libfuzzer_fuzz")] +fuzz_target!(|data: &[u8]| { + feature_flags_run(data.as_ptr(), data.len()); +}); + +#[cfg(feature = "stdin_fuzz")] +fn main() { + use std::io::Read; + + let mut data = Vec::with_capacity(8192); + std::io::stdin().read_to_end(&mut data).unwrap(); + feature_flags_run(data.as_ptr(), data.len()); +} + +#[test] +fn run_test_cases() { + use std::fs; + use std::io::Read; + use lightning_fuzz::utils::test_logger::StringBuffer; + + use std::sync::{atomic, Arc}; + { + let data: Vec = vec![0]; + feature_flags_run(data.as_ptr(), data.len()); + } + let mut threads = Vec::new(); + let threads_running = Arc::new(atomic::AtomicUsize::new(0)); + if let Ok(tests) = fs::read_dir("test_cases/feature_flags") { + for test in tests { + let mut data: Vec = Vec::new(); + let path = test.unwrap().path(); + fs::File::open(&path).unwrap().read_to_end(&mut data).unwrap(); + threads_running.fetch_add(1, atomic::Ordering::AcqRel); + + let thread_count_ref = Arc::clone(&threads_running); + let main_thread_ref = std::thread::current(); + threads.push((path.file_name().unwrap().to_str().unwrap().to_string(), + std::thread::spawn(move || { + let string_logger = StringBuffer::new(); + + let panic_logger = string_logger.clone(); + let res = if ::std::panic::catch_unwind(move || { + feature_flags_test(&data, panic_logger); + }).is_err() { + Some(string_logger.into_string()) + } else { None }; + thread_count_ref.fetch_sub(1, atomic::Ordering::AcqRel); + main_thread_ref.unpark(); + res + }) + )); + while threads_running.load(atomic::Ordering::Acquire) > 32 { + std::thread::park(); + } + } + } + let mut failed_outputs = Vec::new(); + for (test, thread) in threads.drain(..) { + if let Some(output) = thread.join().unwrap() { + println!("\nOutput of {}:\n{}\n", test, output); + failed_outputs.push(test); + } + } + if !failed_outputs.is_empty() { + println!("Test cases which failed: "); + for case in failed_outputs { + println!("{}", case); + } + panic!(); + } +} diff --git a/fuzz/src/bin/gen_target.sh b/fuzz/src/bin/gen_target.sh index 0c51e6278dc..5672363ddd3 100755 --- a/fuzz/src/bin/gen_target.sh +++ b/fuzz/src/bin/gen_target.sh @@ -25,6 +25,7 @@ GEN_TEST indexedmap GEN_TEST onion_hop_data GEN_TEST base32 GEN_TEST fromstr_to_netaddress +GEN_TEST feature_flags GEN_TEST msg_accept_channel msg_targets:: GEN_TEST msg_announcement_signatures msg_targets:: diff --git a/fuzz/src/feature_flags.rs b/fuzz/src/feature_flags.rs new file mode 100644 index 00000000000..5df1dbfb442 --- /dev/null +++ b/fuzz/src/feature_flags.rs @@ -0,0 +1,95 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +use lightning::types::features::FeatureFlags; + +use crate::utils::test_logger; + +use std::ops::{Deref, DerefMut}; + +/// Check various methods on [`FeatureFlags`] given `v` which should be equal to `feat` and an +/// `old_v` which should be equal to `old_feat`. +fn check_eq(v: &Vec, feat: &FeatureFlags, old_v: &mut Vec, old_feat: &mut FeatureFlags) { + assert_eq!(v.len(), feat.len()); + assert_eq!(v.deref(), feat.deref()); + assert_eq!(old_v.deref_mut(), old_feat.deref_mut()); + + let mut feat_clone = feat.clone(); + assert!(feat_clone == *feat); + + let mut feat_iter = feat.iter(); + let mut vec_iter = v.iter(); + assert_eq!(feat_iter.len(), vec_iter.len()); + while let Some(feat) = feat_iter.next() { + let v = vec_iter.next().unwrap(); + assert_eq!(*feat, *v); + } + assert!(vec_iter.next().is_none()); + + let mut feat_iter = feat_clone.iter_mut(); + let mut vec_iter = v.iter(); + assert_eq!(feat_iter.len(), vec_iter.len()); + while let Some(feat) = feat_iter.next() { + let v = vec_iter.next().unwrap(); + assert_eq!(*feat, *v); + } + assert!(vec_iter.next().is_none()); + + assert_eq!(v < old_v, feat < old_feat); + assert_eq!(v.partial_cmp(old_v), feat.partial_cmp(old_feat)); +} + +#[inline] +pub fn do_test(data: &[u8]) { + if data.len() % 3 != 0 { + return; + } + let mut vec = Vec::new(); + let mut features = FeatureFlags::empty(); + + // For each 3-tuple in the input, interpret the first byte as a "command", the second byte as + // an index within `vec`/`features` to mutate, and the third byte as a value. + for step in data.windows(3) { + let mut old_vec = vec.clone(); + let mut old_features = features.clone(); + match step[0] { + 0 => { + vec.resize(step[1] as usize, step[2]); + features.resize(step[1] as usize, step[2]); + }, + 1 => { + if vec.len() > step[1] as usize { + vec[step[1] as usize] = step[2]; + features[step[1] as usize] = step[2]; + } + }, + 2 => { + if vec.len() > step[1] as usize { + *vec.iter_mut().skip(step[1] as usize).next().unwrap() = step[2]; + *features.iter_mut().skip(step[1] as usize).next().unwrap() = step[2]; + } + }, + _ => {}, + } + // After each mutation, check that `vec` and `features` remain the same, and pass the + // previous state for testing comparisons. + check_eq(&vec, &features, &mut old_vec, &mut old_features); + } + + check_eq(&vec, &features, &mut vec.clone(), &mut features.clone()); +} + +pub fn feature_flags_test(data: &[u8], _out: Out) { + do_test(data); +} + +#[no_mangle] +pub extern "C" fn feature_flags_run(data: *const u8, datalen: usize) { + do_test(unsafe { std::slice::from_raw_parts(data, datalen) }); +} diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index 3a8d9c060a9..909d06bea33 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -18,6 +18,7 @@ pub mod bech32_parse; pub mod bolt11_deser; pub mod chanmon_consistency; pub mod chanmon_deser; +pub mod feature_flags; pub mod fromstr_to_netaddress; pub mod full_stack; pub mod indexedmap; diff --git a/fuzz/targets.h b/fuzz/targets.h index 352296027f0..3a3928b4f4d 100644 --- a/fuzz/targets.h +++ b/fuzz/targets.h @@ -18,6 +18,7 @@ void indexedmap_run(const unsigned char* data, size_t data_len); void onion_hop_data_run(const unsigned char* data, size_t data_len); void base32_run(const unsigned char* data, size_t data_len); void fromstr_to_netaddress_run(const unsigned char* data, size_t data_len); +void feature_flags_run(const unsigned char* data, size_t data_len); void msg_accept_channel_run(const unsigned char* data, size_t data_len); void msg_announcement_signatures_run(const unsigned char* data, size_t data_len); void msg_channel_reestablish_run(const unsigned char* data, size_t data_len); diff --git a/lightning-types/src/features.rs b/lightning-types/src/features.rs index 2bb713e946f..e80f442573c 100644 --- a/lightning-types/src/features.rs +++ b/lightning-types/src/features.rs @@ -728,11 +728,13 @@ pub enum FeatureFlags { } impl FeatureFlags { - fn empty() -> Self { + /// Constructs an empty [`FeatureFlags`] + pub fn empty() -> Self { Self::Held { bytes: [0; DIRECT_ALLOC_BYTES], len: 0 } } - fn from(vec: Vec) -> Self { + /// Constructs a [`FeatureFlags`] from the given bytes + pub fn from(vec: Vec) -> Self { if vec.len() <= DIRECT_ALLOC_BYTES { let mut bytes = [0; DIRECT_ALLOC_BYTES]; bytes[..vec.len()].copy_from_slice(&vec); @@ -742,7 +744,10 @@ impl FeatureFlags { } } - fn resize(&mut self, new_len: usize, default: u8) { + /// Resizes a [`FeatureFlags`] to the given length, padding with `default` if required. + /// + /// See [`Vec::resize`] for more info. + pub fn resize(&mut self, new_len: usize, default: u8) { match self { Self::Held { bytes, len } => { let start_len = *len as usize; @@ -767,18 +772,21 @@ impl FeatureFlags { } } - fn len(&self) -> usize { + /// Fetches the length of the [`FeatureFlags`], in bytes. + pub fn len(&self) -> usize { self.deref().len() } - fn iter( + /// Fetches an iterator over the bytes of this [`FeatureFlags`] + pub fn iter( &self, ) -> (impl Clone + ExactSizeIterator + DoubleEndedIterator) { let slice = self.deref(); slice.iter() } - fn iter_mut( + /// Fetches a mutable iterator over the bytes of this [`FeatureFlags`] + pub fn iter_mut( &mut self, ) -> (impl ExactSizeIterator + DoubleEndedIterator) { let slice = self.deref_mut(); From f4912785c7722342ecd5308e115509f28b2e0cee Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 5 May 2025 21:38:19 +0000 Subject: [PATCH 5/5] f comments --- fuzz/src/feature_flags.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fuzz/src/feature_flags.rs b/fuzz/src/feature_flags.rs index 5df1dbfb442..af1f49626b1 100644 --- a/fuzz/src/feature_flags.rs +++ b/fuzz/src/feature_flags.rs @@ -23,6 +23,7 @@ fn check_eq(v: &Vec, feat: &FeatureFlags, old_v: &mut Vec, old_feat: &mu let mut feat_clone = feat.clone(); assert!(feat_clone == *feat); + // Test iteration over the `FeatureFlags` with the base iterator let mut feat_iter = feat.iter(); let mut vec_iter = v.iter(); assert_eq!(feat_iter.len(), vec_iter.len()); @@ -32,6 +33,7 @@ fn check_eq(v: &Vec, feat: &FeatureFlags, old_v: &mut Vec, old_feat: &mu } assert!(vec_iter.next().is_none()); + // Do the same test of iteration over the `FeatureFlags` with the mutable iterator let mut feat_iter = feat_clone.iter_mut(); let mut vec_iter = v.iter(); assert_eq!(feat_iter.len(), vec_iter.len());