Skip to content

Commit c411498

Browse files
committed
features: Switch no-assert-string feature to descriptive-errors
Replace DekuError::AssertionNoStr with DekuError::Assertion, in concert with the deku_error!() macro. deku_error!() discards dynamic formatting when the capability unavailable, usually due to feature selection. Discarding the unique portion of error strings provides the compiler the opportunity to coalesce string constants, reducing space required for .rodata in the spirit of AssertionNoStr. Introducing descriptive-errors and enabling it by default gives us a couple of things: - Maintains the current behaviour where `--features no-assert-string` _isn't_ specified, - What I feel is desirable behaviour if we pass --all-features to `cargo build`, where the no-assert-string feature didn't, and - The ability to control the error message behaviour (though alloc is required for descriptive messages) Demonstrating the impact on wcampbell0x2a/backhand, with descriptive-errors enabled[^1] we have: $ cargo build --release --features="deku/descriptive-errors" Finished `release` profile [optimized] target(s) in 0.06s $ bloaty -d sections target/release/unsquashfs-backhand | grep '[.]rodata$' 8.5% 530Ki 10.7% 530Ki .rodata $ readelf --string-dump .rodata target/release/unsquashfs-backhand | grep --color=never --only-matching -E "Field failed assertion.{5}" Field failed assertion: Dir Field failed assertion: Sup Field failed assertion: Sup Field failed assertion: Sup Field failed assertion: Ext Field failed assertion: Bas Field failed assertion: Dir $ And without: $ cargo build --release Finished `release` profile [optimized] target(s) in 0.05s $ bloaty -d sections target/release/unsquashfs-backhand | grep '[.]rodata$' 8.5% 529Ki 10.7% 529Ki .rodata $ readelf --string-dump .rodata target/release/unsquashfs-backhand | grep --color=never --only-matching -E "Field failed assertion.{5}" Field failed assertionevent Field failed assertionCould $ [^1]: For the purpose of clarity in the demonstration I temporarily changed the patch such that descriptive-errors _isn't_ in the default features Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
1 parent 5dca0c7 commit c411498

9 files changed

Lines changed: 44 additions & 40 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
## Added
66

