feat: trait rework

Notes.md

@@ -486,3 +486,23 @@ after doing the code I realized I had no good way to test it because the
 built-in buf doesn't provide anything that could be used with it, which makes
 me think putting it in parsely might not make sense.  This use case _could_
 have used the custom reader attribute, maybe?
+
+### Using ParselyRead/ParselyWrite with types other than BitBuf/BitBufMut
+
+For things like RTP parsing, we don't want to use the BitBuf/BitBufMut
+abstractions because they limit the efficiency we can get when we use
+Bits/BitsMut directly.  It would be nice to be able to leverage
+ParselyRead/ParselyWrite for other types as well: i.e. manually implementing it
+for the RTP packet types.  The problem is that the trait bound
+(BitBuf/BitBufMut) is built into the trait, so we can't really do that.  I
+looked at changing the "buffer" type to be an associated type, but that has a
+couple problems:
+
+* We can't do `type Buf = impl BitBuf` or `type Buf = dyn BitBUf` so we lose
+the ability to say "some BitBuf type".  We _could_ add another layer of
+indirection here, but that's a bit of a bummer.
+* Supporting multiple buffer types also makes me think that it'd be nice to be
+able to provide different/optimized read/write impls for different types (e.g.
+one for BitBuf and another when we know it's a Bits specifically or something)
+
+--> look into a refactoring of the read/write traits to accomplish this

impl/Cargo.toml

@@ -7,7 +7,7 @@ description = "Macro-based struct serialization/deserialization"
 
 [dependencies]
 anyhow = "1"
-bits-io = { version = "0.5.4" }
+bits-io = { version = "0.5.5" }
 darling = "0.20.10"
 paste = "1"
 proc-macro2 = "1"

impl/src/code_gen/gen_read.rs

@@ -24,7 +24,7 @@ pub fn generate_parsely_read_impl(data: ParselyReadData) -> TokenStream {
 
 fn generate_plain_read(ty: &syn::Type, context_values: &[syn::Expr]) -> TokenStream {
     quote! {
-        #ty::read::<_, T>(buf, (#(#context_values,)*))
+        #ty::read::<T>(buf, (#(#context_values,)*))
     }
 }
 
@@ -182,11 +182,8 @@ fn generate_parsely_read_impl_struct(
 
             #(#field_reads)*
 
-            let __bytes_remaining_end = buf.remaining_bytes();
-            let mut __amount_read = __bytes_remaining_start - __bytes_remaining_end;
-            while __amount_read % #alignment != 0 {
+            while (__bytes_remaining_start - buf.remaining_bytes()) % #alignment != 0 {
                 buf.get_u8().context("padding")?;
-                __amount_read += 1;
             }
         }
     } else {
@@ -202,9 +199,9 @@ fn generate_parsely_read_impl_struct(
     };
 
     quote! {
-        impl ::#crate_name::ParselyRead for #struct_name {
+        impl<B: BitBuf> ::#crate_name::ParselyRead<B> for #struct_name {
             type Ctx = (#(#context_types,)*);
-            fn read<B: BitBuf, T: ::#crate_name::ByteOrder>(buf: &mut B, #ctx_var: (#(#context_types,)*)) -> ::#crate_name::ParselyResult<Self> {
+            fn read<T: ::#crate_name::ByteOrder>(buf: &mut B, #ctx_var: (#(#context_types,)*)) -> ::#crate_name::ParselyResult<Self> {
                 #(#context_assignments)*
 
                 #body

impl/src/code_gen/gen_write.rs

@@ -60,18 +60,18 @@ fn generate_parsely_write_impl_struct(
             } else if f.ty.is_option() {
                 field_write_output.extend(quote! {
                     if let Some(ref v) = self.#field_name {
-                        #write_type::write::<_, T>(v, buf, (#(#context_values,)*)).with_context(|| format!("Writing field '{}'", #field_name_string))?;
+                        #write_type::write::<T>(v, buf, (#(#context_values,)*)).with_context(|| format!("Writing field '{}'", #field_name_string))?;
                     }
                 });
             } else if f.ty.is_collection() {
                 field_write_output.extend(quote! {
                     self.#field_name.iter().enumerate().map(|(idx, v)| {
-                        #write_type::write::<_, T>(v, buf, (#(#context_values,)*)).with_context(|| format!("Index {idx}"))
+                        #write_type::write::<T>(v, buf, (#(#context_values,)*)).with_context(|| format!("Index {idx}"))
                     }).collect::<ParselyResult<Vec<_>>>().with_context(|| format!("Writing field '{}'", #field_name_string))?;
                 });
             } else {
                 field_write_output.extend(quote! {
-                    #write_type::write::<_, T>(&self.#field_name, buf, (#(#context_values,)*)).with_context(|| format!("Writing field '{}'", #field_name_string))?;
+                    #write_type::write::<T>(&self.#field_name, buf, (#(#context_values,)*)).with_context(|| format!("Writing field '{}'", #field_name_string))?;
                 });
             }
 
@@ -127,11 +127,8 @@ fn generate_parsely_write_impl_struct(
 
             #(#field_writes)*
 
-            let __bytes_remaining_end = buf.remaining_mut_bytes();
-            let mut __amount_written = __bytes_remaining_start - __bytes_remaining_end;
-            while __amount_written % #alignment != 0 {
+            while (__bytes_remaining_start - buf.remaining_mut_bytes()) % #alignment != 0 {
                 let _ = buf.put_u8(0).context("padding")?;
-                __amount_written += 1;
             }
 
         }
@@ -142,9 +139,9 @@ fn generate_parsely_write_impl_struct(
     };
 
     quote! {
-        impl ::#crate_name::ParselyWrite for #struct_name {
+        impl<B: BitBufMut> ::#crate_name::ParselyWrite<B> for #struct_name {
             type Ctx = (#(#context_types,)*);
-            fn write<B: BitBufMut, T: ByteOrder>(
+            fn write<T: ByteOrder>(
                 &self,
                 buf: &mut B,
                 ctx: Self::Ctx,

impl/src/error.rs

@@ -5,25 +5,24 @@ pub type ParselyResult<T> = anyhow::Result<T>;
 /// Helper trait to coerce values of both `T: ParselyWrite` and `Result<T, E>: E:
 /// Into<anyhow::Error>` into `ParselyResult<T>`.  We need a trait specifically for writing because
 /// if we don't bound the impl for `T` in some way there's ambiguity: the compiler doesn't know if
-/// we want `ParselyResult<T>` or `ParselyResult<Result<T, E>>`.
-pub trait IntoWritableParselyResult<T> {
-    fn into_parsely_result(self) -> ParselyResult<T>;
+pub trait IntoWritableParselyResult<T, B> {
+    fn into_writable_parsely_result(self) -> ParselyResult<T>;
 }
 
-impl<T> IntoWritableParselyResult<T> for T
+impl<T, B> IntoWritableParselyResult<T, B> for T
 where
-    T: ParselyWrite,
+    T: ParselyWrite<B>,
 {
-    fn into_parsely_result(self) -> ParselyResult<T> {
+    fn into_writable_parsely_result(self) -> ParselyResult<T> {
         Ok(self)
     }
 }
 
-impl<T, E> IntoWritableParselyResult<T> for Result<T, E>
+impl<T, E, B> IntoWritableParselyResult<T, B> for Result<T, E>
 where
     E: Into<anyhow::Error>,
 {
-    fn into_parsely_result(self) -> ParselyResult<T> {
+    fn into_writable_parsely_result(self) -> ParselyResult<T> {
         self.map_err(Into::into)
     }
 }
@@ -33,12 +32,15 @@ where
 /// concrete type and we can rely on type inference in order to figure out what that should be.
 /// Because of that we don't want/need the `ParselyWrite` trait bounds on the impl like we have
 /// above for the writable side, so we need a different trait here.
+// TODO: remove the 'read' from these method calls, as they get used in places like context
+// expression evaluation where the writable limitations also don't exist, but aren't exactly on the
+// 'read path' (for example when syncing state)
 pub trait IntoParselyResult<T> {
-    fn into_parsely_result_read(self) -> ParselyResult<T>;
+    fn into_parsely_result(self) -> ParselyResult<T>;
 }
 
 impl<T> IntoParselyResult<T> for T {
-    fn into_parsely_result_read(self) -> ParselyResult<T> {
+    fn into_parsely_result(self) -> ParselyResult<T> {
         Ok(self)
     }
 }
@@ -47,7 +49,7 @@ impl<T, E> IntoParselyResult<T> for Result<T, E>
 where
     E: Into<anyhow::Error>,
 {
-    fn into_parsely_result_read(self) -> ParselyResult<T> {
+    fn into_parsely_result(self) -> ParselyResult<T> {
         self.map_err(Into::into)
     }
 }

impl/src/lib.rs

@@ -17,6 +17,7 @@ pub use bits_io::{
 };
 
 pub mod nsw_types {
+    pub use bits_io::nsw_types::from_bitslice::BitSliceUxExts;
     pub use bits_io::nsw_types::*;
 }
 

impl/src/model_types.rs

@@ -261,9 +261,9 @@ impl MapExpr {
         // value?
         tokens.extend(quote! {
             {
-                let original_value = ::#crate_name::ParselyRead::read::<_, T>(buf, ())
+                let original_value = ::#crate_name::ParselyRead::read::<T>(buf, ())
                     .with_context(|| format!("Reading raw value for field '{}'", #field_name_string))?;
-                (#map_expr)(original_value).into_parsely_result_read()
+                (#map_expr)(original_value).into_parsely_result()
                     .with_context(|| format!("Mapping raw value for field '{}'", #field_name_string))
             }
         })
@@ -275,9 +275,13 @@ impl MapExpr {
         let map_expr = &self.0;
         tokens.extend(quote! {
             {
-                let mapped_value = (#map_expr)(&self.#field_name).into_parsely_result()
+                let mapped_value = (#map_expr)(&self.#field_name);
+                // Coerce the result of the mapping function into a ParselyResult<T> where we know
+                // T is writable to the buffer.  We need to use this syntax because otherwise the
+                // compiler gets caught up on trying to infer the buffer type.
+                let result = <_ as IntoWritableParselyResult<_, B>>::into_writable_parsely_result(mapped_value)
                     .with_context(|| format!("Mapping raw value for field '{}'", #field_name_string))?;
-                ::#crate_name::ParselyWrite::write::<B, T>(&mapped_value, buf, ())
+                ::#crate_name::ParselyWrite::write::<T>(&result, buf, ())
                     .with_context(|| format!("Writing mapped value for field '{}'", #field_name_string))?;
             }
         })

impl/src/parsely_read.rs

@@ -2,16 +2,16 @@ use bits_io::prelude::*;
 
 use crate::error::ParselyResult;
 
-pub trait ParselyRead: Sized {
+pub trait ParselyRead<B>: Sized {
     type Ctx;
-    fn read<B: BitBuf, T: ByteOrder>(buf: &mut B, ctx: Self::Ctx) -> ParselyResult<Self>;
+    fn read<T: ByteOrder>(buf: &mut B, ctx: Self::Ctx) -> ParselyResult<Self>;
 }
 
 macro_rules! impl_parsely_read_builtin {
     ($type:ty) => {
-        impl ParselyRead for $type {
+        impl<B: BitBuf> ParselyRead<B> for $type {
             type Ctx = ();
-            fn read<B: BitBuf, T: ByteOrder>(buf: &mut B, _: Self::Ctx) -> ParselyResult<Self> {
+            fn read<T: ByteOrder>(buf: &mut B, _: Self::Ctx) -> ParselyResult<Self> {
                 ::paste::paste! {
                     Ok(buf.[<get_ $type>]()?)
                 }
@@ -22,9 +22,9 @@ macro_rules! impl_parsely_read_builtin {
 
 macro_rules! impl_parsely_read_builtin_bo {
     ($type:ty) => {
-        impl ParselyRead for $type {
+        impl<B: BitBuf> ParselyRead<B> for $type {
             type Ctx = ();
-            fn read<B: BitBuf, T: ByteOrder>(buf: &mut B, _: Self::Ctx) -> ParselyResult<Self> {
+            fn read<T: ByteOrder>(buf: &mut B, _: Self::Ctx) -> ParselyResult<Self> {
                 ::paste::paste! {
                     Ok(buf.[<get_ $type>]::<T>()?)
                 }
@@ -33,9 +33,9 @@ macro_rules! impl_parsely_read_builtin_bo {
     };
 }
 
-impl ParselyRead for bool {
+impl<B: BitBuf> ParselyRead<B> for bool {
     type Ctx = ();
-    fn read<B: BitBuf, T: ByteOrder>(buf: &mut B, _ctx: Self::Ctx) -> ParselyResult<Self> {
+    fn read<T: ByteOrder>(buf: &mut B, _ctx: Self::Ctx) -> ParselyResult<Self> {
         Ok(buf.get_bool()?)
     }
 }

impl/src/parsely_write.rs

@@ -22,21 +22,17 @@ macro_rules! impl_stateless_sync {
     };
 }
 
-pub trait ParselyWrite: StateSync + Sized {
+pub trait ParselyWrite<B>: StateSync + Sized {
     type Ctx;
-    fn write<B: BitBufMut, T: ByteOrder>(&self, buf: &mut B, ctx: Self::Ctx) -> ParselyResult<()>;
+    fn write<T: ByteOrder>(&self, buf: &mut B, ctx: Self::Ctx) -> ParselyResult<()>;
 }
 
 macro_rules! impl_parsely_write_builtin {
     ($type:ty) => {
-        impl ParselyWrite for $type {
+        impl<B: BitBufMut> ParselyWrite<B> for $type {
             type Ctx = ();
 
-            fn write<B: BitBufMut, T: ByteOrder>(
-                &self,
-                buf: &mut B,
-                _: Self::Ctx,
-            ) -> ParselyResult<()> {
+            fn write<T: ByteOrder>(&self, buf: &mut B, _: Self::Ctx) -> ParselyResult<()> {
                 ::paste::paste! {
                     Ok(buf.[<put_ $type>](*self)?)
                 }
@@ -47,13 +43,9 @@ macro_rules! impl_parsely_write_builtin {
 
 macro_rules! impl_parsely_write_builtin_bo {
     ($type:ty) => {
-        impl ParselyWrite for $type {
+        impl<B: BitBufMut> ParselyWrite<B> for $type {
             type Ctx = ();
-            fn write<B: BitBufMut, T: ByteOrder>(
-                &self,
-                buf: &mut B,
-                _: Self::Ctx,
-            ) -> ParselyResult<()> {
+            fn write<T: ByteOrder>(&self, buf: &mut B, _: Self::Ctx) -> ParselyResult<()> {
                 ::paste::paste! {
                     Ok(buf.[<put_ $type>]::<T>(*self)?)
                 }

tests/expand/alignment.expanded.rs

@@ -3,43 +3,33 @@ use parsely_rs::*;
 struct Foo {
     one: u8,
 }
-impl ::parsely_rs::ParselyRead for Foo {
+impl<B: BitBuf> ::parsely_rs::ParselyRead<B> for Foo {
     type Ctx = ();
-    fn read<B: BitBuf, T: ::parsely_rs::ByteOrder>(
+    fn read<T: ::parsely_rs::ByteOrder>(
         buf: &mut B,
         _ctx: (),
     ) -> ::parsely_rs::ParselyResult<Self> {
         let __bytes_remaining_start = buf.remaining_bytes();
-        let one = u8::read::<_, T>(buf, ()).with_context(|| "Reading field 'one'")?;
-        let __bytes_remaining_end = buf.remaining_bytes();
-        let mut __amount_read = __bytes_remaining_start - __bytes_remaining_end;
-        while __amount_read % 4usize != 0 {
+        let one = u8::read::<T>(buf, ()).with_context(|| "Reading field 'one'")?;
+        while (__bytes_remaining_start - buf.remaining_bytes()) % 4usize != 0 {
             buf.get_u8().context("padding")?;
-            __amount_read += 1;
         }
         Ok(Self { one })
     }
 }
-impl ::parsely_rs::ParselyWrite for Foo {
+impl<B: BitBufMut> ::parsely_rs::ParselyWrite<B> for Foo {
     type Ctx = ();
-    fn write<B: BitBufMut, T: ByteOrder>(
-        &self,
-        buf: &mut B,
-        ctx: Self::Ctx,
-    ) -> ParselyResult<()> {
+    fn write<T: ByteOrder>(&self, buf: &mut B, ctx: Self::Ctx) -> ParselyResult<()> {
         let __bytes_remaining_start = buf.remaining_mut_bytes();
-        u8::write::<_, T>(&self.one, buf, ())
+        u8::write::<T>(&self.one, buf, ())
             .with_context(|| ::alloc::__export::must_use({
                 let res = ::alloc::fmt::format(
                     format_args!("Writing field \'{0}\'", "one"),
                 );
                 res
             }))?;
-        let __bytes_remaining_end = buf.remaining_mut_bytes();
-        let mut __amount_written = __bytes_remaining_start - __bytes_remaining_end;
-        while __amount_written % 4usize != 0 {
+        while (__bytes_remaining_start - buf.remaining_mut_bytes()) % 4usize != 0 {
             let _ = buf.put_u8(0).context("padding")?;
-            __amount_written += 1;
         }
         Ok(())
     }