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.

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

Remove the default bound from Packable::Visitor #76

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packable/packable-derive-test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ name = "tests"
path = "tests/lib.rs"

[dev-dependencies]
packable = { version = "=0.10.1", path = "../packable", default-features = false }
packable = { version = "=0.11.0", path = "../packable", default-features = false }

rustversion = { version = "1.0.14", default-features = false }
trybuild = { version = "1.0.88", default-features = false, features = ["diff"] }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ impl From<Infallible> for PickyError {
}
}

fn verify_value<const VERIFY: bool>(&value: &u64) -> Result<(), PickyError> {
if !VERIFY || value == 42 {
fn verify_value(&value: &u64) -> Result<(), PickyError> {
if value == 42 {
Ok(())
} else {
Err(PickyError(value as u8))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ error[E0308]: mismatched types
--> tests/fail/invalid_verify_with_field_type.rs:32:10
|
32 | #[derive(Packable)]
| ^^^^^^^^
| |
| expected `&u64`, found `&u8`
| arguments to this function are incorrect
| ^^^^^^^^ expected `&u64`, found `&u8`
33 | #[packable(unpack_error = PickyError)]
34 | pub struct Picky(#[packable(verify_with = verify_value)] u8);
| ------------ arguments to this function are incorrect
|
= note: expected reference `&u64`
found reference `&u8`
note: function defined here
--> tests/fail/invalid_verify_with_field_type.rs:24:4
|
24 | fn verify_value<const VERIFY: bool>(&value: &u64) -> Result<(), PickyError> {
| ^^^^^^^^^^^^ ------------
24 | fn verify_value(&value: &u64) -> Result<(), PickyError> {
| ^^^^^^^^^^^^ ------------
= note: this error originates in the derive macro `Packable` (in Nightly builds, run with -Z macro-backtrace for more info)
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ impl From<Infallible> for PickyError {
}
}

fn verify<const VERIFY: bool>(&value: &u64) -> Result<(), PickyError> {
if !VERIFY || value == 42 {
fn verify(&value: &u64) -> Result<(), PickyError> {
if value == 42 {
Ok(())
} else {
Err(PickyError(value as u8))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ error[E0308]: mismatched types
--> tests/fail/invalid_verify_with_struct_type.rs:32:10
|
32 | #[derive(Packable)]
| ^^^^^^^^
| |
| expected `&u64`, found `&Picky`
| arguments to this function are incorrect
| ^^^^^^^^ expected `&u64`, found `&Picky`
33 | #[packable(unpack_error = PickyError)]
34 | #[packable(verify_with = verify)]
| ------ arguments to this function are incorrect
|
= note: expected reference `&u64`
found reference `&Picky`
note: function defined here
--> tests/fail/invalid_verify_with_struct_type.rs:24:4
|
24 | fn verify<const VERIFY: bool>(&value: &u64) -> Result<(), PickyError> {
| ^^^^^^ ------------
24 | fn verify(&value: &u64) -> Result<(), PickyError> {
| ^^^^^^ ------------
= note: this error originates in the derive macro `Packable` (in Nightly builds, run with -Z macro-backtrace for more info)
2 changes: 1 addition & 1 deletion packable/packable-derive-test/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,4 @@ macro_rules! make_test {
#[rustversion::stable]
make_test!();
#[rustversion::not(stable)]
make_test!();
make_test!(incorrect_tag_enum);
6 changes: 3 additions & 3 deletions packable/packable-derive-test/tests/pass/error_coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ impl Packable for Picky {
self.0.pack(packer)
}

fn unpack<U: Unpacker, const VERIFY: bool>(
fn unpack<U: Unpacker>(
unpacker: &mut U,
visitor: &Self::UnpackVisitor,
visitor: Option<&Self::UnpackVisitor>,
) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
let value = u8::unpack::<_, VERIFY>(unpacker, visitor).coerce()?;
let value = u8::unpack(unpacker, visitor).coerce()?;

if value == 42 {
Ok(Self(value))
Expand Down
8 changes: 2 additions & 6 deletions packable/packable-derive-test/tests/pass/verify_with_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,8 @@ impl From<Infallible> for PickyError {
}
}

fn verify_value<const VERIFY: bool>(&value: &u8) -> Result<(), PickyError> {
if !VERIFY || value == 42 {
Ok(())
} else {
Err(PickyError(value))
}
fn verify_value(&value: &u8) -> Result<(), PickyError> {
if value == 42 { Ok(()) } else { Err(PickyError(value)) }
}

#[derive(Packable)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,8 @@ impl From<Infallible> for PickyError {
}
}

fn verify_value<const VERIFY: bool>(&value: &u8, _: &()) -> Result<(), PickyError> {
if !VERIFY || value == 42 {
Ok(())
} else {
Err(PickyError(value))
}
fn verify_value(&value: &u8, _: &()) -> Result<(), PickyError> {
if value == 42 { Ok(()) } else { Err(PickyError(value)) }
}

#[derive(Packable)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ impl From<Infallible> for PickyError {
}
}

fn verify<const VERIFY: bool>(value: &Picky) -> Result<(), PickyError> {
if !VERIFY || value.0 == 42 {
fn verify(value: &Picky) -> Result<(), PickyError> {
if value.0 == 42 {
Ok(())
} else {
Err(PickyError(value.0))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ impl From<Infallible> for PickyError {
}
}

fn verify<const VERIFY: bool>(value: &Picky, _: &()) -> Result<(), PickyError> {
if !VERIFY || value.0 == 42 {
fn verify(value: &Picky, _: &()) -> Result<(), PickyError> {
if value.0 == 42 {
Ok(())
} else {
Err(PickyError(value.0))
Expand Down
7 changes: 7 additions & 0 deletions packable/packable-derive/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Security -->

## 0.10.0 - 2024-02-09

### Changed

- Updated to new `Packable` trait;
- `verify_with` function is no longer called if the visitor is `None`;

## 0.9.0 - 2023-11-17

### Changed
Expand Down
2 changes: 1 addition & 1 deletion packable/packable-derive/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "packable-derive"
version = "0.9.0"
version = "0.10.0"
authors = ["IOTA Stiftung"]
edition = "2021"
description = "Derive macro for the `packable` crate."
Expand Down
40 changes: 29 additions & 11 deletions packable/packable-derive/src/fragments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,39 @@ impl Fragments {
fields_type,
} = info;

let fields_verification = fields_verify_with.into_iter().zip(fields_ident.iter()).map(|(verify_with, field_ident)| match verify_with {
Some(verify_with) => if unpack_visitor_info.explicit {
quote!(#verify_with::<VERIFY>(&#field_ident, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;)
} else {
quote!(#verify_with::<VERIFY>(&#field_ident).map_err(#crate_name::error::UnpackError::from_packable)?;)
}
None => quote!(),
});
let fields_verification = fields_verify_with.into_iter().zip(fields_ident.iter()).map(
|(verify_with, field_ident)| match verify_with {
Some(verify_with) => if unpack_visitor_info.explicit {
quote! {
if let Some(visitor) = visitor {
#verify_with(&#field_ident, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;
}
}
} else {
quote! {
if visitor.is_some() {
#verify_with(&#field_ident).map_err(#crate_name::error::UnpackError::from_packable)?;
}
Thoralf-M marked this conversation as resolved.
Show resolved Hide resolved
}
},
None => quote!(),
},
);

let verify_with = match verify_with {
Some(verify_with) => {
if unpack_visitor_info.explicit {
quote!(#verify_with::<VERIFY>(&unpacked, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;)
quote! {
if let Some(visitor) = visitor {
#verify_with(&unpacked, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;
}
}
} else {
quote!(#verify_with::<VERIFY>(&unpacked).map_err(#crate_name::error::UnpackError::from_packable)?;)
quote! {
if visitor.is_some() {
#verify_with(&unpacked).map_err(#crate_name::error::UnpackError::from_packable)?;
}
}
}
}
None => quote!(),
Expand All @@ -60,7 +78,7 @@ impl Fragments {
},
unpack: quote! {
#(
let #fields_ident = <#fields_type as #crate_name::Packable>::unpack::<_, VERIFY>(unpacker, Borrow::<<#fields_type as #crate_name::Packable>::UnpackVisitor>::borrow(visitor)).map_packable_err(#fields_unpack_error_with).coerce()?;
let #fields_ident = <#fields_type as #crate_name::Packable>::unpack(unpacker, visitor.map(Borrow::<<#fields_type as #crate_name::Packable>::UnpackVisitor>::borrow)).map_packable_err(#fields_unpack_error_with).coerce()?;
#fields_verification
)*

Expand Down
4 changes: 2 additions & 2 deletions packable/packable-derive/src/trait_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl TraitImpl {
#(#tag_decls)*
#(#tag_asserts)*

match <#tag_type as #crate_name::Packable>::unpack::<_, VERIFY>(unpacker, Borrow::<<#tag_type as #crate_name::Packable>::UnpackVisitor>::borrow(visitor)).coerce()? {
match <#tag_type as #crate_name::Packable>::unpack(unpacker, visitor.map(Borrow::<<#tag_type as #crate_name::Packable>::UnpackVisitor>::borrow)).coerce()? {
#(#unpack_arms)*
tag => Err(#crate_name::error::UnpackError::from_packable(#tag_with_error(tag)))
}
Expand Down Expand Up @@ -153,7 +153,7 @@ impl ToTokens for TraitImpl {
#pack
}

fn unpack<U: #crate_name::unpacker::Unpacker, const VERIFY: bool>(unpacker: &mut U, visitor: &Self::UnpackVisitor) -> Result<Self, #crate_name::error::UnpackError<Self::UnpackError, U::Error>> {
fn unpack<U: #crate_name::unpacker::Unpacker>(unpacker: &mut U, visitor: Option<&Self::UnpackVisitor>) -> Result<Self, #crate_name::error::UnpackError<Self::UnpackError, U::Error>> {
use #crate_name::error::UnpackErrorExt;
use core::borrow::Borrow;
#unpack
Expand Down
12 changes: 12 additions & 0 deletions packable/packable/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Security -->

## 0.11.0 - 2024-02-09

### Added

- `Packable::unpack_inner`, `Packable::unpack_verified`, and `Packable::unpack_unverified`;

### Changed

- Removed `Default` bound from `Packable::UnpackVisitor`;
- Replaced `const VERIFY` in `Packable::unpack` with `Option<&Self::UnpackVisitor>`, which can be checked instead;
- Renamed `PackableExt::unpack_verified` and `unpack_unverified` to `unpack_bytes_verified` and `unpack_bytes_unverified`;

## 0.10.1 - 2024-01-08

### Added
Expand Down
5 changes: 3 additions & 2 deletions packable/packable/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "packable"
version = "0.10.1"
version = "0.11.0"
authors = ["IOTA Stiftung"]
edition = "2021"
description = "A crate for packing and unpacking binary representations."
Expand All @@ -20,7 +20,7 @@ usize = []
autocfg = { version = "1.1.0", default-features = false }

[dependencies]
packable-derive = { version = "=0.9.0", path = "../packable-derive", default-features = false }
packable-derive = { version = "=0.10.0", path = "../packable-derive", default-features = false }

hashbrown = { version = "0.14.3", default-features = false, features = [
"ahash",
Expand All @@ -29,4 +29,5 @@ hashbrown = { version = "0.14.3", default-features = false, features = [
primitive-types = { version = "0.12.2", default-features = false, optional = true }
serde = { version = "1.0.195", default-features = false, features = [
"derive",
"alloc"
], optional = true }
6 changes: 3 additions & 3 deletions packable/packable/src/packable/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ impl<T: Packable, const N: usize> Packable for [T; N] {
}

#[inline]
fn unpack<U: Unpacker, const VERIFY: bool>(
fn unpack<U: Unpacker>(
unpacker: &mut U,
visitor: &Self::UnpackVisitor,
visitor: Option<&Self::UnpackVisitor>,
) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
if TypeId::of::<T>() == TypeId::of::<u8>() {
let mut bytes = [0u8; N];
Expand All @@ -40,7 +40,7 @@ impl<T: Packable, const N: usize> Packable for [T; N] {
let mut array = unsafe { MaybeUninit::<[MaybeUninit<T>; N]>::uninit().assume_init() };

for item in array.iter_mut() {
let unpacked = T::unpack::<_, VERIFY>(unpacker, visitor)?;
let unpacked = T::unpack(unpacker, visitor)?;

// Safety: each `item` is only visited once so we are never overwriting nor dropping values that are
// already initialized.
Expand Down
6 changes: 3 additions & 3 deletions packable/packable/src/packable/bool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ impl Packable for bool {

/// Booleans are unpacked if the byte used to represent them is non-zero.
#[inline]
fn unpack<U: Unpacker, const VERIFY: bool>(
fn unpack<U: Unpacker>(
unpacker: &mut U,
visitor: &Self::UnpackVisitor,
visitor: Option<&Self::UnpackVisitor>,
) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
Ok(u8::unpack::<_, VERIFY>(unpacker, visitor).coerce()? != 0)
Ok(u8::unpack(unpacker, visitor).coerce()? != 0)
}
}
Loading
Loading