77
- The `alloc` feature, allowing use in environments lacking a heap [#582](https://github.com/sharksforarms/deku/pull/582)
8+
- The `descriptive-errors` feature, replacing `no-assertion-string` [#582](https://github.com/sharksforarms/deku/pull/582)
89

910
## [0.19.1] - 2025-05-22
1011

Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ members = [
2020
]
2121

2222
[features]
23-
default = ["std", "bits"]
23+
default = ["std", "bits", "descriptive-errors"]
2424
std = ["deku_derive/std", "bitvec?/std", "alloc", "no_std_io/std"]
2525
alloc = ["bitvec?/alloc", "deku_derive/alloc", "no_std_io/alloc" ]
2626
logging = ["deku_derive/logging", "log"]
27-
no-assert-string = ["deku_derive/no-assert-string"]
2827
bits = ["dep:bitvec", "deku_derive/bits", "alloc" ]
28+
descriptive-errors = ["alloc"]
2929

3030
[dependencies]
3131
deku_derive = { version = "^0.19.1", path = "deku-derive", default-features = false}

deku-derive/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ proc-macro = true
1616
std = ["proc-macro-crate", "alloc"]
1717
alloc = []
1818
logging = []
19-
no-assert-string = []
2019
bits = []
2120

2221
[dependencies]

deku-derive/src/macros/mod.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -417,13 +417,6 @@ fn assertion_failed(
417417
} else {
418418
quote! { stringify!(#v) }
419419
};
420-
#[cfg(feature = "no-assert-string")]
421-
{
422-
quote! {
423-
return Err(::#crate_::DekuError::AssertionNoStr);
424-
}
425-
}
426-
#[cfg(not(feature = "no-assert-string"))]
427420
{
428421
quote! {
429422
return Err(::#crate_::deku_error!(::#crate_::DekuError::Assertion, "Field failed assertion", "{}.{}: {}", #ident, #field_ident_str, #stringify));

src/attributes.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -672,12 +672,12 @@ let data: &[u8] = &[0x00, 0x01, 0x02];
672672
673673
let value = DekuTest::try_from(data);
674674
675-
#[cfg(feature = "alloc")]
675+
#[cfg(feature = "descriptive-errors")]
676676
assert_eq!(
677677
Err(DekuError::Assertion("Field failed assertion: DekuTest.data: * data >= 8".into())),
678678
value
679679
);
680-
#[cfg(not(feature = "alloc"))]
680+
#[cfg(not(feature = "descriptive-errors"))]
681681
assert_eq!(
682682
Err(DekuError::Assertion("Field failed assertion".into())),
683683
value
@@ -717,10 +717,16 @@ value.data = 0x02;
717717
718718
let value: Result<Vec<u8>, DekuError> = value.try_into();
719719
720+
# #[cfg(feature = "descriptive-errors")]
720721
assert_eq!(
721722
Err(DekuError::Assertion("Field failed assertion: DekuTest.data: data == 0x01".into())),
722723
value
723724
);
725+
# #[cfg(not(feature = "descriptive-errors"))]
726+
# assert_eq!(
727+
# Err(DekuError::Assertion("Field failed assertion".into())),
728+
# value
729+
# );
724730
# }
725731
#
726732
# #[cfg(not(feature = "alloc"))]

src/error.rs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Error module
22
3-
#[cfg(feature = "alloc")]
3+
#[cfg(feature = "descriptive-errors")]
44
use alloc::borrow::Cow;
55

66
use no_std_io::io::ErrorKind;
@@ -31,10 +31,10 @@ impl NeedSize {
3131
}
3232
}
3333

34-
#[cfg(feature = "alloc")]
34+
#[cfg(feature = "descriptive-errors")]
3535
type DekuErrorString = Cow<'static, str>;
3636

37-
#[cfg(not(feature = "alloc"))]
37+
#[cfg(not(feature = "descriptive-errors"))]
3838
type DekuErrorString = &'static str;
3939

4040
/// Deku errors
@@ -49,8 +49,6 @@ pub enum DekuError {
4949
InvalidParam(DekuErrorString),
5050
/// Assertion error from `assert` or `assert_eq` attributes
5151
Assertion(DekuErrorString),
52-
/// Assertion error from `assert` or `assert_eq` attributes, without string
53-
AssertionNoStr,
5452
/// Could not resolve `id` for variant
5553
IdVariantNotFound,
5654
/// IO error while reading or writing
@@ -76,7 +74,7 @@ pub enum DekuError {
7674
/// bit_size,
7775
/// input_size);
7876
/// ```
79-
#[cfg(feature = "alloc")]
77+
#[cfg(feature = "descriptive-errors")]
8078
#[macro_export]
8179
macro_rules! deku_error {
8280
($p:path, $desc:expr, $fmt:expr, $($arg:expr),*) => {{
@@ -112,7 +110,7 @@ macro_rules! deku_error {
112110
/// bit_size,
113111
/// input_size);
114112
/// ```
115-
#[cfg(not(feature = "alloc"))]
113+
#[cfg(not(feature = "descriptive-errors"))]
116114
#[macro_export]
117115
macro_rules! deku_error {
118116
($p:path, $desc:expr, $fmt:expr, $($arg:expr),*) => {{
@@ -158,8 +156,7 @@ impl core::fmt::Display for DekuError {
158156
),
159157
DekuError::Parse(ref err) => write!(f, "Parse error: {err}"),
160158
DekuError::InvalidParam(ref err) => write!(f, "Invalid param error: {err}"),
161-
DekuError::Assertion(ref err) => write!(f, "Assertion error: {err}"),
162-
DekuError::AssertionNoStr => write!(f, "Assertion error"),
159+
DekuError::Assertion(ref err) => write!(f, "{err}"),
163160
DekuError::IdVariantNotFound => write!(f, "Could not resolve `id` for variant"),
164161
DekuError::Io(ref e) => write!(f, "io errorr: {e:?}"),
165162
}
@@ -211,7 +208,6 @@ impl From<DekuError> for std::io::Error {
211208
DekuError::Parse(_) => io::Error::new(io::ErrorKind::InvalidData, error),
212209
DekuError::InvalidParam(_) => io::Error::new(io::ErrorKind::InvalidInput, error),
213210
DekuError::Assertion(_) => io::Error::new(io::ErrorKind::InvalidData, error),
214-
DekuError::AssertionNoStr => io::Error::from(io::ErrorKind::InvalidData),
215211
DekuError::IdVariantNotFound => io::Error::new(io::ErrorKind::NotFound, error),
216212
DekuError::Io(e) => io::Error::new(e, error),
217213
}

src/impls/cstring.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
use alloc::borrow::Cow;
21
use alloc::ffi::CString;
3-
use alloc::format;
2+
43
use alloc::vec::Vec;
54
use no_std_io::io::{Read, Seek, Write};
65

@@ -32,7 +31,12 @@ impl DekuReader<'_> for CString {
3231
Vec::<u8>::from_reader_with_ctx(reader, (Limit::from(|b: &u8| *b == 0x00), ()))?;
3332

3433
let value = CString::from_vec_with_nul(bytes).map_err(|e| {
35-
DekuError::Parse(Cow::from(format!("Failed to convert Vec to CString: {e}")))
34+
crate::deku_error!(
35+
DekuError::Parse,
36+
"Failed to convert Vec to CString",
37+
"{}",
38+
e
39+
)
3640
})?;
3741

3842
Ok(value)
@@ -50,7 +54,12 @@ where
5054
let bytes = Vec::from_reader_with_ctx(reader, (Limit::from(byte_size.0), ()))?;
5155

5256
let value = CString::from_vec_with_nul(bytes).map_err(|e| {
53-
DekuError::Parse(Cow::from(format!("Failed to convert Vec to CString: {e}")))
57+
crate::deku_error!(
58+
DekuError::Parse,
59+
"Failed to convert Vec to CString",
60+
"{}",
61+
e
62+
)
5463
})?;
5564

5665
Ok(value)
@@ -95,7 +104,7 @@ mod tests {
95104
b"a",
96105
),
97106
98-
#[should_panic(expected = "Parse(\"Failed to convert Vec to CString: data provided is not nul terminated\")")]
107+
#[should_panic(expected = "Failed to convert Vec to CString")]
99108
case(
100109
b"test",
101110
Some(4),

tests/test_bit_container_size.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,19 @@ struct Test {
1818

1919
#[rstest(input,
2020
#[should_panic(
21-
expected = "InvalidParam(\"bit size of input is larger than bit requested size: 5 exceeds 4\")"
21+
expected = "bit size of input is larger than bit requested size"
2222
)]
2323
case::field_u8_be( Test { field_u8_be: 0b11111, ..Test::default()}),
2424
#[should_panic(
25-
expected = "InvalidParam(\"bit size of input is larger than bit requested size: 5 exceeds 4\")"
25+
expected = "bit size of input is larger than bit requested size"
2626
)]
2727
case::field_be( Test { field_be: 0b11111, ..Test::default()}),
2828
#[should_panic(
29-
expected = "InvalidParam(\"bit size of input is larger than requested size: 13 exceeds 12\")"
29+
expected = "bit size of input is larger than requested size"
3030
)]
3131
case::field_le( Test { field_le: 0b1111111111111, ..Test::default()}),
3232
#[should_panic(
33-
expected = "InvalidParam(\"bit size of input is larger than bit requested size: 13 exceeds 9\")"
33+
expected = "bit size of input is larger than bit requested size"
3434
)]
3535
case::field_u32_be( Test { field_u32_be: 0b1111111111111, ..Test::default()}),
3636
)]

tests/test_magic.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,16 @@ use rstest::rstest;
1313
#[rstest(input,
1414
case(&hex!("64656b75")),
1515
16-
#[should_panic(expected = "Parse(\"Missing magic value: [100, 101, 107, 117]\")")]
16+
#[should_panic(expected = "Missing magic value")]
1717
case(&hex!("64656bde")),
1818
19-
#[should_panic(expected = "Parse(\"Missing magic value: [100, 101, 107, 117]\")")]
19+
#[should_panic(expected = "Missing magic value")]
2020
case(&hex!("6465ad75")),
2121
22-
#[should_panic(expected = "Parse(\"Missing magic value: [100, 101, 107, 117]\")")]
22+
#[should_panic(expected = "Missing magic value")]
2323
case(&hex!("64be6b75")),
2424
25-
#[should_panic(expected = "Parse(\"Missing magic value: [100, 101, 107, 117]\")")]
25+
#[should_panic(expected = "Missing magic value")]
2626
case(&hex!("ef656b75")),
2727
2828
#[should_panic(expected = "Incomplete(NeedSize { bits: 8 })")]
@@ -44,19 +44,19 @@ fn test_magic_struct(input: &[u8]) {
4444
#[rstest(input,
4545
case(&hex!("64656b7500")),
4646
47-
#[should_panic(expected = "Parse(\"Missing magic value: [100, 101, 107, 117]\")")]
47+
#[should_panic(expected = "Missing magic value")]
4848
case(&hex!("64656bde00")),
4949
50-
#[should_panic(expected = "Parse(\"Missing magic value: [100, 101, 107, 117]\")")]
50+
#[should_panic(expected = "Missing magic value")]
5151
case(&hex!("6465ad7500")),
5252
53-
#[should_panic(expected = "Parse(\"Missing magic value: [100, 101, 107, 117]\")")]
53+
#[should_panic(expected = "Missing magic value")]
5454
case(&hex!("64be6b7500")),
5555
56-
#[should_panic(expected = "Parse(\"Missing magic value: [100, 101, 107, 117]\")")]
56+
#[should_panic(expected = "Missing magic value")]
5757
case(&hex!("ef656b7500")),
5858
59-
#[should_panic(expected = "Parse(\"Missing magic value: [100, 101, 107, 117]\")")]
59+
#[should_panic(expected = "Missing magic value")]
6060
case(&hex!("64656b00")),
6161
6262
#[should_panic(expected = "Incomplete(NeedSize { bits: 8 })")]

0 commit comments

Comments
 (0)