Propagate reader I/O errors and some polishing

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Breaking
+
+- Remove `Reader::end()` in favor of `Reader::try_end()`, which propagates I/O errors
+
 ### Fixed
 
 - `Writer`: an `Lsb0` write that ends on a byte boundary no longer reorders the

Cargo.toml

@@ -18,6 +18,7 @@ bench = false
 members = [
     "deku-derive"
 ]
+exclude = ["tests/renamed_dependency"]
 
 [features]
 default = ["std", "bits", "descriptive-errors"]
@@ -30,7 +31,7 @@ descriptive-errors = ["alloc"]
 [dependencies]
 deku_derive = { version = "^0.20.3", path = "deku-derive", default-features = false}
 bitvec = { version = "1.1.0", default-features = false, optional = true }
-log = { version = "0.4.28", optional = true }
+log = { version = "0.4.28", optional = true, default-features = false }
 no_std_io = { version = "0.9.4", default-features = false, package = "no_std_io2" }
 rustversion = "1.0.22"
 

Justfile

@@ -8,8 +8,7 @@ pipeline := env_var_or_default("DEKU_TOOLCHAIN", "stable")
 toolchain := if pipeline == "msrv" { msrv } else if pipeline == "stable" { stable } else if pipeline == "beta" { beta } else { pipeline }
 trybuild_profile := if pipeline == "msrv" { "msrv" } else if pipeline == "beta" { "beta" } else { "stable" }
 
-# Non-default feature combinations exercised by the test and coverage matrix.
-feature_matrix := "std alloc descriptive-errors bits logging bits,alloc"
+# Non-default feature combinations are generated by scripts/feature-matrix.sh.
 
 default:
     @just --list
@@ -21,12 +20,14 @@ test:
     #!/usr/bin/env bash
     set -euo pipefail
     export DEKU_TRYBUILD_PROFILE="{{trybuild_profile}}"
-    cargo +{{toolchain}} test --all
-    cargo +{{toolchain}} test --all-features
-    cargo +{{toolchain}} test --no-default-features
-    for features in {{feature_matrix}}; do
-        cargo +{{toolchain}} test --no-default-features --features="${features}"
+    cargo +{{toolchain}} test --workspace
+    cargo +{{toolchain}} test --workspace --all-features
+    cargo +{{toolchain}} test --workspace --no-default-features
+    feature_matrix="$(bash scripts/feature-matrix.sh)"
+    for features in $feature_matrix; do
+        cargo +{{toolchain}} test --workspace --no-default-features --features="${features}"
     done
+    cargo +{{toolchain}} test --manifest-path tests/renamed_dependency/Cargo.toml
 
 miri-test:
     #!/usr/bin/env bash
@@ -35,7 +36,8 @@ miri-test:
     cargo +{{nightly}} miri test --workspace
     cargo +{{nightly}} miri test --workspace --all-features
     cargo +{{nightly}} miri test --workspace --no-default-features
-    for features in {{feature_matrix}}; do
+    feature_matrix="$(bash scripts/feature-matrix.sh)"
+    for features in $feature_matrix; do
         cargo +{{nightly}} miri test --workspace --no-default-features --features="${features}"
     done
 
@@ -104,7 +106,8 @@ coverage:
     cargo +{{stable}} llvm-cov --workspace --no-report
     cargo +{{stable}} llvm-cov --workspace --all-features --no-report
     cargo +{{stable}} llvm-cov --package deku --no-default-features --no-report
-    for features in {{feature_matrix}}; do
+    feature_matrix="$(bash scripts/feature-matrix.sh)"
+    for features in $feature_matrix; do
         cargo +{{stable}} llvm-cov --package deku --no-default-features --features="${features}" --no-report
     done
 

deku-derive/src/lib.rs

@@ -57,7 +57,6 @@ enum Id {
     Int(syn::LitInt),
     Bool(syn::LitBool),
 }
