From 400345ae50104d4baf5d3e688735a62cd861eed0 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Mon, 17 Feb 2025 17:40:07 -0800 Subject: [PATCH 1/8] Test count_with helper for usage of read function on primitive Test is written to "what a user would expect", this currently fails. We should either move the optimization to the count helper, or fix it in some other way. --- binrw/tests/binread_impls.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/binrw/tests/binread_impls.rs b/binrw/tests/binread_impls.rs index 064ab2ee..85b2f71d 100644 --- a/binrw/tests/binread_impls.rs +++ b/binrw/tests/binread_impls.rs @@ -111,3 +111,23 @@ fn vec_u8() { binrw::Error::Io(..) )); } + +#[test] +fn count_with_correctness() { + // This doesn't work for some reason, complains about specific lifetime versus any lifetime + //let read = |reader, _, _| u8::read(reader).map(|v| v & 0x0F); + fn weird_u8_read(reader: &mut R, endian: binrw::Endian, args: ()) -> binrw::BinResult + where + R: binrw::io::Read + binrw::io::Seek, + { + u8::read(reader).map(|v| v & 0x0F) + } + let read = weird_u8_read; + + let read = binrw::helpers::count_with(1, read); + let val: Vec = read(&mut Cursor::new(&[0xF3u8]), binrw::Endian::Little, ()).unwrap(); + assert_eq!( + val[0], 0x03, + "binrw::helpers::count_with ignored the passed read function!" + ) +} From 651e0513eb2eabed19528f46ae41134eec9c8c7a Mon Sep 17 00:00:00 2001 From: Kitlith Date: Mon, 17 Feb 2025 18:01:47 -0800 Subject: [PATCH 2/8] Make BinRead impl for Vec no longer require 'static. Fixes jam1garner/binrw#298 This implementation chooses to delegate reading a known number of items to a new method `read_options_count` with a default, naive implementation. The `Vec` impl and the `count` helper delegate to `T::read_options_count`. The `count_with` helper is de-optimized into a naive loop, for correctness' sake. (This fixes the failing test introduced in the previous commit.) One nit remains: Arrays don't have this optimization applied. --- binrw/src/binread/impls.rs | 120 +++++++++++++++++++++++++++++++++++-- binrw/src/binread/mod.rs | 55 +++++++++++++++++ binrw/src/helpers.rs | 87 +++------------------------ 3 files changed, 180 insertions(+), 82 deletions(-) diff --git a/binrw/src/binread/impls.rs b/binrw/src/binread/impls.rs index d06ab525..ccf46969 100644 --- a/binrw/src/binread/impls.rs +++ b/binrw/src/binread/impls.rs @@ -10,7 +10,9 @@ use core::num::{ }; macro_rules! binread_impl { - ($($type_name:ty),*$(,)?) => { + // `$(some_lit $(__unused $is_lit_present:tt))?` allows us to match on the present of a literal + // using `$($($is_list_present)? fn do_whatever() {})?` + ($($type_name:ty $(as int $(__unused $is_int:tt)?)?),*$(,)?) => { $( impl BinRead for $type_name { type Args<'a> = (); @@ -29,12 +31,122 @@ macro_rules! binread_impl { } }) } + + $( + $($is_int)? + fn read_options_count<'a, R>( + reader: &mut R, + endian: Endian, + _args: Self::Args<'a>, + count: usize, + ) -> BinResult> + where + R: Read + Seek, + Self::Args<'a>: Clone, + { + let mut list = Vec::::new(); + let mut start = 0; + let mut remaining = count; + // Allocating and reading from the source in chunks is done to keep + // a bad `count` from causing huge memory allocations that are + // doomed to fail + while remaining != 0 { + // Using a similar strategy as std `default_read_to_end` to + // leverage the memory growth strategy of the underlying Vec + // implementation (in std this will be exponential) using a + // minimum byte allocation + let growth: usize = 32 / core::mem::size_of::(); + list.reserve(remaining.min(growth.max(1))); + + let items_to_read = remaining.min(list.capacity() - start); + let end = start + items_to_read; + + // In benchmarks, this resize decreases performance by 27–40% + // relative to using `unsafe` to write directly to uninitialised + // memory, but nobody ever got fired for buying IBM + list.resize(end, 0); + reader.read_exact(&mut bytemuck::cast_slice_mut::<_, u8>( + &mut list[start..end], + ))?; + + remaining -= items_to_read; + start += items_to_read; + } + + if core::mem::size_of::() != 1 + && ((cfg!(target_endian = "big") && endian == crate::Endian::Little) + || (cfg!(target_endian = "little") && endian == crate::Endian::Big)) + { + for value in list.iter_mut() { + *value = value.swap_bytes(); + } + } + Ok(list) + } + )? } )* } } -binread_impl!(u8, u16, u32, u64, u128, i8, i16, i32, i64, i128, f32, f64); +binread_impl!( + u16 as int, + u32 as int, + u64 as int, + u128 as int, + i8 as int, + i16 as int, + i32 as int, + i64 as int, + i128 as int, + f32, + f64 +); + +impl BinRead for u8 { + type Args<'a> = (); + fn read_options( + reader: &mut R, + _endian: Endian, + _args: Self::Args<'_>, + ) -> BinResult { + let mut val = 0u8; + let pos = reader.stream_position()?; + reader + .read_exact(core::slice::from_mut(&mut val)) + .or_else(crate::__private::restore_position(reader, pos))?; + + Ok(val) + } + + // This extra impl for `u8` makes it faster than + // `binread_impl`, but *only* because `binread_impl` is not allowed + // to use unsafe code to eliminate the unnecessary zero-fill. + // Otherwise, performance would be identical and it could be + // deleted. + fn read_options_count<'a, R>( + reader: &mut R, + _endian: Endian, + _args: Self::Args<'a>, + count: usize, + ) -> BinResult> + where + R: Read + Seek, + Self::Args<'a>: Clone, + { + let mut container = Vec::::new(); + container.reserve_exact(count); + let byte_count = reader + .take(count.try_into().map_err(crate::helpers::not_enough_bytes)?) + .read_to_end(&mut container)?; + + if byte_count == count { + Ok(container) + } else { + Err(crate::helpers::not_enough_bytes(())) + } + } +} fn unexpected_zero_num() -> Error { Error::Io(io::Error::new( @@ -133,7 +245,7 @@ pub struct VecArgs { impl BinRead for Vec where - B: BinRead + 'static, + B: BinRead, for<'a> B::Args<'a>: Clone, { type Args<'a> = VecArgs>; @@ -143,7 +255,7 @@ where endian: Endian, args: Self::Args<'_>, ) -> BinResult { - crate::helpers::count_with(args.count, B::read_options)(reader, endian, args.inner) + B::read_options_count(reader, endian, args.inner, args.count) } } diff --git a/binrw/src/binread/mod.rs b/binrw/src/binread/mod.rs index 908d5aeb..624cffd7 100644 --- a/binrw/src/binread/mod.rs +++ b/binrw/src/binread/mod.rs @@ -192,6 +192,61 @@ pub trait BinRead: Sized { endian: Endian, args: Self::Args<'_>, ) -> BinResult; + + /// Read `count` items of `Self` from the reader using the given [`Endian`] and arguments + /// + /// A vehicle for optimizations of types that can easily be read many-at-a-time. + /// For example, the integral types {i,u}{8,16,32,64,128}. + /// + /// # Errors + /// + /// If reading fails, an [`Error`](crate::Error) variant will be returned. + /// + /// # Examples + /// + /// ``` + /// # use binrw::{io::{Read, Seek}, BinRead, BinResult, Endian}; + /// struct CustomU8(u8); + /// + /// impl BinRead for CustomU8 { + /// type Args<'a> = ::Args<'a>; + /// + /// fn read_options( + /// reader: &mut R, + /// endian: binrw::Endian, + /// args: Self::Args<'_>, + /// ) -> BinResult { + /// u8::read_options(reader, endian, args).map(CustomU8) + /// } + /// + /// fn read_options_count<'a, R>( + /// reader: &mut R, + /// endian: Endian, + /// args: Self::Args<'a>, + /// count: usize, + /// ) -> BinResult> + /// where + /// R: Read + Seek, + /// Self::Args<'a>: Clone, + /// { + /// u8::read_options_count(reader, endian, args, count).map(|c| c.into_iter().map(CustomU8).collect()) + /// } + /// } + /// ``` + fn read_options_count<'a, R>( + reader: &mut R, + endian: Endian, + args: Self::Args<'a>, + count: usize, + ) -> BinResult> + where + R: Read + Seek, + Self::Args<'a>: Clone, + { + core::iter::repeat_with(|| Self::read_options(reader, endian, args.clone())) + .take(count) + .collect() + } } /// Extension methods for reading [`BinRead`] objects directly from a reader. diff --git a/binrw/src/helpers.rs b/binrw/src/helpers.rs index def27e48..53199c39 100644 --- a/binrw/src/helpers.rs +++ b/binrw/src/helpers.rs @@ -450,9 +450,11 @@ where T: for<'a> BinRead = Arg>, R: Read + Seek, Arg: Clone, - Ret: FromIterator + 'static, + Ret: FromIterator, { - count_with(n, T::read_options) + move |reader, endian, arg| { + T::read_options_count(reader, endian, arg, n).map(|v| v.into_iter().collect()) + } } /// Creates a parser that uses a given function to read N items into a @@ -492,34 +494,12 @@ where R: Read + Seek, Arg: Clone, ReadFn: Fn(&mut R, Endian, Arg) -> BinResult, - Ret: FromIterator + 'static, + Ret: FromIterator, { move |reader, endian, args| { - let mut container = core::iter::empty::().collect::(); - - vec_fast_int!(try (i8 i16 u16 i32 u32 i64 u64 i128 u128) using (container, reader, endian, n) else { - // This extra branch for `Vec` makes it faster than - // `vec_fast_int`, but *only* because `vec_fast_int` is not allowed - // to use unsafe code to eliminate the unnecessary zero-fill. - // Otherwise, performance would be identical and it could be - // deleted. - if let Some(bytes) = ::downcast_mut::>(&mut container) { - bytes.reserve_exact(n); - let byte_count = reader - .take(n.try_into().map_err(not_enough_bytes)?) - .read_to_end(bytes)?; - - if byte_count == n { - Ok(container) - } else { - Err(not_enough_bytes(())) - } - } else { - core::iter::repeat_with(|| read(reader, endian, args.clone())) - .take(n) - .collect() - } - }) + core::iter::repeat_with(|| read(reader, endian, args.clone())) + .take(n) + .collect() } } @@ -602,58 +582,9 @@ pub fn write_u24(value: &u32) -> binrw::BinResult<()> { writer.write_all(&buf[range]).map_err(Into::into) } -fn not_enough_bytes(_: T) -> Error { +pub(crate) fn not_enough_bytes(_: T) -> Error { Error::Io(io::Error::new( io::ErrorKind::UnexpectedEof, "not enough bytes in reader", )) } - -macro_rules! vec_fast_int { - (try ($($Ty:ty)+) using ($list:expr, $reader:expr, $endian:expr, $count:expr) else { $($else:tt)* }) => { - $(if let Some(list) = ::downcast_mut::>(&mut $list) { - let mut start = 0; - let mut remaining = $count; - // Allocating and reading from the source in chunks is done to keep - // a bad `count` from causing huge memory allocations that are - // doomed to fail - while remaining != 0 { - // Using a similar strategy as std `default_read_to_end` to - // leverage the memory growth strategy of the underlying Vec - // implementation (in std this will be exponential) using a - // minimum byte allocation - const GROWTH: usize = 32 / core::mem::size_of::<$Ty>(); - list.reserve(remaining.min(GROWTH.max(1))); - - let items_to_read = remaining.min(list.capacity() - start); - let end = start + items_to_read; - - // In benchmarks, this resize decreases performance by 27–40% - // relative to using `unsafe` to write directly to uninitialised - // memory, but nobody ever got fired for buying IBM - list.resize(end, 0); - $reader.read_exact(&mut bytemuck::cast_slice_mut::<_, u8>(&mut list[start..end]))?; - - remaining -= items_to_read; - start += items_to_read; - } - - if - core::mem::size_of::<$Ty>() != 1 - && ( - (cfg!(target_endian = "big") && $endian == crate::Endian::Little) - || (cfg!(target_endian = "little") && $endian == crate::Endian::Big) - ) - { - for value in list.iter_mut() { - *value = value.swap_bytes(); - } - } - Ok($list) - } else)* { - $($else)* - } - } -} - -use vec_fast_int; From ca149c76c364b58f2f2e2e0974eae821ee7d0a3b Mon Sep 17 00:00:00 2001 From: Kitlith Date: Mon, 17 Feb 2025 18:41:56 -0800 Subject: [PATCH 3/8] WIP: Apply "read multiple items" optimization to arrays. Not a big fan of this implementation. Not sure that there's a way to proceed without compromising on ergonomics either. Let's see if anyone else has ideas on how to proceed. --- binrw/src/binread/impls.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/binrw/src/binread/impls.rs b/binrw/src/binread/impls.rs index ccf46969..b3337396 100644 --- a/binrw/src/binread/impls.rs +++ b/binrw/src/binread/impls.rs @@ -263,6 +263,7 @@ impl BinRead for [B; N] where B: BinRead, for<'a> B::Args<'a>: Clone, + [B; N]: for<'b> TryFrom<&'b [B]>, { type Args<'a> = B::Args<'a>; @@ -271,7 +272,17 @@ where endian: Endian, args: Self::Args<'_>, ) -> BinResult { - array_init::try_array_init(|_| BinRead::read_options(reader, endian, args.clone())) + // Not a fan of this. Is there a better signature for read_options_count + // that could handle arrays in a nicer fashion, without compromising + // ergonomics too much? + let pos = reader.stream_position()?; + BinRead::read_options_count(reader, endian, args, N).and_then(|v| { + Self::try_from(&v).map_err(|_e| Error::AssertFail { + pos, + message: "Unable to turn slice into array".to_string(), + }) + }) + //array_init::try_array_init(|_| BinRead::read_options(reader, endian, args.clone())) } } From 4317a3a3b97d87dc98a06edc43fa6c1f0eff7439 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Mon, 17 Feb 2025 19:32:29 -0800 Subject: [PATCH 4/8] WIP: Use array_init::from_iter to move from Vec into array --- binrw/src/binread/impls.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/binrw/src/binread/impls.rs b/binrw/src/binread/impls.rs index b3337396..6c028015 100644 --- a/binrw/src/binread/impls.rs +++ b/binrw/src/binread/impls.rs @@ -263,7 +263,6 @@ impl BinRead for [B; N] where B: BinRead, for<'a> B::Args<'a>: Clone, - [B; N]: for<'b> TryFrom<&'b [B]>, { type Args<'a> = B::Args<'a>; @@ -272,17 +271,16 @@ where endian: Endian, args: Self::Args<'_>, ) -> BinResult { - // Not a fan of this. Is there a better signature for read_options_count + // Is there a better signature for read_options_count // that could handle arrays in a nicer fashion, without compromising // ergonomics too much? let pos = reader.stream_position()?; BinRead::read_options_count(reader, endian, args, N).and_then(|v| { - Self::try_from(&v).map_err(|_e| Error::AssertFail { + array_init::from_iter(v).ok_or_else(|| Error::AssertFail { pos, - message: "Unable to turn slice into array".to_string(), + message: "Unable to turn vec into array".to_string(), }) }) - //array_init::try_array_init(|_| BinRead::read_options(reader, endian, args.clone())) } } From 7c3c96b1675ec64da895897954d0ba32803e911a Mon Sep 17 00:00:00 2001 From: Kitlith Date: Mon, 17 Feb 2025 23:34:16 -0800 Subject: [PATCH 5/8] Make read_options_count return Vec *or* array Oops, the new Container trait is also a Functor. Still TODO: add documentation to the new Container stuffs. --- binrw/src/binread/impls.rs | 82 +++++++-------------------- binrw/src/binread/mod.rs | 23 ++++---- binrw/src/container.rs | 113 +++++++++++++++++++++++++++++++++++++ binrw/src/helpers.rs | 19 ++++--- binrw/src/lib.rs | 1 + 5 files changed, 157 insertions(+), 81 deletions(-) create mode 100644 binrw/src/container.rs diff --git a/binrw/src/binread/impls.rs b/binrw/src/binread/impls.rs index 6c028015..4b33b339 100644 --- a/binrw/src/binread/impls.rs +++ b/binrw/src/binread/impls.rs @@ -34,54 +34,32 @@ macro_rules! binread_impl { $( $($is_int)? - fn read_options_count<'a, R>( + fn read_options_count<'a, R, C>( reader: &mut R, endian: Endian, _args: Self::Args<'a>, - count: usize, - ) -> BinResult> + count: C::Count, + ) -> BinResult where R: Read + Seek, Self::Args<'a>: Clone, + C: crate::container::Container, { - let mut list = Vec::::new(); - let mut start = 0; - let mut remaining = count; - // Allocating and reading from the source in chunks is done to keep - // a bad `count` from causing huge memory allocations that are - // doomed to fail - while remaining != 0 { - // Using a similar strategy as std `default_read_to_end` to - // leverage the memory growth strategy of the underlying Vec - // implementation (in std this will be exponential) using a - // minimum byte allocation - let growth: usize = 32 / core::mem::size_of::(); - list.reserve(remaining.min(growth.max(1))); - - let items_to_read = remaining.min(list.capacity() - start); - let end = start + items_to_read; - - // In benchmarks, this resize decreases performance by 27–40% - // relative to using `unsafe` to write directly to uninitialised - // memory, but nobody ever got fired for buying IBM - list.resize(end, 0); + C::new_smart(count, |mut buf| { reader.read_exact(&mut bytemuck::cast_slice_mut::<_, u8>( - &mut list[start..end], + &mut buf, ))?; - remaining -= items_to_read; - start += items_to_read; - } - - if core::mem::size_of::() != 1 - && ((cfg!(target_endian = "big") && endian == crate::Endian::Little) - || (cfg!(target_endian = "little") && endian == crate::Endian::Big)) - { - for value in list.iter_mut() { - *value = value.swap_bytes(); + if core::mem::size_of::() != 1 + && ((cfg!(target_endian = "big") && endian == crate::Endian::Little) + || (cfg!(target_endian = "little") && endian == crate::Endian::Big)) + { + for value in buf.iter_mut() { + *value = value.swap_bytes(); + } } - } - Ok(list) + Ok(()) + }) } )? } @@ -124,27 +102,18 @@ impl BinRead for u8 { // to use unsafe code to eliminate the unnecessary zero-fill. // Otherwise, performance would be identical and it could be // deleted. - fn read_options_count<'a, R>( + fn read_options_count<'a, R, C>( reader: &mut R, _endian: Endian, _args: Self::Args<'a>, - count: usize, - ) -> BinResult> + count: C::Count, + ) -> BinResult where R: Read + Seek, Self::Args<'a>: Clone, + C: crate::container::Container, { - let mut container = Vec::::new(); - container.reserve_exact(count); - let byte_count = reader - .take(count.try_into().map_err(crate::helpers::not_enough_bytes)?) - .read_to_end(&mut container)?; - - if byte_count == count { - Ok(container) - } else { - Err(crate::helpers::not_enough_bytes(())) - } + C::new_smart(count, |buf| reader.read_exact(buf).map_err(Into::into)) } } @@ -271,16 +240,7 @@ where endian: Endian, args: Self::Args<'_>, ) -> BinResult { - // Is there a better signature for read_options_count - // that could handle arrays in a nicer fashion, without compromising - // ergonomics too much? - let pos = reader.stream_position()?; - BinRead::read_options_count(reader, endian, args, N).and_then(|v| { - array_init::from_iter(v).ok_or_else(|| Error::AssertFail { - pos, - message: "Unable to turn vec into array".to_string(), - }) - }) + BinRead::read_options_count(reader, endian, args, ()) } } diff --git a/binrw/src/binread/mod.rs b/binrw/src/binread/mod.rs index 624cffd7..cbd821bf 100644 --- a/binrw/src/binread/mod.rs +++ b/binrw/src/binread/mod.rs @@ -205,7 +205,7 @@ pub trait BinRead: Sized { /// # Examples /// /// ``` - /// # use binrw::{io::{Read, Seek}, BinRead, BinResult, Endian}; + /// # use binrw::{io::{Read, Seek}, BinRead, BinResult, Endian, container::Container}; /// struct CustomU8(u8); /// /// impl BinRead for CustomU8 { @@ -219,33 +219,34 @@ pub trait BinRead: Sized { /// u8::read_options(reader, endian, args).map(CustomU8) /// } /// - /// fn read_options_count<'a, R>( + /// fn read_options_count<'a, R, C>( /// reader: &mut R, /// endian: Endian, /// args: Self::Args<'a>, - /// count: usize, - /// ) -> BinResult> + /// count: C::Count, + /// ) -> BinResult /// where /// R: Read + Seek, /// Self::Args<'a>: Clone, + /// C: Container /// { - /// u8::read_options_count(reader, endian, args, count).map(|c| c.into_iter().map(CustomU8).collect()) + /// let c: C::HigherSelf = u8::read_options_count(reader, endian, args, count)?; + /// Ok(c.map(CustomU8)) /// } /// } /// ``` - fn read_options_count<'a, R>( + fn read_options_count<'a, R, C>( reader: &mut R, endian: Endian, args: Self::Args<'a>, - count: usize, - ) -> BinResult> + count: C::Count, + ) -> BinResult where R: Read + Seek, Self::Args<'a>: Clone, + C: crate::container::Container, { - core::iter::repeat_with(|| Self::read_options(reader, endian, args.clone())) - .take(count) - .collect() + C::new_naive(count, || Self::read_options(reader, endian, args.clone())) } } diff --git a/binrw/src/container.rs b/binrw/src/container.rs new file mode 100644 index 00000000..674b9825 --- /dev/null +++ b/binrw/src/container.rs @@ -0,0 +1,113 @@ +//! container module +#[cfg(not(feature = "std"))] +use alloc::vec::Vec; + +/// Container +pub trait Container: Sized + IntoIterator { + /// Count + type Count; + + /// naive + fn new_naive(count: Self::Count, f: Fun) -> Result + where + Fun: FnMut() -> Result; + + /// smart + fn new_smart(count: Self::Count, f: Fun) -> Result + where + Fun: FnMut(&mut [Self::Item]) -> Result<(), Error>, + Self::Item: Default + Clone; + + // whee it's a Functor + /// Type Constructor + type HigherSelf: Container = Self> + + IntoIterator; + + /// map + fn map(self, f: Fun) -> Self::HigherSelf + where + Fun: FnMut(Self::Item) -> T; +} + +impl Container for [T; N] { + type Count = (); + type HigherSelf = [X; N]; + + fn new_naive(_count: (), mut f: Fun) -> Result + where + Fun: FnMut() -> Result, + { + array_init::try_array_init(|_| f()) + } + + fn new_smart(_count: (), mut f: Fun) -> Result + where + Fun: FnMut(&mut [Self::Item]) -> Result<(), Error>, + Self::Item: Default + Clone, + { + let mut res = array_init::array_init(|_| Self::Item::default()); + f(&mut res)?; + Ok(res) + } + + fn map(self, f: Fun) -> Self::HigherSelf + where + Fun: FnMut(Self::Item) -> X, + { + self.map(f) + } +} + +impl Container for Vec { + type Count = usize; + type HigherSelf = Vec; + + fn new_naive(count: usize, f: Fun) -> Result + where + Fun: FnMut() -> Result, + { + core::iter::repeat_with(f).take(count).collect() + } + + fn new_smart(count: usize, mut f: Fun) -> Result + where + Fun: FnMut(&mut [Self::Item]) -> Result<(), Error>, + Self::Item: Default + Clone, + { + let mut list = Self::default(); + let mut start = 0; + let mut remaining = count; + // Allocating and reading from the source in chunks is done to keep + // a bad `count` from causing huge memory allocations that are + // doomed to fail + while remaining != 0 { + // Using a similar strategy as std `default_read_to_end` to + // leverage the memory growth strategy of the underlying Vec + // implementation (in std this will be exponential) using a + // minimum byte allocation + let growth: usize = 32 / core::mem::size_of::(); + list.reserve(remaining.min(growth.max(1))); + + let items_to_read = remaining.min(list.capacity() - start); + let end = start + items_to_read; + + // In benchmarks, this resize decreases performance by 27–40% + // relative to using `unsafe` to write directly to uninitialised + // memory, but nobody ever got fired for buying IBM + list.resize(end, Self::Item::default()); + f(&mut list[start..end])?; + + remaining -= items_to_read; + start += items_to_read; + } + + Ok(list) + } + + fn map(self, f: Fun) -> Self::HigherSelf + where + Fun: FnMut(Self::Item) -> X, + { + self.into_iter().map(f).collect() + } +} diff --git a/binrw/src/helpers.rs b/binrw/src/helpers.rs index 53199c39..7632b6ab 100644 --- a/binrw/src/helpers.rs +++ b/binrw/src/helpers.rs @@ -1,8 +1,8 @@ //! Helper functions for reading and writing data. use crate::{ - io::{self, Read, Seek}, - BinRead, BinResult, Endian, Error, + io::{Read, Seek}, + BinRead, BinResult, Endian, }; #[cfg(not(feature = "std"))] use alloc::vec::Vec; @@ -453,7 +453,8 @@ where Ret: FromIterator, { move |reader, endian, arg| { - T::read_options_count(reader, endian, arg, n).map(|v| v.into_iter().collect()) + let res: Vec<_> = T::read_options_count(reader, endian, arg, n)?; + Ok(res.into_iter().collect()) } } @@ -582,9 +583,9 @@ pub fn write_u24(value: &u32) -> binrw::BinResult<()> { writer.write_all(&buf[range]).map_err(Into::into) } -pub(crate) fn not_enough_bytes(_: T) -> Error { - Error::Io(io::Error::new( - io::ErrorKind::UnexpectedEof, - "not enough bytes in reader", - )) -} +// pub(crate) fn not_enough_bytes(_: T) -> Error { +// Error::Io(io::Error::new( +// io::ErrorKind::UnexpectedEof, +// "not enough bytes in reader", +// )) +// } diff --git a/binrw/src/lib.rs b/binrw/src/lib.rs index a66d3507..48198f8b 100644 --- a/binrw/src/lib.rs +++ b/binrw/src/lib.rs @@ -22,6 +22,7 @@ extern crate std; pub mod __private; mod binread; mod binwrite; +pub mod container; pub mod docs; pub mod endian; pub mod error; From 5e854ad576416b56ae67f83e869718a4aec540d3 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Mon, 17 Feb 2025 23:39:33 -0800 Subject: [PATCH 6/8] fixup! add Error sections to placeholder documentation --- binrw/src/container.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/binrw/src/container.rs b/binrw/src/container.rs index 674b9825..5d108b6c 100644 --- a/binrw/src/container.rs +++ b/binrw/src/container.rs @@ -8,11 +8,19 @@ pub trait Container: Sized + IntoIterator { type Count; /// naive + /// + /// # Errors + /// + /// If `f` returns an error, the error will be returned fn new_naive(count: Self::Count, f: Fun) -> Result where Fun: FnMut() -> Result; /// smart + /// + /// # Errors + /// + /// If `f` returns an error, the error will be returned fn new_smart(count: Self::Count, f: Fun) -> Result where Fun: FnMut(&mut [Self::Item]) -> Result<(), Error>, From d57269f1870859388dea68481622f8dce2152fee Mon Sep 17 00:00:00 2001 From: Kitlith Date: Mon, 17 Feb 2025 23:42:13 -0800 Subject: [PATCH 7/8] fixup! oops the test has build warnings --- binrw/tests/binread_impls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/binrw/tests/binread_impls.rs b/binrw/tests/binread_impls.rs index 85b2f71d..9013a2f3 100644 --- a/binrw/tests/binread_impls.rs +++ b/binrw/tests/binread_impls.rs @@ -116,7 +116,7 @@ fn vec_u8() { fn count_with_correctness() { // This doesn't work for some reason, complains about specific lifetime versus any lifetime //let read = |reader, _, _| u8::read(reader).map(|v| v & 0x0F); - fn weird_u8_read(reader: &mut R, endian: binrw::Endian, args: ()) -> binrw::BinResult + fn weird_u8_read(reader: &mut R, _endian: binrw::Endian, _args: ()) -> binrw::BinResult where R: binrw::io::Read + binrw::io::Seek, { From 97a6d5b8d9c16e7fa2e9a810be19f81fcbc305c8 Mon Sep 17 00:00:00 2001 From: Kitlith Date: Mon, 17 Feb 2025 23:45:22 -0800 Subject: [PATCH 8/8] fixup! missing doc link after removing "unused" import --- binrw/src/helpers.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/binrw/src/helpers.rs b/binrw/src/helpers.rs index 7632b6ab..6fcd5562 100644 --- a/binrw/src/helpers.rs +++ b/binrw/src/helpers.rs @@ -195,7 +195,7 @@ where /// # Errors /// /// If reading fails for a reason other than reaching the end of the input, an -/// [`Error`] variant will be returned. +/// [`crate::Error`] variant will be returned. /// /// # Examples /// @@ -236,7 +236,7 @@ where /// # Errors /// /// If reading fails for a reason other than reaching the end of the input, an -/// [`Error`] variant will be returned. +/// [`crate::Error`] variant will be returned. /// /// # Examples ///