Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove BinRead::after_parse and add offset-table helpers #210

Closed
wants to merge 3 commits into from
Closed

Remove BinRead::after_parse and add offset-table helpers #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
065fcf0
Remove `BinRead::after_parse`
csnover Jun 29, 2023
924e094
Allow mutable borrows in map functions
csnover Jul 2, 2023
f823c31
Add helper functions for reading offset tables
csnover Jul 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 5 additions & 65 deletions binrw/doc/attribute.md
View file
Open in desktop
Expand Up @@ -87,7 +87,6 @@ Glossary of directives in binrw attributes (`#[br]`, `#[bw]`, `#[brw]`).
| r | [`count`](#count) | field | Sets the length of a vector.
| r | [`dbg`](#debug) | field | Prints the value and offset of a field to `stderr`.
| r | [`default`](#ignore) | field | An alias for `ignore`.
| r | [`deref_now`](#postprocessing) | field | An alias for `postprocess_now`.
| r | [`err_context`](#backtrace) | field | Adds additional context to errors.
| rw | [`if`](#conditional-values) | field | <span class="brw">Reads or writes</span><span class="br">Reads</span><span class="bw">Writes</span> data only if a condition is true.
| rw | [`ignore`](#ignore) | field | <span class="brw">For `BinRead`, uses the [`default`](core::default::Default) value for a field instead of reading data. For `BinWrite`, skips writing the field.</span><span class="br">Uses the [`default`](core::default::Default) value for a field instead of reading data.</span><span class="bw">Skips writing the field.</span>
Expand All @@ -100,12 +99,10 @@ Glossary of directives in binrw attributes (`#[br]`, `#[bw]`, `#[brw]`).
| rw | [`map`](#map) | all except unit variant | Maps an object or value to a new value.
| rw | [`map_stream`](#stream-access-and-manipulation) | all except unit variant | Maps the <span class="br">read</span><span class="bw">write</span> stream to a new stream.
| r | [`offset`](#offset) | field | Modifies the offset used by a [`FilePtr`](crate::FilePtr) while parsing.
| r | [`offset_after`](#offset) | field | Modifies the offset used by a [`FilePtr`](crate::FilePtr) after parsing.
| rw | [`pad_after`](#padding-and-alignment) | field | Skips N bytes after <span class="br">reading</span><span class="bw">writing</span> a field.
| rw | [`pad_before`](#padding-and-alignment) | field | Skips N bytes before <span class="br">reading</span><span class="bw">writing</span> a field.
| rw | [`pad_size_to`](#padding-and-alignment) | field | Ensures the <span class="br">reader</span><span class="bw">writer</span> is always advanced at least N bytes.
| r | [`parse_with`](#custom-parserswriters) | field | Specifies a custom function for reading a field.
| r | [`postprocess_now`](#postprocessing) | field | Calls [`after_parse`](crate::BinRead::after_parse) immediately after reading data instead of after all fields have been read.
| r | [`pre_assert`](#pre-assert) | struct, non-unit enum, unit variant | Like `assert`, but checks the condition before parsing.
| rw | [`repr`](#repr) | unit-like enum | Specifies the underlying type for a unit-like (C-style) enum.
| rw | [`restore_position`](#restore-position) | field | Restores the <span class="br">reader’s</span><span class="bw">writer’s</span> position after <span class="br">reading</span><span class="bw">writing</span> a field.
Expand Down Expand Up @@ -1760,26 +1757,21 @@ reset to where it was before parsing started.

# Offset

The `offset` and `offset_after` directives are shorthands for passing
`offset` and `offset_after` arguments to a parser that operates like
[`FilePtr`](crate::FilePtr):
The `offset` directive is shorthand for passing `offset` to a parser that
operates like [`FilePtr`](crate::FilePtr):

```text
#[br(offset = $offset:expr)] or #[br(offset($offset:expr))]
#[br(offset_after = $offset:expr)] or #[br(offset_after($offset:expr))]
```

When manually implementing
[`BinRead::read_options`](crate::BinRead::read_options) or a
[custom parser function](#custom-parserswriters), the offset is accessible
from a named argument named `offset`.

For `offset`, any earlier field or [import](#arguments) can be referenced by
the expression in the directive.

For `offset_after`, *all* fields and imports can be referenced by the
expression in the directive, but [`deref_now`](#postprocessing) cannot be
used.
Any <span class="brw">(earlier only, when reading)</span><span class="br">earlier</span>
field or [import](#arguments) can be
referenced by the expression in the directive.

## Examples

Expand Down Expand Up @@ -1988,58 +1980,6 @@ started.

<div class="br">

# Postprocessing

The `deref_now` directive, and its alias `postprocess_now`, cause a
field’s [`after_parse`](crate::BinRead::after_parse) function to be called
immediately after the field is parsed, instead of deferring the call until
the entire parent object has been parsed:

```text
#[br(deref_now)] or #[br(postprocess_now)]
```

The [`BinRead::after_parse`](crate::BinRead::after_parse) function is
normally used to perform additional work after the whole parent object has
been parsed. For example, the [`FilePtr`](crate::FilePtr) type reads an
object offset during parsing with
[`read_options`](crate::BinRead::read_options), then actually seeks to and
parses the pointed-to object in `after_parse`. This improves read
performance by reading the whole parent object sequentially before seeking
to read the pointed-to object.

However, if another field in the parent object needs to access data from the
pointed-to object, `after_parse` needs to be called earlier. Adding
`deref_now` (or its alias, `postprocess_now`) to the earlier field causes
this to happen.

## Examples

```
# use binrw::{prelude::*, FilePtr32, NullString, io::Cursor};
#[derive(BinRead, Debug)]
#[br(big, magic = b"TEST")]
struct TestFile {
#[br(deref_now)]
ptr: FilePtr32<NullString>,

value: i32,

// Notice how `ptr` can be used as it has already been postprocessed
#[br(calc = ptr.len())]
ptr_len: usize,
}

# let test_contents = b"\x54\x45\x53\x54\x00\x00\x00\x10\xFF\xFF\xFF\xFF\x00\x00\x00\x00\x54\x65\x73\x74\x20\x73\x74\x72\x69\x6E\x67\x00\x00\x00\x00\x69";
# let test = Cursor::new(test_contents).read_be::<TestFile>().unwrap();
# assert_eq!(test.ptr_len, 11);
# assert_eq!(test.value, -1);
# assert_eq!(test.ptr.to_string(), "Test string");
```
</div>

<div class="br">

# Pre-assert

`pre_assert` works like [`assert`](#assert), but checks the condition before
Expand Down
60 changes: 0 additions & 60 deletions binrw/src/binread/impls.rs
View file
Open in desktop
Expand Up @@ -147,22 +147,6 @@ where
) -> BinResult<Self> {
crate::helpers::count_with(args.count, B::read_options)(reader, endian, args.inner)
}

fn after_parse<R>(
&mut self,
reader: &mut R,
endian: Endian,
args: Self::Args<'_>,
) -> BinResult<()>
where
R: Read + Seek,
{
for val in self.iter_mut() {
val.after_parse(reader, endian, args.inner.clone())?;
}

Ok(())
}
}

impl<B, const N: usize> BinRead for [B; N]
Expand All @@ -179,22 +163,6 @@ where
) -> BinResult<Self> {
array_init::try_array_init(|_| BinRead::read_options(reader, endian, args.clone()))
}

fn after_parse<R>(
&mut self,
reader: &mut R,
endian: Endian,
args: Self::Args<'_>,
) -> BinResult<()>
where
R: Read + Seek,
{
for val in self.iter_mut() {
val.after_parse(reader, endian, args.clone())?;
}

Ok(())
}
}

macro_rules! binread_tuple_impl {
Expand All @@ -211,19 +179,6 @@ macro_rules! binread_tuple_impl {
),*
))
}

fn after_parse<R: Read + Seek>(&mut self, reader: &mut R, endian: Endian, args: Self::Args<'_>) -> BinResult<()> {
let ($type1, $(
$types
),*) = self;

$type1.after_parse(reader, endian, args.clone())?;
$(
$types.after_parse(reader, endian, args.clone())?;
)*

Ok(())
}
}

binread_tuple_impl!($($types),*);
Expand Down Expand Up @@ -267,21 +222,6 @@ impl<T: BinRead> BinRead for Option<T> {
) -> BinResult<Self> {
Ok(Some(T::read_options(reader, endian, args)?))
}

fn after_parse<R>(
&mut self,
reader: &mut R,
endian: Endian,
args: Self::Args<'_>,
) -> BinResult<()>
where
R: Read + Seek,
{
match self {
Some(val) => val.after_parse(reader, endian, args),
None => Ok(()),
}
}
}

impl<T> BinRead for core::marker::PhantomData<T> {
Expand Down
41 changes: 9 additions & 32 deletions binrw/src/binread/mod.rs
View file
Open in desktop
Expand Up @@ -160,22 +160,6 @@ pub trait BinRead: Sized {
endian: Endian,
args: Self::Args<'_>,
) -> BinResult<Self>;

/// Runs any post-processing steps required to finalize construction of the
/// object.
///
/// # Errors
///
/// If post-processing fails, an [`Error`](crate::Error) variant will be
/// returned.
fn after_parse<R: Read + Seek>(
&mut self,
_: &mut R,
_: Endian,
_: Self::Args<'_>,
) -> BinResult<()> {
Ok(())
}
}

/// Extension methods for reading [`BinRead`] objects directly from a reader.
Expand All @@ -202,7 +186,7 @@ pub trait BinReaderExt: Read + Seek + Sized {
fn read_type<'a, T>(&mut self, endian: Endian) -> BinResult<T>
where
T: BinRead,
T::Args<'a>: Required + Clone,
T::Args<'a>: Required,
{
self.read_type_args(endian, T::Args::args())
}
Expand All @@ -216,7 +200,7 @@ pub trait BinReaderExt: Read + Seek + Sized {
fn read_be<'a, T>(&mut self) -> BinResult<T>
where
T: BinRead,
T::Args<'a>: Required + Clone,
T::Args<'a>: Required,
{
self.read_type(Endian::Big)
}
Expand All @@ -230,7 +214,7 @@ pub trait BinReaderExt: Read + Seek + Sized {
fn read_le<'a, T>(&mut self) -> BinResult<T>
where
T: BinRead,
T::Args<'a>: Required + Clone,
T::Args<'a>: Required,
{
self.read_type(Endian::Little)
}
Expand All @@ -244,7 +228,7 @@ pub trait BinReaderExt: Read + Seek + Sized {
fn read_ne<'a, T>(&mut self) -> BinResult<T>
where
T: BinRead,
T::Args<'a>: Required + Clone,
T::Args<'a>: Required,
{
self.read_type(Endian::NATIVE)
}
Expand All @@ -254,15 +238,11 @@ pub trait BinReaderExt: Read + Seek + Sized {
/// # Errors
///
/// If reading fails, an [`Error`](crate::Error) variant will be returned.
fn read_type_args<'a, T>(&mut self, endian: Endian, args: T::Args<'a>) -> BinResult<T>
fn read_type_args<T>(&mut self, endian: Endian, args: T::Args<'_>) -> BinResult<T>
where
T: BinRead,
T::Args<'a>: Clone,
{
let mut res = T::read_options(self, endian, args.clone())?;
res.after_parse(self, endian, args)?;

Ok(res)
T::read_options(self, endian, args)
}

/// Read `T` from the reader, assuming big-endian byte order, using the
Expand All @@ -272,10 +252,9 @@ pub trait BinReaderExt: Read + Seek + Sized {
///
/// If reading fails, an [`Error`](crate::Error) variant will be returned.
#[inline]
fn read_be_args<'a, T>(&mut self, args: T::Args<'a>) -> BinResult<T>
fn read_be_args<T>(&mut self, args: T::Args<'_>) -> BinResult<T>
where
T: BinRead,
T::Args<'a>: Clone,
{
self.read_type_args(Endian::Big, args)
}
Expand All @@ -287,10 +266,9 @@ pub trait BinReaderExt: Read + Seek + Sized {
///
/// If reading fails, an [`Error`](crate::Error) variant will be returned.
#[inline]
fn read_le_args<'a, T>(&mut self, args: T::Args<'a>) -> BinResult<T>
fn read_le_args<T>(&mut self, args: T::Args<'_>) -> BinResult<T>
where
T: BinRead,
T::Args<'a>: Clone,
{
self.read_type_args(Endian::Little, args)
}
Expand All @@ -302,10 +280,9 @@ pub trait BinReaderExt: Read + Seek + Sized {
///
/// If reading fails, an [`Error`](crate::Error) variant will be returned.
#[inline]
fn read_ne_args<'a, T>(&mut self, args: T::Args<'a>) -> BinResult<T>
fn read_ne_args<T>(&mut self, args: T::Args<'_>) -> BinResult<T>
where
T: BinRead,
T::Args<'a>: Clone,
{
self.read_type_args(Endian::NATIVE, args)
}
Expand Down