From bd7d8eafbe1b3ae43334c467aece9bf02cec0b27 Mon Sep 17 00:00:00 2001 From: Colin Snover Date: Tue, 6 Sep 2022 00:28:27 -0500 Subject: [PATCH] Move `offset` for `FilePtr` from `ReadOptions` to named arguments --- binrw/doc/attribute.md | 2 +- binrw/src/binread/options.rs | 27 +- binrw/src/file_ptr.rs | 242 ++++++++++-------- binrw/src/helpers.rs | 2 + binrw/tests/after_parse_test.rs | 8 +- binrw/tests/derive/struct.rs | 2 +- binrw/tests/ui/invalid_offset_type.stderr | 56 ++-- binrw_derive/src/codegen/mod.rs | 35 ++- binrw_derive/src/codegen/read_options.rs | 12 - .../src/codegen/read_options/struct.rs | 44 ++-- binrw_derive/src/parser/field_level_attrs.rs | 31 ++- 11 files changed, 252 insertions(+), 209 deletions(-) diff --git a/binrw/doc/attribute.md b/binrw/doc/attribute.md index 753e17d5..b755efb9 100644 --- a/binrw/doc/attribute.md +++ b/binrw/doc/attribute.md @@ -1658,7 +1658,7 @@ The `offset` and `offset_after` directives are shorthands for passing When manually implementing [`BinRead::read_options`](crate::BinRead::read_options) or a [custom parser function](#custom-parserswriters), the offset is accessible -from [`ReadOptions::offset`](crate::ReadOptions::offset). +from a named argument named `offset`. For `offset`, any earlier field or [import](#arguments) can be referenced by the expression in the directive. diff --git a/binrw/src/binread/options.rs b/binrw/src/binread/options.rs index 811e24d5..9e896c21 100644 --- a/binrw/src/binread/options.rs +++ b/binrw/src/binread/options.rs @@ -12,20 +12,13 @@ pub struct ReadOptions { /// [byte order directives](crate::docs::attribute#byte-order), this option /// will be overridden by the directive. endian: Endian, - - /// An absolute offset added to the [`FilePtr::ptr`](crate::FilePtr::ptr) - /// offset before reading the pointed-to value. - offset: u64, } impl ReadOptions { /// Creates a new `ReadOptions` with the given [endianness](crate::Endian). #[must_use] pub fn new(endian: Endian) -> Self { - Self { - endian, - offset: <_>::default(), - } + Self { endian } } /// The [byte order](crate::Endian) to use when reading data. @@ -38,24 +31,12 @@ impl ReadOptions { self.endian } - /// An absolute offset added to the [`FilePtr::ptr`](crate::FilePtr::ptr) - /// offset before reading the pointed-to value. - #[must_use] - pub fn offset(&self) -> u64 { - self.offset - } - /// Creates a copy of this `ReadOptions` using the given /// [endianness](crate::Endian). #[must_use] + // Lint: API compatibility. + #[allow(clippy::unused_self)] pub fn with_endian(self, endian: Endian) -> Self { - Self { endian, ..self } - } - - /// Creates a copy of this `ReadOptions` using the given - /// [offset](crate::docs::attribute#offset). - #[must_use] - pub fn with_offset(self, offset: u64) -> Self { - Self { offset, ..self } + Self { endian } } } diff --git a/binrw/src/file_ptr.rs b/binrw/src/file_ptr.rs index e42198ce..39aea459 100644 --- a/binrw/src/file_ptr.rs +++ b/binrw/src/file_ptr.rs @@ -8,11 +8,34 @@ use core::num::{ }; use core::ops::{Deref, DerefMut}; +use crate::NamedArgs; use crate::{ io::{Read, Seek, SeekFrom}, BinRead, BinResult, ReadOptions, }; +/// A type alias for [`FilePtr`] with 8-bit offsets. +pub type FilePtr8 = FilePtr; +/// A type alias for [`FilePtr`] with 16-bit offsets. +pub type FilePtr16 = FilePtr; +/// A type alias for [`FilePtr`] with 32-bit offsets. +pub type FilePtr32 = FilePtr; +/// A type alias for [`FilePtr`] with 64-bit offsets. +pub type FilePtr64 = FilePtr; +/// A type alias for [`FilePtr`] with 128-bit offsets. +pub type FilePtr128 = FilePtr; + +/// A type alias for [`FilePtr`] with non-zero 8-bit offsets. +pub type NonZeroFilePtr8 = FilePtr; +/// A type alias for [`FilePtr`] with non-zero 16-bit offsets. +pub type NonZeroFilePtr16 = FilePtr; +/// A type alias for [`FilePtr`] with non-zero 32-bit offsets. +pub type NonZeroFilePtr32 = FilePtr; +/// A type alias for [`FilePtr`] with non-zero 64-bit offsets. +pub type NonZeroFilePtr64 = FilePtr; +/// A type alias for [`FilePtr`] with non-zero 128-bit offsets. +pub type NonZeroFilePtr128 = FilePtr; + /// A wrapper type which represents a layer of indirection within a file. /// /// `FilePtr` is composed of two types. The pointer type `P` is the @@ -55,30 +78,8 @@ pub struct FilePtr { pub value: Option, } -/// A type alias for [`FilePtr`] with 8-bit offsets. -pub type FilePtr8 = FilePtr; -/// A type alias for [`FilePtr`] with 16-bit offsets. -pub type FilePtr16 = FilePtr; -/// A type alias for [`FilePtr`] with 32-bit offsets. -pub type FilePtr32 = FilePtr; -/// A type alias for [`FilePtr`] with 64-bit offsets. -pub type FilePtr64 = FilePtr; -/// A type alias for [`FilePtr`] with 128-bit offsets. -pub type FilePtr128 = FilePtr; - -/// A type alias for [`FilePtr`] with non-zero 8-bit offsets. -pub type NonZeroFilePtr8 = FilePtr; -/// A type alias for [`FilePtr`] with non-zero 16-bit offsets. -pub type NonZeroFilePtr16 = FilePtr; -/// A type alias for [`FilePtr`] with non-zero 32-bit offsets. -pub type NonZeroFilePtr32 = FilePtr; -/// A type alias for [`FilePtr`] with non-zero 64-bit offsets. -pub type NonZeroFilePtr64 = FilePtr; -/// A type alias for [`FilePtr`] with non-zero 128-bit offsets. -pub type NonZeroFilePtr128 = FilePtr; - -impl + IntoSeekFrom, BR: BinRead> BinRead for FilePtr { - type Args = BR::Args; +impl + IntoSeekFrom, Value: BinRead> BinRead for FilePtr { + type Args = FilePtrArgs; /// Reads the offset of the value from the reader. /// @@ -96,27 +97,34 @@ impl + IntoSeekFrom, BR: BinRead> BinRead for FilePtr(&mut self, reader: &mut R, ro: &ReadOptions, args: BR::Args) -> BinResult<()> + fn after_parse( + &mut self, + reader: &mut R, + ro: &ReadOptions, + args: FilePtrArgs, + ) -> BinResult<()> where R: Read + Seek, { - self.after_parse_with_parser(BR::read_options, BR::after_parse, reader, ro, args) + self.after_parse_with_parser(Value::read_options, Value::after_parse, reader, ro, args) } } -impl + IntoSeekFrom, T> FilePtr { +impl + IntoSeekFrom, Value> FilePtr { + // Lint: Non-consumed argument is required to match the API. + #[allow(clippy::trivially_copy_pass_by_ref)] fn read_with_parser( parser: Parser, after_parse: AfterParse, reader: &mut R, options: &ReadOptions, - args: Args, + args: FilePtrArgs, ) -> BinResult where R: Read + Seek, Args: Clone, - Parser: Fn(&mut R, &ReadOptions, Args) -> BinResult, - AfterParse: Fn(&mut T, &mut R, &ReadOptions, Args) -> BinResult<()>, + Parser: Fn(&mut R, &ReadOptions, Args) -> BinResult, + AfterParse: Fn(&mut Value, &mut R, &ReadOptions, Args) -> BinResult<()>, { let mut file_ptr = Self { ptr: Ptr::read_options(reader, options, ())?, @@ -126,28 +134,30 @@ impl + IntoSeekFrom, T> FilePtr { Ok(file_ptr) } + // Lint: Non-consumed argument is required to match the API. + #[allow(clippy::trivially_copy_pass_by_ref)] fn after_parse_with_parser( &mut self, parser: Parser, after_parse: AfterParse, reader: &mut R, options: &ReadOptions, - args: Args, + args: FilePtrArgs, ) -> BinResult<()> where R: Read + Seek, Args: Clone, - Parser: Fn(&mut R, &ReadOptions, Args) -> BinResult, - AfterParse: Fn(&mut T, &mut R, &ReadOptions, Args) -> BinResult<()>, + Parser: Fn(&mut R, &ReadOptions, Args) -> BinResult, + AfterParse: Fn(&mut Value, &mut R, &ReadOptions, Args) -> BinResult<()>, { - let relative_to = options.offset(); + let relative_to = args.offset; let before = reader.stream_position()?; reader.seek(SeekFrom::Start(relative_to))?; reader.seek(self.ptr.into_seek_from())?; - let mut inner: T = parser(reader, options, args.clone())?; + let mut inner: Value = parser(reader, options, args.inner.clone())?; - after_parse(&mut inner, reader, options, args)?; + after_parse(&mut inner, reader, options, args.inner)?; reader.seek(SeekFrom::Start(before))?; self.value = Some(inner); @@ -162,16 +172,24 @@ impl + IntoSeekFrom, T> FilePtr { /// # Errors /// /// If reading fails, an [`Error`](crate::Error) variant will be returned. - pub fn parse(reader: &mut R, options: &ReadOptions, args: Args) -> BinResult + pub fn parse( + reader: &mut R, + options: &ReadOptions, + args: FilePtrArgs, + ) -> BinResult where R: Read + Seek, Args: Clone, - T: BinRead, + Value: BinRead, { - Ok( - Self::read_with_parser(T::read_options, T::after_parse, reader, options, args)? - .into_inner(), - ) + Ok(Self::read_with_parser( + Value::read_options, + Value::after_parse, + reader, + options, + args, + )? + .into_inner()) } /// Custom parser for use with the @@ -182,14 +200,16 @@ impl + IntoSeekFrom, T> FilePtr { /// # Errors /// /// If reading fails, an [`Error`](crate::Error) variant will be returned. - pub fn parse_with(parser: F) -> impl Fn(&mut R, &ReadOptions, Args) -> BinResult + pub fn parse_with( + parser: F, + ) -> impl Fn(&mut R, &ReadOptions, FilePtrArgs) -> BinResult where R: Read + Seek, Args: Clone, - F: Fn(&mut R, &ReadOptions, Args) -> BinResult, + F: Fn(&mut R, &ReadOptions, Args) -> BinResult, { move |reader, ro, args| { - let after_parse = |_: &mut T, _: &mut R, _: &ReadOptions, _: Args| Ok(()); + let after_parse = |_: &mut Value, _: &mut R, _: &ReadOptions, _: Args| Ok(()); Ok(Self::read_with_parser(&parser, after_parse, reader, ro, args)?.into_inner()) } } @@ -202,14 +222,16 @@ impl + IntoSeekFrom, T> FilePtr { /// # Errors /// /// If reading fails, an [`Error`](crate::Error) variant will be returned. - pub fn with(parser: F) -> impl Fn(&mut R, &ReadOptions, Args) -> BinResult + pub fn with( + parser: F, + ) -> impl Fn(&mut R, &ReadOptions, FilePtrArgs) -> BinResult where R: Read + Seek, Args: Clone, - F: Fn(&mut R, &ReadOptions, Args) -> BinResult, + F: Fn(&mut R, &ReadOptions, Args) -> BinResult, { move |reader, ro, args| { - let after_parse = |_: &mut T, _: &mut R, _: &ReadOptions, _: Args| Ok(()); + let after_parse = |_: &mut Value, _: &mut R, _: &ReadOptions, _: Args| Ok(()); Self::read_with_parser(&parser, after_parse, reader, ro, args) } } @@ -220,11 +242,68 @@ impl + IntoSeekFrom, T> FilePtr { /// /// Will panic if `FilePtr` hasn’t been finalized by calling /// [`after_parse()`](Self::after_parse). - pub fn into_inner(self) -> T { + pub fn into_inner(self) -> Value { self.value.unwrap() } } +/// Dereferences the value. +/// +/// # Panics +/// +/// Will panic if `FilePtr` hasn’t been finalized by calling +/// [`after_parse()`](Self::after_parse). +impl Deref for FilePtr { + type Target = Value; + + fn deref(&self) -> &Self::Target { + match self.value.as_ref() { + Some(x) => x, + None => panic!( + "Deref'd FilePtr before reading (make sure to use FilePtr::after_parse first)" + ), + } + } +} + +/// # Panics +/// Will panic if the `FilePtr` has not been read yet using +/// [`BinRead::after_parse`](BinRead::after_parse) +impl DerefMut for FilePtr { + fn deref_mut(&mut self) -> &mut Value { + match self.value.as_mut() { + Some(x) => x, + None => panic!( + "Deref'd FilePtr before reading (make sure to use FilePtr::after_parse first)" + ), + } + } +} + +impl fmt::Debug for FilePtr +where + Ptr: fmt::Debug + IntoSeekFrom, + Value: fmt::Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(ref value) = self.value { + fmt::Debug::fmt(value, f) + } else { + f.debug_tuple("UnreadPointer").field(&self.ptr).finish() + } + } +} + +impl PartialEq> for FilePtr +where + Ptr: PartialEq + IntoSeekFrom, + Value: PartialEq, +{ + fn eq(&self, other: &Self) -> bool { + self.value == other.value + } +} + /// A trait to convert from an integer into /// [`SeekFrom::Current`](crate::io::SeekFrom::Current). pub trait IntoSeekFrom: Copy + fmt::Debug { @@ -271,59 +350,18 @@ impl_into_seek_from_for_non_zero!( NonZeroU8 ); -/// Dereferences the value. -/// -/// # Panics +/// Named arguments for the [`BinRead::read_options()`] implementation of [`FilePtr`]. /// -/// Will panic if `FilePtr` hasn’t been finalized by calling -/// [`after_parse()`](Self::after_parse). -impl Deref for FilePtr { - type Target = BR; +/// The `inner` field can be omitted completely if the inner type doesn’t +/// require arguments, in which case a default value will be used. +#[derive(Clone, Default, NamedArgs)] +pub struct FilePtrArgs { + /// An absolute offset added to the [`FilePtr::ptr`](crate::FilePtr::ptr) + /// offset before reading the pointed-to value. + #[named_args(default = 0)] + pub offset: u64, - fn deref(&self) -> &Self::Target { - match self.value.as_ref() { - Some(x) => x, - None => panic!( - "Deref'd FilePtr before reading (make sure to use FilePtr::after_parse first)" - ), - } - } -} - -/// # Panics -/// Will panic if the `FilePtr` has not been read yet using -/// [`BinRead::after_parse`](BinRead::after_parse) -impl DerefMut for FilePtr { - fn deref_mut(&mut self) -> &mut BR { - match self.value.as_mut() { - Some(x) => x, - None => panic!( - "Deref'd FilePtr before reading (make sure to use FilePtr::after_parse first)" - ), - } - } -} - -impl fmt::Debug for FilePtr -where - Ptr: BinRead + IntoSeekFrom, - BR: BinRead + fmt::Debug, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - if let Some(ref value) = self.value { - fmt::Debug::fmt(value, f) - } else { - f.debug_tuple("UnreadPointer").field(&self.ptr).finish() - } - } -} - -impl PartialEq> for FilePtr -where - Ptr: BinRead + IntoSeekFrom, - BR: BinRead + PartialEq, -{ - fn eq(&self, other: &Self) -> bool { - **self == **other - } + /// The [arguments](crate::BinRead::Args) for the inner type. + #[named_args(try_optional)] + pub inner: Inner, } diff --git a/binrw/src/helpers.rs b/binrw/src/helpers.rs index e5b2aa5d..e98262ae 100644 --- a/binrw/src/helpers.rs +++ b/binrw/src/helpers.rs @@ -375,6 +375,8 @@ where } } +// Lint: Non-consumed argument is required to match the API. +#[allow(clippy::trivially_copy_pass_by_ref)] fn default_reader>( reader: &mut R, options: &ReadOptions, diff --git a/binrw/tests/after_parse_test.rs b/binrw/tests/after_parse_test.rs index 44004a0b..1c6c1190 100644 --- a/binrw/tests/after_parse_test.rs +++ b/binrw/tests/after_parse_test.rs @@ -8,12 +8,12 @@ fn BinReaderExt_calls_after_parse() { assert_eq!(*test, 0xFF); } -#[derive(BinRead)] -struct Try>(#[br(try)] Option
); - #[test] fn try_calls_after_parse() { - let test: Try> = Cursor::new([0x01, 0xFF]).read_be().unwrap(); + #[derive(BinRead)] + struct Try, Args: Default + 'static>(#[br(try)] Option
); + + let test: Try, _> = Cursor::new([0x01, 0xFF]).read_be().unwrap(); assert_eq!(*test.0.unwrap(), 0xFF) } diff --git a/binrw/tests/derive/struct.rs b/binrw/tests/derive/struct.rs index 7366d899..3ac53680 100644 --- a/binrw/tests/derive/struct.rs +++ b/binrw/tests/derive/struct.rs @@ -16,7 +16,7 @@ fn all_the_things() { struct Test { extra_entry_count: u32, - #[br(count = extra_entry_count + 1, args { inner: args! { extra_val: 0x69 } })] + #[br(count = extra_entry_count + 1, args { inner: args! { inner: args! { extra_val: 0x69 } } })] entries: Vec>, #[br(default)] diff --git a/binrw/tests/ui/invalid_offset_type.stderr b/binrw/tests/ui/invalid_offset_type.stderr index bb6a6cca..d80cf361 100644 --- a/binrw/tests/ui/invalid_offset_type.stderr +++ b/binrw/tests/ui/invalid_offset_type.stderr @@ -1,37 +1,37 @@ error[E0308]: mismatched types - --> tests/ui/invalid_offset_type.rs:6:19 - | -3 | #[derive(BinRead)] - | ------- arguments to this function are incorrect + --> tests/ui/invalid_offset_type.rs:6:19 + | +3 | #[derive(BinRead)] + | ------- arguments to this function are incorrect ... -6 | #[br(offset = a)] - | ^ expected `u64`, found `u8` - | +6 | #[br(offset = a)] + | ^ expected `u64`, found `u8` + | note: associated function defined here - --> src/binread/options.rs - | - | pub fn with_offset(self, offset: u64) -> Self { - | ^^^^^^^^^^^ + --> src/file_ptr.rs + | + | pub offset: u64, + | ^^^^^^ help: you can convert a `u8` to a `u64` - | -6 | #[br(offset = a.into())] - | +++++++ + | +6 | #[br(offset = a.into())] + | +++++++ error[E0308]: mismatched types - --> tests/ui/invalid_offset_type.rs:8:25 - | -3 | #[derive(BinRead)] - | ------- arguments to this function are incorrect + --> tests/ui/invalid_offset_type.rs:8:25 + | +3 | #[derive(BinRead)] + | ------- arguments to this function are incorrect ... -8 | #[br(offset_after = d)] - | ^ expected `u64`, found `u8` - | +8 | #[br(offset_after = d)] + | ^ expected `u64`, found `u8` + | note: associated function defined here - --> src/binread/options.rs - | - | pub fn with_offset(self, offset: u64) -> Self { - | ^^^^^^^^^^^ + --> src/file_ptr.rs + | + | pub offset: u64, + | ^^^^^^ help: you can convert a `u8` to a `u64` - | -8 | #[br(offset_after = d.into())] - | +++++++ + | +8 | #[br(offset_after = d.into())] + | +++++++ diff --git a/binrw_derive/src/codegen/mod.rs b/binrw_derive/src/codegen/mod.rs index e944b2d0..ae690ef8 100644 --- a/binrw_derive/src/codegen/mod.rs +++ b/binrw_derive/src/codegen/mod.rs @@ -183,13 +183,10 @@ fn get_destructured_imports( fn get_passed_args(field: &StructField) -> Option { let args = &field.args; match args { - PassedArgs::Named(fields) => Some(if let Some(count) = &field.count { + PassedArgs::Named(fields) => Some({ + let extra_args = directives_to_args(field); quote! { - #ARGS_MACRO! { count: ((#count) as usize) #(, #fields)* } - } - } else { - quote! { - #ARGS_MACRO! { #(#fields),* } + #ARGS_MACRO! { #extra_args #(#fields, )* } } }), PassedArgs::List(list) => Some(quote! { (#(#list,)*) }), @@ -197,14 +194,32 @@ fn get_passed_args(field: &StructField) -> Option { let tuple = tuple.as_ref(); Some(quote! { #tuple }) } - PassedArgs::None => field - .count - .as_ref() - .map(|count| quote! { #ARGS_MACRO! { count: ((#count) as usize) }}), + PassedArgs::None => { + let extra_args = directives_to_args(field); + (!extra_args.is_empty()).then(|| { + quote! { #ARGS_MACRO! { #extra_args } } + }) + } } .map(|ts| fixup_span(ts, args.span().unwrap_or_else(|| field.ty.span()))) } +fn directives_to_args(field: &StructField) -> TokenStream { + let args = field + .count + .as_ref() + .map(|count| quote! { count: ((#count) as usize) }) + .into_iter() + .chain( + field + .offset + .as_ref() + .map(|offset| quote! { offset: (#offset) }) + .into_iter(), + ); + quote! { #(#args,)* } +} + // For an unknown reason, this seems to be the least invasive way to associate // the arguments correctly with the args token; quote_spanned does not get it // done and neither does only resetting the span on only the generated tokens diff --git a/binrw_derive/src/codegen/read_options.rs b/binrw_derive/src/codegen/read_options.rs index dcdb3b42..fabf95c6 100644 --- a/binrw_derive/src/codegen/read_options.rs +++ b/binrw_derive/src/codegen/read_options.rs @@ -193,16 +193,4 @@ impl ReadOptionsGenerator { } } } - - fn offset(mut self, offset: &Option) -> Self { - if let Some(offset) = &offset { - let head = self.out; - self.out = quote! { - #head - let #TEMP = #TEMP.with_offset(#offset); - }; - } - - self - } } diff --git a/binrw_derive/src/codegen/read_options/struct.rs b/binrw_derive/src/codegen/read_options/struct.rs index 6b345ed4..c758bce4 100644 --- a/binrw_derive/src/codegen/read_options/struct.rs +++ b/binrw_derive/src/codegen/read_options/struct.rs @@ -5,8 +5,8 @@ use crate::{ codegen::{ get_assertions, get_passed_args, sanitization::{ - make_ident, IdentStr, AFTER_PARSE, ARGS_TYPE_HINT, BACKTRACE_FRAME, BINREAD_TRAIT, - COERCE_FN, MAP_ARGS_TYPE_HINT, POS, READER, READ_FUNCTION, READ_METHOD, + make_ident, IdentStr, AFTER_PARSE, ARGS_MACRO, ARGS_TYPE_HINT, BACKTRACE_FRAME, + BINREAD_TRAIT, COERCE_FN, MAP_ARGS_TYPE_HINT, POS, READER, READ_FUNCTION, READ_METHOD, REQUIRED_ARG_TRAIT, SAVED_POSITION, SEEK_FROM, SEEK_TRAIT, TEMP, WITH_CONTEXT, }, }, @@ -117,7 +117,6 @@ fn generate_after_parse(field: &StructField) -> Option { AfterParseCallGenerator::new(field) .get_value_from_ident() .call_after_parse(after_parse_fn, &options_var, &args_var) - .prefix_offset_options(&options_var) .finish() }) } else { @@ -175,7 +174,24 @@ impl<'field> AfterParseCallGenerator<'field> { let options_var = options_var.as_ref().expect( "called `AfterParseCallGenerator::call_after_parse` but no `options_var` was generated", ); - let args_arg = get_args_argument(self.field, args_var); + + let args_arg = if let Some(offset) = &self.field.offset_after { + let offset = offset.as_ref(); + if let Some(args_var) = args_var { + quote! {{ + let mut #TEMP = #args_var; + #TEMP.offset = #offset; + #TEMP + }} + } else { + quote! { + #ARGS_MACRO! { offset: (#offset) } + } + } + } else { + get_args_argument(self.field, args_var) + }; + self.out = quote! { #after_parse_fn(#value, #READER, #options_var, #args_arg)?; }; @@ -199,23 +215,6 @@ impl<'field> AfterParseCallGenerator<'field> { self } - - fn prefix_offset_options(mut self, options_var: &Option) -> Self { - if let (Some(options_var), Some(offset)) = (options_var, &self.field.offset_after) { - let tail = self.out; - let offset = offset.as_ref(); - self.out = quote! { - let #options_var = &{ - let mut #TEMP = *#options_var; - let #TEMP = #TEMP.with_offset(#offset); - #TEMP - }; - #tail - }; - } - - self - } } struct FieldGenerator<'field> { @@ -388,7 +387,6 @@ impl<'field> FieldGenerator<'field> { let options = self.options_var.as_ref().map(|options_var| { ReadOptionsGenerator::new(options_var) .endian(&self.field.endian) - .offset(&self.field.offset) .finish() }); @@ -616,7 +614,7 @@ fn get_return_type(variant_ident: Option<&Ident>) -> TokenStream { } fn make_field_vars(field: &StructField) -> (Option, Option) { - let args_var = if field.args.is_some() || field.count.is_some() { + let args_var = if field.needs_args() { Some(make_ident(&field.ident, "args")) } else { None diff --git a/binrw_derive/src/parser/field_level_attrs.rs b/binrw_derive/src/parser/field_level_attrs.rs index 168055e2..7c21c2ea 100644 --- a/binrw_derive/src/parser/field_level_attrs.rs +++ b/binrw_derive/src/parser/field_level_attrs.rs @@ -96,6 +96,17 @@ impl StructField { !self.generated_value() || self.magic.is_some() } + /// Returns true if the field requires arguments. + pub(crate) fn needs_args(&self) -> bool { + self.args.is_some() || self.count.is_some() || self.offset.is_some() + } + + /// Returns true if the field is using shorthand directives that are + /// converted into named arguments. + pub(crate) fn has_named_arg_directives(&self) -> bool { + self.count.is_some() || self.offset.is_some() || self.offset_after.is_some() + } + /// Returns true if the only field-level attributes are asserts pub(crate) fn has_no_attrs(&self) -> bool { macro_rules! all_fields_none { @@ -175,7 +186,9 @@ impl StructField { ); } - if self.count.is_some() && !matches!(self.args, PassedArgs::None | PassedArgs::Named(..)) { + if self.has_named_arg_directives() + && !matches!(self.args, PassedArgs::None | PassedArgs::Named(..)) + { let (span, repr) = match &self.args { PassedArgs::Named(_) | PassedArgs::None => unreachable!(), PassedArgs::List(list) => ( @@ -189,10 +202,18 @@ impl StructField { PassedArgs::Tuple(raw) => (raw.span(), raw.to_string()), }; - combine_error(&mut all_errors, syn::Error::new( - span, - format!("`count` can only be used with named args; did you mean `args {{ inner: {} }}`?", repr) - )); + for (used, name) in [ + (self.count.is_some(), "count"), + (self.offset.is_some(), "offset"), + (self.offset_after.is_some(), "offset_after"), + ] { + if used { + combine_error(&mut all_errors, syn::Error::new( + span, + format!("`{}` can only be used with named args; did you mean `args {{ inner: {} }}`?", name, repr) + )); + } + } } if let Some(error) = all_errors {