Repository: sharksforarms/deku
Pull request #118 (2020-10-22 21:39:49)

Implement the backend for reading a vector until a predicate or until a number of bits
This PR implements a potential backend (i.e., the context types only, and no proc-macros to use them yet) for both #110 and #76. I've done it like this to make sure we're happy with how this is implemented before I go any further, as this is a rather important breaking change (and also because the latter of those issues has not yet reached a conclusion as to the attribute it would use).

We convert the `Count` context type into a `Limit` enum, allowing either a count (the original behaviour), a number of bits, or a general predicate.

Annoyingly, the `Limit` type is now quite ugly, due to the need to have type parameters for the `Until` variant - I cannot think of any particularly nice options here. The best alternative is probably to template `Limit` on `<T>` only, and then take a `&mut dyn FnMut` in the `Until` variant, but I'm open to any suggestions here. Potentialy however, this ugliness doesn't _really_ matter, as the type is mostly designed to be hidden behind the nice proc-macro attributes once those are actually implemented for the two new cases.


Patch:
diff --git a/deku-derive/src/macros/deku_read.rs b/deku-derive/src/macros/deku_read.rs
index 591d979..e6725fa 100644
--- a/deku-derive/src/macros/deku_read.rs
+++ b/deku-derive/src/macros/deku_read.rs
@@ -359,7 +359,7 @@ fn emit_field_read(
             quote! {
                 {
                     use core::borrow::Borrow;
-                    DekuRead::read(rest, (deku::ctx::Count(usize::try_from(*((#field_count).borrow()))?), (#read_args)))
+                    DekuRead::read(rest, (usize::try_from(*((#field_count).borrow()))?.into(), (#read_args)))
                 }
             }
         } else {
diff --git a/ensure_wasm/Cargo.toml b/ensure_wasm/Cargo.toml
index 9d5074b..126ba56 100644
--- a/ensure_wasm/Cargo.toml
+++ b/ensure_wasm/Cargo.toml
@@ -13,7 +13,7 @@ crate-type = ["cdylib", "rlib"]
 default = ["console_error_panic_hook", "wee_alloc"]
 
 [dependencies]
-wasm-bindgen = "=0.2.65"
+wasm-bindgen = "=0.2.68"
 
 # The `console_error_panic_hook` crate provides better debugging of panics by
 # logging them with `console.error`. This is great for development, but requires
@@ -37,3 +37,7 @@ wasm-bindgen-test = "0.3"
 [profile.release]
 # Tell `rustc` to optimize for small code size.
 opt-level = "s"
+
+# https://github.com/rustwasm/wasm-pack/issues/886#issuecomment-667669802
+[package.metadata.wasm-pack.profile.release]
+wasm-opt = ["-Oz", "--enable-mutable-globals"]
diff --git a/src/ctx.rs b/src/ctx.rs
index e25efdc..8eb8112 100644
--- a/src/ctx.rs
+++ b/src/ctx.rs
@@ -1,6 +1,7 @@
 //! Types for context representation
 //! See [ctx attribute](../attributes/index.html#ctx) for more information.
 
+use core::marker::PhantomData;
 use core::ops::{Deref, DerefMut};
 use core::str::FromStr;
 
@@ -72,33 +73,34 @@ impl FromStr for Endian {
     }
 }
 
-/// The count of a container's elements
+/// A limit placed on a container's elements
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
-pub struct Count(pub usize);
+pub enum Limit<T, Predicate: FnMut(&T) -> bool> {
+    /// Read a specific count of elements
+    Count(usize),
 
-impl Into<usize> for Count {
-    fn into(self) -> usize {
-        self.0
-    }
+    /// Read until a given predicate holds true
+    Until(Predicate, PhantomData<T>),
+
+    /// Read until a given quantity of bits have been read
+    Bits(BitSize),
 }
 
-impl From<usize> for Count {
+impl<T> From<usize> for Limit<T, fn(&T) -> bool> {
     fn from(n: usize) -> Self {
-        Self(n)
+        Limit::Count(n)
     }
 }
 
-impl Deref for Count {
-    type Target = usize;
-
-    fn deref(&self) -> &Self::Target {
-        &self.0
+impl<T, Predicate: for<'a> FnMut(&'a T) -> bool> From<Predicate> for Limit<T, Predicate> {
+    fn from(predicate: Predicate) -> Self {
+        Limit::Until(predicate, PhantomData)
     }
 }
 
-impl DerefMut for Count {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.0
+impl<T> From<BitSize> for Limit<T, fn(&T) -> bool> {
+    fn from(bits: BitSize) -> Self {
+        Limit::Bits(bits)
     }
 }
 
diff --git a/src/lib.rs b/src/lib.rs
index d0adf8b..287783d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -215,7 +215,7 @@ use alloc::{format, vec::Vec};
 #[cfg(feature = "std")]
 use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 
-use crate::ctx::{BitSize, Count, Endian};
+use crate::ctx::{BitSize, Endian, Limit};
 use bitvec::prelude::*;
 use core::convert::TryInto;
 pub use deku_derive::*;
@@ -498,9 +498,47 @@ macro_rules! ImplDekuTraits {
     };
 }
 
-impl<T: DekuRead<Ctx>, Ctx: Copy> DekuRead<(Count, Ctx)> for Vec<T> {
-    /// Read the specified number of `T`s from input.
-    /// * `count` - the number of `T`s you want to read.
+/// Read `T`s into a vec until a given predicate returns true
+/// * `capacity` - an optional capacity to pre-allocate the vector with
+/// * `ctx` - The context required by `T`. It will be passed to every `T` when constructing.
+/// * `predicate` - the predicate that decides when to stop reading `T`s
+/// The predicate takes two parameters: the number of bits that have been read so far,
+/// and a borrow of the latest value to have been read. It should return `true` if reading
+/// should now stop, and `false` otherwise
+fn read_vec_with_predicate<T: DekuRead<Ctx>, Ctx: Copy, Predicate: FnMut(usize, &T) -> bool>(
+    input: &BitSlice<Msb0, u8>,
+    capacity: Option<usize>,
+    ctx: Ctx,
+    mut predicate: Predicate,
+) -> Result<(&BitSlice<Msb0, u8>, Vec<T>), DekuError> {
+    let mut res = if let Some(capacity) = capacity {
+        Vec::with_capacity(capacity)
+    } else {
+        Vec::new()
+    };
+
+    let mut rest = input;
+
+    loop {
+        let (new_rest, val) = <T>::read(rest, ctx)?;
+        res.push(val);
+        rest = new_rest;
+
+        // This unwrap is safe as we are pushing to the vec immediately before it,
+        // so there will always be a last element
+        if predicate(input.offset_from(rest) as usize, res.last().unwrap()) {
+            break;
+        }
+    }
+
+    Ok((rest, res))
+}
+
+impl<T: DekuRead<Ctx>, Ctx: Copy, Predicate: FnMut(&T) -> bool> DekuRead<(Limit<T, Predicate>, Ctx)>
+    for Vec<T>
+{
+    /// Read `T`s until the given limit
+    /// * `limit` - the limiting factor on the amount of `T`s to read
     /// * `inner_ctx` - The context required by `T`. It will be passed to every `T`s when constructing.
     /// # Examples
     /// ```rust
@@ -514,35 +552,51 @@ impl<T: DekuRead<Ctx>, Ctx: Copy> DekuRead<(Count, Ctx)> for Vec<T> {
     /// ```
     fn read(
         input: &BitSlice<Msb0, u8>,
-        (count, inner_ctx): (Count, Ctx),
+        (limit, inner_ctx): (Limit<T, Predicate>, Ctx),
     ) -> Result<(&BitSlice<Msb0, u8>, Self), DekuError>
     where
         Self: Sized,
     {
-        let count: usize = count.into();
-
-        let mut res = Vec::with_capacity(count);
-        let mut rest = input;
-        for _i in 0..count {
-            let (new_rest, val) = <T>::read(rest, inner_ctx)?;
-            res.push(val);
-            rest = new_rest;
-        }
+        match limit {
+            // Read a given count of elements
+            Limit::Count(mut count) => {
+                // Handle the trivial case of reading an empty vector
+                if count == 0 {
+                    return Ok((input, Vec::new()));
+                }
+
+                // Otherwise, read until we have read `count` elements
+                read_vec_with_predicate(input, Some(count), inner_ctx, move |_, _| {
+                    count -= 1;
+                    count == 0
+                })
+            }
+
+            // Read until a given predicate returns true
+            Limit::Until(mut predicate, _) => {
+                read_vec_with_predicate(input, None, inner_ctx, move |_, value| predicate(value))
+            }
 
-        Ok((rest, res))
+            // Read until a given quanity of bits have been read
+            Limit::Bits(bits) => {
+                read_vec_with_predicate(input, None, inner_ctx, move |read_bits, _| {
+                    read_bits == bits.into()
+                })
+            }
+        }
     }
 }
 
-impl<T: DekuRead> DekuRead<Count> for Vec<T> {
-    /// Read the specified number of `T`s from input for types which don't require context.
+impl<T: DekuRead, Predicate: FnMut(&T) -> bool> DekuRead<Limit<T, Predicate>> for Vec<T> {
+    /// Read `T`s until the given limit from input for types which don't require context.
     fn read(
         input: &BitSlice<Msb0, u8>,
-        count: Count,
+        limit: Limit<T, Predicate>,
     ) -> Result<(&BitSlice<Msb0, u8>, Self), DekuError>
     where
         Self: Sized,
     {
-        Vec::read(input, (count, ()))
+        Vec::read(input, (limit, ()))
     }
 }
 
@@ -855,25 +909,31 @@ mod tests {
         assert_eq!(expected_write, res_write.into_vec());
     }
 
-    #[rstest(input,endian,bit_size,count,expected,expected_rest,
-        case::count_0([0xAA].as_ref(), Endian::Little, Some(8), 0, vec![], bits![Msb0, u8; 1, 0, 1, 0, 1, 0, 1, 0]),
-        case::count_1([0xAA, 0xBB].as_ref(), Endian::Little, Some(8), 1, vec![0xAA], bits![Msb0, u8; 1, 0, 1, 1, 1, 0, 1, 1]),
-        case::count_2([0xAA, 0xBB, 0xCC].as_ref(), Endian::Little, Some(8), 2, vec![0xAA, 0xBB], bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0]),
-        case::bits_6([0b0110_1001, 0b1110_1001].as_ref(), Endian::Little, Some(6), 2, vec![0b00_011010, 0b00_011110], bits![Msb0, u8; 1, 0, 0, 1]),
+    #[rstest(input,endian,bit_size,limit,expected,expected_rest,
+        case::count_0([0xAA].as_ref(), Endian::Little, Some(8), 0.into(), vec![], bits![Msb0, u8; 1, 0, 1, 0, 1, 0, 1, 0]),
+        case::count_1([0xAA, 0xBB].as_ref(), Endian::Little, Some(8), 1.into(), vec![0xAA], bits![Msb0, u8; 1, 0, 1, 1, 1, 0, 1, 1]),
+        case::count_2([0xAA, 0xBB, 0xCC].as_ref(), Endian::Little, Some(8), 2.into(), vec![0xAA, 0xBB], bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0]),
+        case::until_null([0xAA, 0, 0xBB].as_ref(), Endian::Little, None, (|v: &u8| *v == 0u8).into(), vec![0xAA, 0], bits![Msb0, u8; 1, 0, 1, 1, 1, 0, 1, 1]),
+        case::until_bits([0xAA, 0xBB].as_ref(), Endian::Little, None, BitSize(8).into(), vec![0xAA], bits![Msb0, u8; 1, 0, 1, 1, 1, 0, 1, 1]),
+        case::bits_6([0b0110_1001, 0b1110_1001].as_ref(), Endian::Little, Some(6), 2.into(), vec![0b00_011010, 0b00_011110], bits![Msb0, u8; 1, 0, 0, 1]),
         #[should_panic(expected = "Parse(\"too much data: container of 8 bits cannot hold 9 bits\")")]
-        case::not_enough_data([].as_ref(), Endian::Little, Some(9), 1, vec![], bits![Msb0, u8;]),
+        case::not_enough_data([].as_ref(), Endian::Little, Some(9), 1.into(), vec![], bits![Msb0, u8;]),
         #[should_panic(expected = "Parse(\"too much data: container of 8 bits cannot hold 9 bits\")")]
-        case::not_enough_data([0xAA].as_ref(), Endian::Little, Some(9), 1, vec![], bits![Msb0, u8;]),
+        case::not_enough_data([0xAA].as_ref(), Endian::Little, Some(9), 1.into(), vec![], bits![Msb0, u8;]),
+        #[should_panic(expected = "Parse(\"not enough data: expected 8 bits got 0 bits\")")]
+        case::not_enough_data([0xAA].as_ref(), Endian::Little, Some(8), 2.into(), vec![], bits![Msb0, u8;]),
+        #[should_panic(expected = "Parse(\"not enough data: expected 8 bits got 0 bits\")")]
+        case::not_enough_data_until([0xAA].as_ref(), Endian::Little, Some(8), (|_: &u8| false).into(), vec![], bits![Msb0, u8;]),
         #[should_panic(expected = "Parse(\"not enough data: expected 8 bits got 0 bits\")")]
-        case::not_enough_data([0xAA].as_ref(), Endian::Little, Some(8), 2, vec![], bits![Msb0, u8;]),
+        case::not_enough_data_bits([0xAA].as_ref(), Endian::Little, Some(8), (BitSize(16)).into(), vec![], bits![Msb0, u8;]),
         #[should_panic(expected = "Parse(\"too much data: container of 8 bits cannot hold 9 bits\")")]
-        case::too_much_data([0xAA, 0xBB].as_ref(), Endian::Little, Some(9), 1, vec![], bits![Msb0, u8;]),
+        case::too_much_data([0xAA, 0xBB].as_ref(), Endian::Little, Some(9), 1.into(), vec![], bits![Msb0, u8;]),
     )]
-    fn test_vec_read(
+    fn test_vec_read<Predicate: FnMut(&u8) -> bool>(
         input: &[u8],
         endian: Endian,
         bit_size: Option<usize>,
-        count: usize,
+        limit: Limit<u8, Predicate>,
         expected: Vec<u8>,
         expected_rest: &BitSlice<Msb0, u8>,
     ) {
@@ -881,9 +941,9 @@ mod tests {
 
         let (rest, res_read) = match bit_size {
             Some(bit_size) => {
-                Vec::<u8>::read(bit_slice, (Count(count), (endian, BitSize(bit_size)))).unwrap()
+                Vec::<u8>::read(bit_slice, (limit, (endian, BitSize(bit_size)))).unwrap()
             }
-            None => Vec::<u8>::read(bit_slice, (Count(count), (endian))).unwrap(),
+            None => Vec::<u8>::read(bit_slice, (limit, (endian))).unwrap(),
         };
 
         assert_eq!(expected, res_read);
@@ -899,15 +959,19 @@ mod tests {
         assert_eq!(expected, res_write.into_vec());
     }
 
-    #[rstest(input, endian, bit_size, count, expected, expected_rest, expected_write,
-        case::normal_le([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Little, Some(16), 2, vec![0xBBAA, 0xDDCC], bits![Msb0, u8;], vec![0xAA, 0xBB, 0xCC, 0xDD]),
-        case::normal_be([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Big, Some(16), 2, vec![0xAABB, 0xCCDD], bits![Msb0, u8;], vec![0xAA, 0xBB, 0xCC, 0xDD]),
+    #[rstest(input, endian, bit_size, limit, expected, expected_rest, expected_write,
+        case::normal_le([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Little, Some(16), 2.into(), vec![0xBBAA, 0xDDCC], bits![Msb0, u8;], vec![0xAA, 0xBB, 0xCC, 0xDD]),
+        case::normal_be([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Big, Some(16), 2.into(), vec![0xAABB, 0xCCDD], bits![Msb0, u8;], vec![0xAA, 0xBB, 0xCC, 0xDD]),
+        case::predicate_le([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Little, Some(16), (|v: &u16| *v == 0xBBAA).into(), vec![0xBBAA], bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 1], vec![0xAA, 0xBB]),
+        case::predicate_be([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Big, Some(16), (|v: &u16| *v == 0xAABB).into(), vec![0xAABB], bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 1], vec![0xAA, 0xBB]),
+        case::bytes_le([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Little, Some(16), BitSize(16).into(), vec![0xBBAA], bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 1], vec![0xAA, 0xBB]),
+        case::bytes_be([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Big, Some(16), BitSize(16).into(), vec![0xAABB], bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 1], vec![0xAA, 0xBB]),
     )]
-    fn test_vec_read_write(
+    fn test_vec_read_write<Predicate: FnMut(&u16) -> bool>(
         input: &[u8],
         endian: Endian,
         bit_size: Option<usize>,
-        count: usize,
+        limit: Limit<u16, Predicate>,
         expected: Vec<u16>,
         expected_rest: &BitSlice<Msb0, u8>,
         expected_write: Vec<u8>,
@@ -918,7 +982,7 @@ mod tests {
         let bit_size = bit_size.unwrap();
 
         let (rest, res_read) =
-            Vec::<u16>::read(bit_slice, (Count(count), (endian, BitSize(bit_size)))).unwrap();
+            Vec::<u16>::read(bit_slice, (limit, (endian, BitSize(bit_size)))).unwrap();
         assert_eq!(expected, res_read);
         assert_eq!(expected_rest, rest);
 


Test patch:
diff --git a/tests/test_generic.rs b/tests/test_generic.rs
index 0b41674..668799a 100644
--- a/tests/test_generic.rs
+++ b/tests/test_generic.rs
@@ -4,7 +4,7 @@ use std::convert::{TryFrom, TryInto};
 #[test]
 fn test_generic_struct() {
     #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
-    pub struct TestStruct<T: deku::DekuWrite + deku::DekuRead>
+    pub struct TestStruct<T>
     where
         T: deku::DekuWrite + deku::DekuRead,
     {
@@ -24,7 +24,7 @@ fn test_generic_struct() {
 fn test_generic_enum() {
     #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
     #[deku(type = "u8")]
-    pub enum TestEnum<T: deku::DekuRead + deku::DekuWrite>
+    pub enum TestEnum<T>
     where
         T: deku::DekuWrite + deku::DekuRead,
     {