-
 impl Display for Id {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.write_str(&self.to_token_stream().to_string())
@@ -715,7 +714,7 @@ struct FieldData {
     /// a predicate to decide when to stop reading elements into the container
     until: Option<TokenStream>,
 
-    /// read until `reader.end()`
+    /// read until `reader.try_end()` returns `Ok(true)`
     read_all: bool,
 
     /// apply a function to the field after it's read
@@ -1006,11 +1005,21 @@ impl FieldData {
         }
 
         // Validate usage of read_all
-        #[cfg(feature = "bits")]
+        let bits_read_set = {
+            #[cfg(feature = "bits")]
+            {
+                data.bits_read.is_some()
+            }
+            #[cfg(not(feature = "bits"))]
+            {
+                false
+            }
+        };
         if data.read_all
             && (data.until.is_some()
                 || data.count.is_some()
-                || (data.bits_read.is_some() || data.bytes_read.is_some()))
+                || bits_read_set
+                || data.bytes_read.is_some())
         {
             return Err(cerror(
                 data.read_all.span(),
@@ -1306,7 +1315,7 @@ struct DekuFieldReceiver {
     #[darling(default = "default_res_opt", map = "map_litstr_as_tokenstream")]
     until: Result<Option<TokenStream>, ReplacementError>,
 
-    /// read until `reader.end()`
+    /// read until `reader.try_end()` returns `Ok(true)`
     #[darling(default)]
     read_all: bool,
 
@@ -1595,11 +1604,15 @@ pub fn deku_derive(
 
 #[cfg(test)]
 mod tests {
+    #[cfg(feature = "bits")]
     use rstest::rstest;
+    #[cfg(feature = "bits")]
     use syn::parse_str;
 
+    #[cfg(feature = "bits")]
     use super::*;
 
+    #[cfg(feature = "bits")]
     #[rstest(input,
         // Valid struct
         case::struct_empty(r#"struct Test {}"#),

deku-derive/src/macros/deku_read.rs

@@ -113,7 +113,7 @@ fn emit_struct(input: &DekuData) -> Result<TokenStream, syn::Error> {
         let from_reader_body = quote! {
             use core::convert::TryFrom;
             use ::#crate_::DekuReader as _;
-            let __deku_reader = &mut deku::reader::Reader::new(__deku_input.0);
+            let __deku_reader = &mut ::#crate_::reader::Reader::new(__deku_input.0);
             if __deku_input.1 != 0 {
                 __deku_reader.skip_bits(__deku_input.1, ::#crate_::ctx::Order::default())?;
             }
@@ -127,7 +127,7 @@ fn emit_struct(input: &DekuData) -> Result<TokenStream, syn::Error> {
             use core::convert::TryFrom;
             use ::#crate_::DekuReader as _;
             let mut __deku_cursor = #crate_::no_std_io::Cursor::new(__deku_input.0);
-            let mut __deku_reader = &mut deku::reader::Reader::new(&mut __deku_cursor);
+            let mut __deku_reader = &mut ::#crate_::reader::Reader::new(&mut __deku_cursor);
             if __deku_input.1 != 0 {
                 __deku_reader.skip_bits(__deku_input.1, ::#crate_::ctx::Order::default())?;
             }
@@ -140,7 +140,7 @@ fn emit_struct(input: &DekuData) -> Result<TokenStream, syn::Error> {
                 (__deku_reader.bits_read - (__deku_reader.bits_read % 8)) / 8
             };
             let Some(rest) = __deku_input.0.get(idx..) else {
-                return Err(deku::DekuError::Incomplete(deku::prelude::NeedSize::new(8 * (idx - __deku_input.0.len()))));
+                return Err(::#crate_::DekuError::Incomplete(::#crate_::prelude::NeedSize::new(8 * (idx - __deku_input.0.len()))));
             };
             Ok(((rest, __deku_reader.bits_read % 8), __deku_value))
         };
@@ -400,7 +400,7 @@ fn emit_enum(input: &DekuData) -> Result<TokenStream, syn::Error> {
         let from_reader_body = quote! {
             use core::convert::TryFrom;
             use ::#crate_::DekuReader as _;
-            let __deku_reader = &mut deku::reader::Reader::new(__deku_input.0);
+            let __deku_reader = &mut ::#crate_::reader::Reader::new(__deku_input.0);
             if __deku_input.1 != 0 {
                 __deku_reader.skip_bits(__deku_input.1, ::#crate_::ctx::Order::default())?;
             }
@@ -414,7 +414,7 @@ fn emit_enum(input: &DekuData) -> Result<TokenStream, syn::Error> {
             use core::convert::TryFrom;
             use ::#crate_::DekuReader as _;
             let mut __deku_cursor = #crate_::no_std_io::Cursor::new(__deku_input.0);
-            let mut __deku_reader = &mut deku::reader::Reader::new(&mut __deku_cursor);
+            let mut __deku_reader = &mut ::#crate_::reader::Reader::new(&mut __deku_cursor);
             if __deku_input.1 != 0 {
                 __deku_reader.skip_bits(__deku_input.1, ::#crate_::ctx::Order::default())?;
             }
@@ -427,7 +427,7 @@ fn emit_enum(input: &DekuData) -> Result<TokenStream, syn::Error> {
                 (__deku_reader.bits_read - (__deku_reader.bits_read % 8)) / 8
             };
             let Some(rest) = __deku_input.0.get(idx..) else {
-                return Err(deku::DekuError::Incomplete(deku::prelude::NeedSize::new(8 * (idx - __deku_input.0.len()))));
+                return Err(::#crate_::DekuError::Incomplete(::#crate_::prelude::NeedSize::new(8 * (idx - __deku_input.0.len()))));
             };
             Ok(((rest, __deku_reader.bits_read % 8), __deku_value))
         };
@@ -684,7 +684,7 @@ fn emit_bit_run_read(
         };
         let trace_field_log = if cfg!(feature = "logging") {
             let field_ident_str = field_ident.to_string();
-            quote! { log::trace!("Reading: {}.{}", #ident, #field_ident_str); }
+            quote! { ::#crate_::log::trace!("Reading: {}.{}", #ident, #field_ident_str); }
         } else {
             quote! {}
         };
@@ -894,7 +894,7 @@ fn emit_field_read(
 
     let trace_field_log = if cfg!(feature = "logging") {
         quote! {
-            log::trace!("Reading: {}.{}", #ident, #field_ident_str);
+            ::#crate_::log::trace!("Reading: {}.{}", #ident, #field_ident_str);
         }
     } else {
         quote! {}

deku-derive/src/macros/deku_write.rs

@@ -535,7 +535,9 @@ fn emit_bit_run_write(
 
         if cfg!(feature = "logging") {
             let field_ident_str = field_ident.to_string();
-            traces.extend(quote! { log::trace!("Writing: {}.{}", #ident, #field_ident_str); });
+            traces.extend(
+                quote! { ::#crate_::log::trace!("Writing: {}.{}", #ident, #field_ident_str); },
+            );
         }
 
         let value = quote! { (*(#object_prefix #field_ident) as u64) };
@@ -822,7 +824,7 @@ fn emit_field_write(
 
     let trace_field_log = if cfg!(feature = "logging") {
         quote! {
-            log::trace!("Writing: {}.{}", #ident, #field_ident_str);
+            ::#crate_::log::trace!("Writing: {}.{}", #ident, #field_ident_str);
         }
     } else {
         quote! {}
@@ -899,7 +901,7 @@ fn emit_field_write(
 
     let skipping_log = if cfg!(feature = "logging") {
         quote! {
-            log::trace!("skipping");
+            ::#crate_::log::trace!("skipping");
         }
     } else {
         quote! {}

scripts/feature-matrix.sh

@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+# The base states are the distinct std/alloc/descriptive-errors combinations
+# after accounting for std and descriptive-errors both implying alloc.
+bases=("" alloc std descriptive-errors std,descriptive-errors)
+modifiers=("" bits logging bits,logging)
+
+for base in "${bases[@]}"; do
+    for modifier in "${modifiers[@]}"; do
+        # `just test` already runs no-default-features, default features, and
+        # all-features, so omit those three configurations from this matrix.
+        if [[ -z "$base" && -z "$modifier" ]]; then
+            continue
+        fi
+        if [[ "$base" == "std,descriptive-errors" && ( "$modifier" == "bits" || "$modifier" == "bits,logging" ) ]]; then
+            continue
+        fi
+
+        if [[ -n "$base" && -n "$modifier" ]]; then
+            printf '%s,%s\n' "$base" "$modifier"
+        elif [[ -n "$base" ]]; then
+            printf '%s\n' "$base"
+        else
+            printf '%s\n' "$modifier"
+        fi
+    done
+done

src/attributes.rs

@@ -51,7 +51,7 @@ enum DekuEnum {
 | [bits_read](#bits_read) | field | Set the field representing the number of bits to read into a container
 | [bytes_read](#bytes_read) | field | Set the field representing the number of bytes to read into a container
 | [until](#until) | field | Set a predicate returning when to stop reading elements into a container
-| [read_all](#read_all) | field | Read until [reader.end()] returns `true`
+| [read_all](#read_all) | field | Read until [reader.try_end()] returns `Ok(true)`
 | [update](#update) | field | Apply code over the field when `.update()` is called
 | [temp](#temp) | field | Read the field but exclude it from the struct/enum
 | [temp_value](#temp_value) | field | Write the field but exclude it from the struct/enum
@@ -1149,7 +1149,9 @@ assert_eq!(
 ```
 # read_all
 
-Read values into the container until [reader.end()] returns `true`.
+Read values into the container until [reader.try_end()] returns `Ok(true)`. I/O errors from the
+end probe propagate. EOF before the next element ends the collection successfully. Errors while
+decoding an element, including EOF after only part of an element, propagate.
 
 Example:
 ```rust
@@ -2289,5 +2291,5 @@ assert_eq!(data, value);
 # fn main() {}
 ```
 
-[reader.end()]: crate::reader::Reader::end()
+[reader.try_end()]: crate::reader::Reader::try_end()
 */

src/ctx.rs

@@ -106,7 +106,8 @@ pub enum Limit<T, Predicate: FnMut(&T) -> bool> {
     /// Read until a given quantity of bits have been read
     BitSize(BitSize),
 
-    /// Read until `reader.end()` is true. Used for `read_all` attribute.
+    /// Read until [`crate::reader::Reader::try_end`] returns `Ok(true)`. Errors from the end probe
+    /// propagate. Used for the `read_all` attribute.
     End,
 }
 
@@ -149,7 +150,8 @@ impl<T, Predicate: for<'a> FnMut(&'a T) -> bool> Limit<T, Predicate> {
 }
 
 impl<T> Limit<T, fn(&T) -> bool> {
-    /// Read until `reader.end()` is true
+    /// Construct a limit that reads until [`crate::reader::Reader::try_end`] returns `Ok(true)`.
+    /// Errors from the end probe propagate.
     #[inline]
     pub fn end() -> Self {
         Self::End

src/error.rs

@@ -144,7 +144,6 @@ impl From<core::convert::Infallible> for DekuError {
     }
 }
 
-#[cfg(feature = "std")]
 impl core::fmt::Display for DekuError {
     fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
         match self {
@@ -158,21 +157,14 @@ impl core::fmt::Display for DekuError {
             DekuError::InvalidParam(err) => write!(f, "Invalid param error: {err}"),
             DekuError::Assertion(err) => write!(f, "{err}"),
             DekuError::IdVariantNotFound => write!(f, "Could not resolve `id` for variant"),
-            DekuError::Io(e) => write!(f, "io errorr: {e:?}"),
+            DekuError::Io(e) => write!(f, "io error: {e:?}"),
         }
     }
 }
 
 #[cfg(not(feature = "std"))]
 impl core::error::Error for DekuError {}
 
-#[cfg(not(feature = "std"))]
-impl core::fmt::Display for DekuError {
-    fn fmt(&self, _f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        Ok(())
-    }
-}
-
 #[cfg(not(feature = "std"))]
 impl From<DekuError> for no_std_io::io::Error {
     fn from(error: DekuError) -> Self {
@@ -219,3 +211,25 @@ impl From<no_std_io::io::Error> for DekuError {
         DekuError::Io(value.kind())
     }
 }
+
+#[cfg(all(test, feature = "alloc"))]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn display_includes_parse_error_details() {
+        #[cfg(feature = "descriptive-errors")]
+        let error = DekuError::Parse(alloc::borrow::Cow::Borrowed("details"));
+        #[cfg(not(feature = "descriptive-errors"))]
+        let error = DekuError::Parse("details");
+
+        assert_eq!(alloc::format!("{error}"), "Parse error: details");
+    }
+
+    #[test]
+    fn display_formats_io_errors_consistently() {
+        let error = DekuError::Io(ErrorKind::Other);
+
+        assert_eq!(alloc::format!("{error}"), "io error: Other");
+    }
+}

src/impls/hashmap.rs

@@ -56,7 +56,7 @@ where
     let mut res = HashMap::with_capacity_and_hasher(capacity.unwrap_or(0), S::default());
 
     loop {
-        if reader.end() {
+        if reader.try_end()? {
             break;
         }
         let val = <(K, V)>::from_reader_with_ctx(reader, ctx)?;
@@ -170,7 +170,7 @@ where
                 )
             }
 
-            // Read until `reader.end()` is true
+            // Read until `reader.try_end()` returns `Ok(true)`
             Limit::End => from_reader_with_ctx_hashmap_to_end(reader, None, inner_ctx),
         }
     }