diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 7a53108..c7273da 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -16,11 +16,21 @@ jobs: steps: - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: stable + components: clippy + - name: Install cargo expand + run: cargo install cargo-expand - name: Build - run: cargo build --verbose + run: cargo build --verbose --all-features + - name: Run clippy + run: cargo clippy -- --D warnings - name: Run tests - run: cargo test --verbose - - uses: actions-rs/toolchain@v1 + run: cargo test --verbose --all-features + - uses: dtolnay/rust-toolchain@master with: - toolchain: stable - override: true + toolchain: nightly + components: rustfmt + - name: Run fmt check + run: cargo +nightly fmt --check diff --git a/Notes.md b/Notes.md index c3ed7fa..3eca824 100644 --- a/Notes.md +++ b/Notes.md @@ -81,6 +81,92 @@ takes a fair bit of special handling (including additional attributes like these cases, which seems like it may make sense. Will probably play with some implementations of that and see how they feel. +An interesting scenario came up with regards to the 'write' map path vs the 'read': + +Both sides want to take advantage of type inference to avoid having to make the +caller explicitly state types. Assuming a struct like this: + +```rust +#[derive(ParselyRead, ParselyWrite)] +struct Foo { + #[parsely_read(map = "|v: u8| { v.to_string() }")] + #[parsely_write(map = "|v: &str| { v.parse::() }")] + value: String, +} +``` + +the generated read code can look like: + +```rust + let original_value = ::parsely_rs::ParselyRead::read::(buf, ()).with_context(...)?; + (|v: u8| { v.to_string() })(original_value) + .with_context(...)?; + +``` + +In this case the `read` map function isn't ambiguous about its type, but even +if it was it would be inferred ok because eventually we're going to assign it +to the field which _does_ have an explicit type, so it can be inferred from +there. The write side is trickier, but mainly because i didn't want to have to +force the caller to explicitly return a result, i wanted to be able to handle a +function that returned either a raw value or a result that could be converted +into a ParselyResult. The write code looks like this: + +```rust + let mapped_value = (|v: &str| { v.parse() })(&self.value) + .with_context(...)?; + ::parsely_rs::ParselyWrite::write::(&mapped_value, buf, ()) + .with_context(...)?; +``` + +We need to somehow wrap the result of the function in such a way that we end up with a `ParselyResult` regardless of whether it returned some raw type T or some kind of Result. + +The first solution that comes to mind to try and do that is this: + +```rust +pub trait IntoParselyResult { + fn into_parsely_result(self) -> ParselyResult; +} + +impl IntoParselyResult for Result +where + E: Into, +{ + fn into_parsely_result(self) -> ParselyResult { + self.map_err(Into::into) + } +} + +impl IntoParselyResult for T { + fn into_parsely_result(self) -> ParselyResult { + Ok(self) + } +} +``` + +The problem is that for the case where the expression returns `Result` +there's ambiguity: it can't tell if what we want is `ParselyResult` or +`ParselyResult>` so the compiler complains about both +implementations being possible. I tried a bunch of different wrappers and +macros to try and work around this but couldn't come up with anything. Finally +my only idea was to tweak the implementation for `T` slightly: + +```rust +impl IntoParselyResult for T where T: ParselyWrite { + fn into_parsely_result(self) -> ParselyResult { + Ok(self) + } +} +``` + +Since we don't impl `ParselyWrite` for any `Result` type (and I don't think +we'd have a need to) this disambiguates the two cases enough that there's no +issue trying to infer which one touse. + +In order to do this, though, we'll need to change the definition of +`ParselyWrite` to use associated types instead of generics, but I think that +may actually make more sense anyway. + is there an inherent "ordering" to all the attributes that we could always apply in a consistent way? context reads (assigning the context tuple variables into named variables defined by the 'required_context' attribute): I think these can always be first @@ -253,7 +339,8 @@ I want to do that. So maybe the call to `sync` will have to be manual and it will just be generated to do the right things? Going with that for now. TODO: - can we get rid of the ambiguity of a plain "Ok" in sync_func? Could we make it such that plain (no Ok needed) would also work? + can we get rid of the ambiguity of a plain "Ok" in sync_func? Could we make +it such that plain (no Ok needed) would also work? Follow-up on this: I think that, when writing, the generated code should call 'sync' on every field, that way if the user forgets to set the 'sync_with' @@ -270,7 +357,31 @@ the context for writing? I don't think context is needed for writing (I've seen no use cases of it so far, and it seems like any write context), but logically it seems like a use-case could exist for it... -### The buffer type y +Follow-up: +I've separated out the sync code into a trait (`StateSync`). I've also moved +the arguments to an associated type, which means we can make `StateSync` a +supertrait of `ParselyWrite`. The idea is that, when generating the write, +`sync` will be called on every field so there's no chance of it being missed. + +One issue with this is built-in types: they need a default impl so that the +compiler doesn't complain about calling `sync` on them. But sometimes we need +to sync fields that are built-in types. For example, the length field of the +RtcpHeader needs to be sync'd using the sync args. + +There are 3 aspect to synchronizing fields: + +* `sync_args`: This is attached to a type definition and it describes the names +and types of the arguments that will be expected in that type's `StateSync` +impl. +* `sync_with`: This is attached to a field, and describes the values that will be +used when calling that field's `sync` method. +* `sync_func`: This is attached to a field and describes how _that field_ +should be updated in the `sync` method for its surrounding type using the args +passed to it via `sync_args` + +The last one is confusing. Maybe it's better named 'sync_expr'? + +### The buffer type Currently, ParselyRead takes any buffer that is BitRead and ParselyWrite takes any buffer that is BitWrite. The issue here is that BitRead and BitWrite are @@ -306,8 +417,8 @@ struct MyStruct { ... } Some thoughts: -- I think a crate would have to be very consistent with their use of this: mixing and matching could lead to problems -- Is it possible to write an 'alias' for a macro attribute? +* I think a crate would have to be very consistent with their use of this: mixing and matching could lead to problems +* Is it possible to write an 'alias' for a macro attribute? ### Post hooks diff --git a/impl/src/code_gen/gen_read.rs b/impl/src/code_gen/gen_read.rs index 1f6f88d..76805ea 100644 --- a/impl/src/code_gen/gen_read.rs +++ b/impl/src/code_gen/gen_read.rs @@ -3,9 +3,7 @@ use quote::{format_ident, quote}; use crate::{ get_crate_name, - model_types::{ - wrap_read_with_padding_handling, CollectionLimit, FuncOrClosure, TypedFnArgList, - }, + model_types::{wrap_read_with_padding_handling, CollectionLimit, TypedFnArgList}, syn_helpers::TypeExts, ParselyReadData, ParselyReadFieldData, }; @@ -61,35 +59,6 @@ fn generate_collection_read( } } -fn generate_map_read(field_name: &syn::Ident, map_fn: &FuncOrClosure) -> TokenStream { - let field_name_string = field_name.to_string(); - quote! { - { - let original_value = ParselyRead::read::(buf, ()).with_context(|| format!("Reading raw value for field '{}'", #field_name_string))?; - let mapped_value = (#map_fn)(original_value).with_context(|| format!("Mapping raw value for field '{}'", #field_name_string))?; - - ParselyResult::<_>::Ok(mapped_value) - } - } -} - -/// Generate an assertion 'block' that can be appended to a [`Result`] type by embedding it in an -/// `and_then` block. Note that we take a [`syn::Expr`] for the assertion, but it needs to -/// effectively be a function (or a closure) which accepts the value type and returns a boolean. -fn generate_assertion(field_name: &syn::Ident, assertion: &FuncOrClosure) -> TokenStream { - let assertion_string = quote! { #assertion }.to_string(); - let field_name_string = field_name.to_string(); - quote! { - .and_then(|actual_value| { - let assertion_func = #assertion; - if !assertion_func(&actual_value) { - bail!("Assertion failed: value of field '{}' ('{:?}') didn't pass assertion: '{}'", #field_name_string, actual_value, #assertion_string) - } - Ok(actual_value) - }) - } -} - fn wrap_in_optional(when_expr: &syn::Expr, inner: TokenStream) -> TokenStream { quote! { if #when_expr { @@ -115,11 +84,11 @@ fn wrap_in_optional(when_expr: &syn::Expr, inner: TokenStream) -> TokenStream { /// elements should be read. /// 4. If none of the above are the case, do a 'plain' read where we just read the type directly /// from the buffer. -/// 5. If an 'assertion' attribute is present, then generate code to assert on the read value using +/// 5. If an 'assertion' attribute is present then generate code to assert on the read value using /// the given assertion function or closure. /// 6. After the code to perform the read has been generated, we check if the field is an option -/// type. If so, a 'when' attribute is required. This is an expression that determines when -/// the read should actually be done. +/// type. If so, a 'when' attribute is required. This is an expression that determines when the +/// read should actually be done. /// 7. Finally, if an 'alignment' attribute is present, code is added to detect and consume any /// padding after the read. fn generate_field_read(field_data: &ParselyReadFieldData) -> TokenStream { @@ -127,6 +96,7 @@ fn generate_field_read(field_data: &ParselyReadFieldData) -> TokenStream { .ident .as_ref() .expect("Only named fields supported"); + let field_name_str = field_name.to_string(); let read_type = field_data.buffer_type(); // Context values that we need to pass to this field's ParselyRead::read method let context_values = field_data.context_values(); @@ -136,8 +106,8 @@ fn generate_field_read(field_data: &ParselyReadFieldData) -> TokenStream { output.extend(quote! { ParselyResult::<_>::Ok(#assign_expr) }) - } else if let Some(ref map_fn) = field_data.common.map { - output.extend(generate_map_read(field_name, map_fn)); + } else if let Some(ref map_expr) = field_data.common.map { + map_expr.to_read_map_tokens(field_name, &mut output); } else if field_data.ty.is_collection() { let limit = if let Some(ref count) = field_data.count { CollectionLimit::Count(count.clone()) @@ -146,15 +116,14 @@ fn generate_field_read(field_data: &ParselyReadFieldData) -> TokenStream { } else { panic!("Collection field '{field_name}' must have either 'count' or 'while' attribute"); }; - output.extend(generate_collection_read(limit, read_type, context_values)); + output.extend(generate_collection_read(limit, read_type, &context_values)); } else { - output.extend(generate_plain_read(read_type, context_values)); + output.extend(generate_plain_read(read_type, &context_values)); } - // println!("tokenstream: {}", output); - if let Some(ref assertion) = field_data.common.assertion { - output.extend(generate_assertion(field_name, assertion)); + assertion.to_read_assertion_tokens(&field_name_str, &mut output); + // output.extend(generate_assertion(field_name, assertion)); } let error_context = format!("Reading field '{field_name}'"); output.extend(quote! { .with_context(|| #error_context)?}); diff --git a/impl/src/code_gen/gen_write.rs b/impl/src/code_gen/gen_write.rs index eae4755..296c402 100644 --- a/impl/src/code_gen/gen_write.rs +++ b/impl/src/code_gen/gen_write.rs @@ -50,35 +50,28 @@ fn generate_parsely_write_impl_struct( let mut field_write_output = TokenStream::new(); if let Some(ref assertion) = f.common.assertion { - let assertion_string = quote! { #assertion }.to_string(); - field_write_output.extend(quote! { - let assertion_func = #assertion; - if !assertion_func(&self.#field_name) { - bail!("Assertion failed: value of field '{}' ('{:?}') didn't pass assertion: '{}'", #field_name_string, self.#field_name, #assertion_string) - } - }) + assertion.to_write_assertion_tokens(&field_name_string, &mut field_write_output); } - if let Some(ref map_fn) = f.common.map { - field_write_output.extend(quote! { - let mapped_value = (#map_fn)(&self.#field_name).with_context(|| format!("Mapping raw value for field '{}'", #field_name_string))?; - ParselyWrite::write::(&mapped_value, buf, ()).with_context(|| format!("Writing mapped value for field '{}'", #field_name_string))?; - }); + // TODO: these write calls should be qualified. Something like <#write_type as + // ParselyWrite>::write + if let Some(ref map_expr) = f.common.map { + map_expr.to_write_map_tokens(field_name, &mut field_write_output); } else if f.ty.is_option() { field_write_output.extend(quote! { if let Some(ref v) = self.#field_name { - #write_type::write::(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::(v, buf, (#(#context_values,)*)).with_context(|| format!("Index {idx}")) + #write_type::write::<_, T>(v, buf, (#(#context_values,)*)).with_context(|| format!("Index {idx}")) }).collect::>>().with_context(|| format!("Writing field '{}'", #field_name_string))?; }); } else { field_write_output.extend(quote! { - #write_type::write::(&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))?; }); } @@ -104,14 +97,14 @@ fn generate_parsely_write_impl_struct( .iter() // 'sync_with' attirbutes mean this field's 'sync' method needs to be called with some data // Iterate over all fields and either: - // a) call the sync function provided in the sync_func attribute + // a) execute the expression provided in the sync_expr attribute // b) call the sync function on that type with any provided sync_with arguments .map(|f| { let field_name = f.ident.as_ref().expect("Field has a name"); let field_name_string = field_name.to_string(); - if let Some(ref sync_func) = f.sync_func { + if let Some(ref sync_expr) = f.sync_expr { quote! { - self.#field_name = #sync_func.with_context(|| format!("Syncing field '{}'", #field_name_string))?; + self.#field_name = (#sync_expr).into_parsely_result().with_context(|| format!("Syncing field '{}'", #field_name_string))?; } } else if f.sync_with.is_empty() && f.ty.is_wrapped() { // We'll allow this combination to skip a call to sync: for types like Option or @@ -119,7 +112,7 @@ fn generate_parsely_write_impl_struct( // provided. quote! {} } else { - let sync_with = f.sync_with.expressions(); + let sync_with = f.sync_with_expressions(); quote! { self.#field_name.sync((#(#sync_with,)*)).with_context(|| format!("Syncing field '{}'", #field_name_string))?; } @@ -149,8 +142,13 @@ fn generate_parsely_write_impl_struct( }; quote! { - impl ::#crate_name::ParselyWrite for #struct_name { - fn write(&self, buf: &mut B, ctx: (#(#context_types,)*)) -> ::#crate_name::ParselyResult<()> { + impl ::#crate_name::ParselyWrite for #struct_name { + type Ctx = (#(#context_types,)*); + fn write( + &self, + buf: &mut B, + ctx: Self::Ctx, + ) -> ParselyResult<()> { #(#context_assignments)* #body @@ -159,7 +157,8 @@ fn generate_parsely_write_impl_struct( } } - impl StateSync<(#(#sync_args_types,)*)> for #struct_name { + impl StateSync for #struct_name { + type SyncCtx = (#(#sync_args_types,)*); fn sync(&mut self, (#(#sync_args_variables,)*): (#(#sync_args_types,)*)) -> ParselyResult<()> { #(#sync_field_calls)* diff --git a/impl/src/error.rs b/impl/src/error.rs index 1f05531..97ac835 100644 --- a/impl/src/error.rs +++ b/impl/src/error.rs @@ -1 +1,53 @@ +use crate::parsely_write::ParselyWrite; + pub type ParselyResult = anyhow::Result; + +/// Helper trait to coerce values of both `T: ParselyWrite` and `Result: E: +/// Into` into `ParselyResult`. 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` or `ParselyResult>`. +pub trait IntoWritableParselyResult { + fn into_parsely_result(self) -> ParselyResult; +} + +impl IntoWritableParselyResult for T +where + T: ParselyWrite, +{ + fn into_parsely_result(self) -> ParselyResult { + Ok(self) + } +} + +impl IntoWritableParselyResult for Result +where + E: Into, +{ + fn into_parsely_result(self) -> ParselyResult { + self.map_err(Into::into) + } +} + +/// When we need to convert an expression that may or may not be wrapped in a Result on the _read_ +/// path, we can rely on the fact that we'll eventually be assigning the value to a field with a +/// 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. +pub trait IntoParselyResult { + fn into_parsely_result_read(self) -> ParselyResult; +} + +impl IntoParselyResult for T { + fn into_parsely_result_read(self) -> ParselyResult { + Ok(self) + } +} + +impl IntoParselyResult for Result +where + E: Into, +{ + fn into_parsely_result_read(self) -> ParselyResult { + self.map_err(Into::into) + } +} diff --git a/impl/src/lib.rs b/impl/src/lib.rs index 7b92803..ed702ee 100644 --- a/impl/src/lib.rs +++ b/impl/src/lib.rs @@ -26,7 +26,7 @@ pub mod anyhow { use code_gen::{gen_read::generate_parsely_read_impl, gen_write::generate_parsely_write_impl}; use darling::{ast, FromDeriveInput, FromField, FromMeta}; -use model_types::{Context, ExprOrFunc, FuncOrClosure, TypedFnArgList}; +use model_types::{Assertion, Context, ExprOrFunc, MapExpr, TypedFnArgList}; use proc_macro2::TokenStream; use syn::DeriveInput; use syn_helpers::TypeExts; @@ -57,13 +57,13 @@ pub struct ParselyCommonFieldData { // See https://github.com/TedDriggs/darling/issues/330 // generics: Option, - assertion: Option, + assertion: Option, /// Values that need to be passed as context to this fields read or write method context: Option, /// An optional mapping that will be applied to the read value - map: Option, + map: Option, /// An optional indicator that this field is or needs to be aligned to the given byte alignment /// via padding. @@ -109,11 +109,16 @@ impl ParselyReadFieldData { } /// Get the context values that need to be passed to the read or write call for this field - pub(crate) fn context_values(&self) -> &[syn::Expr] { + pub(crate) fn context_values(&self) -> Vec { + let field_name = self + .ident + .as_ref() + .expect("Field must have a name") + .to_string(); if let Some(ref field_context) = self.common.context { - field_context.expressions() + field_context.expressions(&format!("Read context for field '{field_name}'")) } else { - &[] + vec![] } } } @@ -128,9 +133,9 @@ pub struct ParselyWriteFieldData { #[darling(flatten)] common: ParselyCommonFieldData, - /// An optional function or closure that will be called to synchronize this field based on some - /// external data - sync_func: Option, + /// An expression or function call that will be used to update this field in the generated + /// `StateSync` implementation for its parent type. + sync_expr: Option, /// An list of expressions that should be passed as context to this field's sync method. The /// sync method provides an opportunity to synchronize "linked" fields, where one field's value @@ -154,13 +159,29 @@ impl ParselyWriteFieldData { } /// Get the context values that need to be passed to the read or write call for this field - pub(crate) fn context_values(&self) -> &[syn::Expr] { + pub(crate) fn context_values(&self) -> Vec { + let field_name = self + .ident + .as_ref() + .expect("Field must have a name") + .to_string(); if let Some(ref field_context) = self.common.context { - field_context.expressions() + field_context.expressions(&format!("Write context for field '{field_name}'")) } else { - &[] + vec![] } } + + /// Get the context values that need to be passed to the read or write call for this field + pub(crate) fn sync_with_expressions(&self) -> Vec { + let field_name = self + .ident + .as_ref() + .expect("Field must have a name") + .to_string(); + self.sync_with + .expressions(&format!("Sync context for field '{field_name}'")) + } } #[derive(Debug, FromDeriveInput)] diff --git a/impl/src/model_types.rs b/impl/src/model_types.rs index 65bcf83..9e01d43 100644 --- a/impl/src/model_types.rs +++ b/impl/src/model_types.rs @@ -3,6 +3,8 @@ use proc_macro2::TokenStream; use quote::{format_ident, quote, ToTokens}; use syn::parse::Parse; +use crate::get_crate_name; + pub(crate) enum CollectionLimit { Count(syn::Expr), While(syn::Expr), @@ -67,8 +69,23 @@ impl FromMeta for TypedFnArgList { pub(crate) struct Context(Vec); impl Context { - pub(crate) fn expressions(&self) -> &[syn::Expr] { - &self.0 + /// Get the context arguments as a vector of expressions, with an erorr context including the + /// given `context` value. + pub(crate) fn expressions(&self, context: &str) -> Vec { + // We support Context expressions that return a ParselyResult or a raw value. So now wrap + // all the expressions with code that will normalize all of the results into + // ParselyResults. + self.0 + .iter() + .cloned() + .enumerate() + .map(|(idx, e)| { + syn::parse2(quote! { + (#e).into_parsely_result().with_context(|| format!("{}: expression {}", #context, #idx))? + }) + .unwrap() + }) + .collect() } pub(crate) fn is_empty(&self) -> bool { @@ -225,6 +242,93 @@ impl FromMeta for FuncOrClosure { } } +/// A map expression that can be applied to a value after reading or before writing +#[derive(Debug)] +pub(crate) struct MapExpr(FuncOrClosure); + +impl FromMeta for MapExpr { + fn from_string(value: &str) -> darling::Result { + Ok(Self(FuncOrClosure::from_string(value)?)) + } +} + +impl MapExpr { + pub(crate) fn to_read_map_tokens(&self, field_name: &syn::Ident, tokens: &mut TokenStream) { + let crate_name = get_crate_name(); + let field_name_string = field_name.to_string(); + let map_expr = &self.0; + // TODO: is there a case where context might be required for reading the 'buffer_type' + // value? + tokens.extend(quote! { + { + let original_value = ::#crate_name::ParselyRead::read::(buf, ()) + .with_context(|| format!("Reading raw value for field '{}'", #field_name_string))?; + (#map_expr)(original_value).into_parsely_result_read() + .with_context(|| format!("Mapping raw value for field '{}'", #field_name_string)) + } + }) + } + + pub(crate) fn to_write_map_tokens(&self, field_name: &syn::Ident, tokens: &mut TokenStream) { + let crate_name = get_crate_name(); + let field_name_string = field_name.to_string(); + let map_expr = &self.0; + tokens.extend(quote! { + { + let mapped_value = (#map_expr)(&self.#field_name).into_parsely_result() + .with_context(|| format!("Mapping raw value for field '{}'", #field_name_string))?; + ::#crate_name::ParselyWrite::write::(&mapped_value, buf, ()) + .with_context(|| format!("Writing mapped value for field '{}'", #field_name_string))?; + } + }) + } +} + +/// An assertion that can be used after reading a value or before writing one +#[derive(Debug)] +pub(crate) struct Assertion(FuncOrClosure); + +impl Parse for Assertion { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + Ok(Self(FuncOrClosure::parse(input)?)) + } +} + +impl FromMeta for Assertion { + fn from_string(value: &str) -> darling::Result { + Ok(Self(FuncOrClosure::from_string(value)?)) + } +} + +impl Assertion { + pub(crate) fn to_read_assertion_tokens(&self, field_name: &str, tokens: &mut TokenStream) { + let assertion = &self.0; + let assertion_string = quote! { #assertion }.to_string(); + tokens.extend(quote! { + .and_then(|read_value| { + let assertion_func = #assertion; + if !assertion_func(&read_value) { + bail!("Assertion failed: value of field '{}' ('{:?}') didn't pass assertion: '{}'", #field_name, read_value, #assertion_string) + } + Ok(read_value) + }) + }); + } + + pub(crate) fn to_write_assertion_tokens(&self, field_name: &str, tokens: &mut TokenStream) { + let assertion = &self.0; + let assertion_string = quote! { #assertion }.to_string(); + let assertion_func_ident = format_ident!("__{}_assertion_func", field_name); + let field_name_ident = format_ident!("{field_name}"); + tokens.extend(quote! { + let #assertion_func_ident = #assertion; + if !#assertion_func_ident(&self.#field_name_ident) { + bail!("Assertion failed: value of field '{}' ('{:?}') didn't pass assertion: '{}'", #field_name, self.#field_name_ident, #assertion_string) + } + }) + } +} + pub(crate) fn wrap_read_with_padding_handling( element_ident: &syn::Ident, alignment: usize, diff --git a/impl/src/parsely_write.rs b/impl/src/parsely_write.rs index 7bd7ee6..b45ec11 100644 --- a/impl/src/parsely_write.rs +++ b/impl/src/parsely_write.rs @@ -2,21 +2,41 @@ use bits_io::prelude::*; use crate::error::ParselyResult; -pub trait StateSync: Sized { - // TODO: I think we should probably do a default impl of sync here that just returns Ok(()) - fn sync(&mut self, sync_ctx: SyncCtx) -> ParselyResult<()>; +/// A trait for syncing a field with any required context. In order to prevent accidental misses +/// of this trait, it's required for all `ParselyWrite` implementors. When generating the +/// `ParselyWrite` implementation, `sync` will be called on every field. +pub trait StateSync: Sized { + type SyncCtx; + + fn sync(&mut self, _sync_ctx: Self::SyncCtx) -> ParselyResult<()> { + Ok(()) + } +} + +#[macro_export] +macro_rules! impl_stateless_sync { + ($ty:ty) => { + impl StateSync for $ty { + type SyncCtx = (); + } + }; } -// TODO: should this be changed to require StateSync? I think so? Pretty sure we assume it exists, -// so it'll move errors sooner -pub trait ParselyWrite: Sized { - fn write(&self, buf: &mut B, ctx: Ctx) -> ParselyResult<()>; +pub trait ParselyWrite: StateSync + Sized { + type Ctx; + fn write(&self, buf: &mut B, ctx: Self::Ctx) -> ParselyResult<()>; } macro_rules! impl_parsely_write_builtin { ($type:ty) => { - impl ParselyWrite for $type { - fn write(&self, buf: &mut B, _: ()) -> ParselyResult<()> { + impl ParselyWrite for $type { + type Ctx = (); + + fn write( + &self, + buf: &mut B, + _: Self::Ctx, + ) -> ParselyResult<()> { ::paste::paste! { Ok(buf.[](*self)?) } @@ -27,8 +47,13 @@ macro_rules! impl_parsely_write_builtin { macro_rules! impl_parsely_write_builtin_bo { ($type:ty) => { - impl ParselyWrite for $type { - fn write(&self, buf: &mut B, _: ()) -> ParselyResult<()> { + impl ParselyWrite for $type { + type Ctx = (); + fn write( + &self, + buf: &mut B, + _: Self::Ctx, + ) -> ParselyResult<()> { ::paste::paste! { Ok(buf.[]::(*self)?) } @@ -52,7 +77,8 @@ macro_rules! for_all { macro_rules! impl_state_sync_builtin { ($type:ty) => { - impl StateSync<()> for $type { + impl StateSync for $type { + type SyncCtx = (); fn sync(&mut self, _sync_ctx: ()) -> ParselyResult<()> { Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index aa53263..bef3a92 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,7 @@ // TODO: these should be moved to a prelude file pub use parsely_impl::anyhow::{Context, anyhow, bail}; -pub use parsely_impl::error::ParselyResult; +pub use parsely_impl::error::{IntoParselyResult, IntoWritableParselyResult, ParselyResult}; +pub use parsely_impl::impl_stateless_sync; pub use parsely_impl::nsw_types::*; pub use parsely_impl::{BigEndian, ByteOrder, LittleEndian, NetworkOrder}; pub use parsely_impl::{BitBuf, BitBufExts, BitBufMut, BitBufMutExts, Bits, BitsMut}; diff --git a/tests/expand/alignment.expanded.rs b/tests/expand/alignment.expanded.rs index a8f6b02..bcfb7e0 100644 --- a/tests/expand/alignment.expanded.rs +++ b/tests/expand/alignment.expanded.rs @@ -19,14 +19,15 @@ impl ::parsely_rs::ParselyRead for Foo { Ok(Self { one }) } } -impl ::parsely_rs::ParselyWrite for Foo { - fn write( +impl ::parsely_rs::ParselyWrite for Foo { + type Ctx = (); + fn write( &self, buf: &mut B, - ctx: (), - ) -> ::parsely_rs::ParselyResult<()> { + ctx: Self::Ctx, + ) -> ParselyResult<()> { let __bytes_remaining_start = buf.remaining_mut_bytes(); - u8::write::(&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"), @@ -42,7 +43,8 @@ impl ::parsely_rs::ParselyWrite for Foo { Ok(()) } } -impl StateSync<()> for Foo { +impl StateSync for Foo { + type SyncCtx = (); fn sync(&mut self, (): ()) -> ParselyResult<()> { self.one .sync(()) diff --git a/tests/expand/assertion.expanded.rs b/tests/expand/assertion.expanded.rs new file mode 100644 index 0000000..cc10fe1 --- /dev/null +++ b/tests/expand/assertion.expanded.rs @@ -0,0 +1,81 @@ +use parsely_rs::*; +struct Foo { + #[parsely(assertion = "|v: &u8| *v % 2 == 0")] + value: u8, +} +impl ::parsely_rs::ParselyRead for Foo { + fn read( + buf: &mut B, + _ctx: (), + ) -> ::parsely_rs::ParselyResult { + let value = u8::read::(buf, ()) + .and_then(|read_value| { + let assertion_func = |v: &u8| *v % 2 == 0; + if !assertion_func(&read_value) { + return ::anyhow::__private::Err( + ::anyhow::Error::msg( + ::alloc::__export::must_use({ + let res = ::alloc::fmt::format( + format_args!( + "Assertion failed: value of field \'{0}\' (\'{1:?}\') didn\'t pass assertion: \'{2}\'", + "value", read_value, "| v : & u8 | * v % 2 == 0", + ), + ); + res + }), + ), + ); + } + Ok(read_value) + }) + .with_context(|| "Reading field 'value'")?; + Ok(Self { value }) + } +} +impl ::parsely_rs::ParselyWrite for Foo { + type Ctx = (); + fn write( + &self, + buf: &mut B, + ctx: Self::Ctx, + ) -> ParselyResult<()> { + let __value_assertion_func = |v: &u8| *v % 2 == 0; + if !__value_assertion_func(&self.value) { + return ::anyhow::__private::Err( + ::anyhow::Error::msg( + ::alloc::__export::must_use({ + let res = ::alloc::fmt::format( + format_args!( + "Assertion failed: value of field \'{0}\' (\'{1:?}\') didn\'t pass assertion: \'{2}\'", + "value", self.value, "| v : & u8 | * v % 2 == 0", + ), + ); + res + }), + ), + ); + } + u8::write::<_, T>(&self.value, buf, ()) + .with_context(|| ::alloc::__export::must_use({ + let res = ::alloc::fmt::format( + format_args!("Writing field \'{0}\'", "value"), + ); + res + }))?; + Ok(()) + } +} +impl StateSync for Foo { + type SyncCtx = (); + fn sync(&mut self, (): ()) -> ParselyResult<()> { + self.value + .sync(()) + .with_context(|| ::alloc::__export::must_use({ + let res = ::alloc::fmt::format( + format_args!("Syncing field \'{0}\'", "value"), + ); + res + }))?; + Ok(()) + } +} diff --git a/tests/expand/assertion.rs b/tests/expand/assertion.rs new file mode 100644 index 0000000..0ff4b54 --- /dev/null +++ b/tests/expand/assertion.rs @@ -0,0 +1,7 @@ +use parsely_rs::*; + +#[derive(ParselyRead, ParselyWrite)] +struct Foo { + #[parsely(assertion = "|v: &u8| *v % 2 == 0")] + value: u8, +} diff --git a/tests/expand/map.expanded.rs b/tests/expand/map.expanded.rs new file mode 100644 index 0000000..3c3e8b4 --- /dev/null +++ b/tests/expand/map.expanded.rs @@ -0,0 +1,73 @@ +use parsely_rs::*; +struct Foo { + #[parsely_read(map = "|v: u8| { v.to_string() }")] + #[parsely_write(map = "|v: &str| { v.parse::() }")] + value: String, +} +impl ::parsely_rs::ParselyRead for Foo { + fn read( + buf: &mut B, + _ctx: (), + ) -> ::parsely_rs::ParselyResult { + let value = { + let original_value = ::parsely_rs::ParselyRead::read::(buf, ()) + .with_context(|| ::alloc::__export::must_use({ + let res = ::alloc::fmt::format( + format_args!("Reading raw value for field \'{0}\'", "value"), + ); + res + }))?; + (|v: u8| { v.to_string() })(original_value) + .into_parsely_result_read() + .with_context(|| ::alloc::__export::must_use({ + let res = ::alloc::fmt::format( + format_args!("Mapping raw value for field \'{0}\'", "value"), + ); + res + })) + } + .with_context(|| "Reading field 'value'")?; + Ok(Self { value }) + } +} +impl ::parsely_rs::ParselyWrite for Foo { + type Ctx = (); + fn write( + &self, + buf: &mut B, + ctx: Self::Ctx, + ) -> ParselyResult<()> { + { + let mapped_value = (|v: &str| { v.parse::() })(&self.value) + .into_parsely_result() + .with_context(|| ::alloc::__export::must_use({ + let res = ::alloc::fmt::format( + format_args!("Mapping raw value for field \'{0}\'", "value"), + ); + res + }))?; + ::parsely_rs::ParselyWrite::write::(&mapped_value, buf, ()) + .with_context(|| ::alloc::__export::must_use({ + let res = ::alloc::fmt::format( + format_args!("Writing mapped value for field \'{0}\'", "value"), + ); + res + }))?; + } + Ok(()) + } +} +impl StateSync for Foo { + type SyncCtx = (); + fn sync(&mut self, (): ()) -> ParselyResult<()> { + self.value + .sync(()) + .with_context(|| ::alloc::__export::must_use({ + let res = ::alloc::fmt::format( + format_args!("Syncing field \'{0}\'", "value"), + ); + res + }))?; + Ok(()) + } +} diff --git a/tests/expand/map.rs b/tests/expand/map.rs new file mode 100644 index 0000000..b88cd5b --- /dev/null +++ b/tests/expand/map.rs @@ -0,0 +1,8 @@ +use parsely_rs::*; + +#[derive(ParselyRead, ParselyWrite)] +struct Foo { + #[parsely_read(map = "|v: u8| { v.to_string() }")] + #[parsely_write(map = "|v: &str| { v.parse::() }")] + value: String, +} diff --git a/tests/ui/pass/alignment.rs b/tests/ui/pass/alignment.rs index ddecd9a..dd82e08 100644 --- a/tests/ui/pass/alignment.rs +++ b/tests/ui/pass/alignment.rs @@ -15,6 +15,6 @@ fn main() { let mut bits_mut = BitsMut::new(); - Foo::write::(&foo, &mut bits_mut, ()).unwrap(); + Foo::write::<_, NetworkOrder>(&foo, &mut bits_mut, ()).unwrap(); assert_eq!(bits_mut.len_bytes(), 4); } diff --git a/tests/ui/pass/basic_write.rs b/tests/ui/pass/basic_write.rs new file mode 100644 index 0000000..25aeeeb --- /dev/null +++ b/tests/ui/pass/basic_write.rs @@ -0,0 +1,17 @@ +use parsely_rs::*; + +#[derive(ParselyWrite)] +struct Foo { + one: bool, + two: u3, +} + +fn main() { + let mut bits_mut = BitsMut::new(); + let foo = Foo { + one: true, + two: u3::new(4), + }; + + Foo::write::<_, NetworkOrder>(&foo, &mut bits_mut, ()).unwrap(); +} diff --git a/tests/ui/pass/map.rs b/tests/ui/pass/map.rs index cc59ff2..27224dd 100644 --- a/tests/ui/pass/map.rs +++ b/tests/ui/pass/map.rs @@ -1,24 +1,26 @@ -use bitvec::prelude::*; use parsely_rs::*; #[derive(ParselyRead, ParselyWrite)] struct Foo { - #[parsely_read(map = "|v: u1| -> ParselyResult { Ok(v > 0) }")] - #[parsely_write(map = "|v: &bool| -> ParselyResult { Ok(u1::from(*v)) }")] - one: bool, + #[parsely_read(map = "|v: u8| { v.to_string() }")] + #[parsely_write(map = "|v: &str| { v.parse::() }")] + value: String, } fn main() { - let mut bits = Bits::from_static_bytes(&[0b10101010]); + let mut bits = Bits::from_static_bytes(&[42]); let foo = Foo::read::(&mut bits, ()).expect("successful parse"); - assert!(foo.one); + assert_eq!(foo.value, "42"); let mut bits_mut = BitsMut::new(); - let foo = Foo { one: true }; + let foo = Foo { + value: String::from("42"), + }; - foo.write::(&mut bits_mut, ()) + foo.write::<_, NetworkOrder>(&mut bits_mut, ()) .expect("successful write"); - assert_eq!(&bits_mut[..], bits![u8, Msb0; 1]); + let mut bits = bits_mut.freeze(); + assert_eq!(bits.get_u8().unwrap(), 42); } diff --git a/tests/ui/pass/sync.rs b/tests/ui/pass/sync.rs index 98de080..6a27736 100644 --- a/tests/ui/pass/sync.rs +++ b/tests/ui/pass/sync.rs @@ -7,10 +7,10 @@ use parsely_rs::*; struct Header { version: u8, packet_type: u8, - // sync_func can refer to an expression or a function and will be used to update the annotated + // sync_expr can refer to an expression or a function and will be used to update the annotated // field, it should evaluate to ParselyResult where T is the type of the field. You can // refer to variables defined in sync_args - #[parsely_write(sync_func = "ParselyResult::Ok(payload_length_bytes + 4)")] + #[parsely_write(sync_expr = "payload_length_bytes + 4")] length_bytes: u16, }