Repository: knurling-rs/defmt (rust)

Issue:
how to implement Format for [T; N]? 
It would be quite handy if we could print arrays of any Format type. I had a go at implementing it, but it seems there is already a conflicting implementation for [u8; N] (overlaps because u8 is also Format).

I think this is not possible without specialization. What do we do?

- Not implement it?
- Remove the [u8] impl?
- use `min_specialization` behind a feature, so you can at least have both in nightly?

Pull request description:
implement Format arrays, like {:[?;16]}
this adds support for printing arrays of `Format`s.

The Format impl is changed to work on `[impl Format; N]` for all types. The `[u8; N]` impl is removed because it would overlap, as discussed in #206.

I've renamed "slice" and "array" to "u8_slice" and "u8_array" in a few places, so the names are clearer now that we have multiple kinds of arrays/slices.

Fixes #206 

Patch:
diff --git a/decoder/src/lib.rs b/decoder/src/lib.rs
index 4cc83bd..8cab063 100644
--- a/decoder/src/lib.rs
+++ b/decoder/src/lib.rs
@@ -512,6 +512,89 @@ impl<'t, 'b> Decoder<'t, 'b> {
             .ok_or(DecodeError::Malformed)
     }
 
+    fn decode_format_slice(
+        &mut self,
+        num_elements: usize,
+    ) -> Result<Vec<FormatSliceElement<'t>>, DecodeError> {
+        if num_elements == 0 {
+            return Ok(vec![]);
+        }
+
+        let format = self.get_format()?;
+
+        // let variant_format = if
+        let is_enum = format.contains('|');
+        let below_enum = self.below_enum;
+
+        if is_enum {
+            self.below_enum = true;
+        }
+
+        let mut elements = Vec::with_capacity(num_elements);
+        let mut formats = vec![];
+        let mut cursor = 0;
+        for i in 0..num_elements {
+            let is_first = i == 0;
+
+            let format = if is_enum {
+                self.get_variant(format)?
+            } else {
+                format
+            };
+
+            let args = if let Some(list) = &mut self.format_list {
+                match list {
+                    FormatList::Use { .. } => self.decode_format(format)?,
+
+                    FormatList::Build { formats } => {
+                        if is_first {
+                            cursor = formats.len();
+                            self.decode_format(format)?
+                        } else {
+                            let formats = formats.clone();
+                            let old = mem::replace(
+                                &mut self.format_list,
+                                Some(FormatList::Use { formats, cursor }),
+                            );
+                            let args = self.decode_format(format)?;
+                            self.format_list = old;
+                            args
+                        }
+                    }
+                }
+            } else {
+                if is_first {
+                    let mut old =
+                        mem::replace(&mut self.format_list, Some(FormatList::Build { formats }));
+                    let args = self.decode_format(format)?;
+                    mem::swap(&mut self.format_list, &mut old);
+                    formats = match old {
+                        Some(FormatList::Build { formats, .. }) => formats,
+                        _ => unreachable!(),
+                    };
+                    args
+                } else {
+                    let formats = formats.clone();
+                    let old = mem::replace(
+                        &mut self.format_list,
+                        Some(FormatList::Use { formats, cursor: 0 }),
+                    );
+                    let args = self.decode_format(format)?;
+                    self.format_list = old;
+                    args
+                }
+            };
+
+            elements.push(FormatSliceElement { format, args });
+        }
+
+        if is_enum {
+            self.below_enum = below_enum;
+        }
+
+        Ok(elements)
+    }
+
     /// Decodes arguments from the stream, according to `format`.
     fn decode_format(&mut self, format: &str) -> Result<Vec<Arg<'t>>, DecodeError> {
         let mut args = vec![]; // will contain the deserialized arguments on return
@@ -545,88 +628,8 @@ impl<'t, 'b> Decoder<'t, 'b> {
 
                 Type::FormatSlice => {
                     let num_elements = read_leb128(&mut self.bytes)? as usize;
-
-                    let arg = if num_elements == 0 {
-                        Arg::FormatSlice { elements: vec![] }
-                    } else {
-                        let format = self.get_format()?;
-
-                        // let variant_format = if
-                        let is_enum = format.contains('|');
-                        let below_enum = self.below_enum;
-
-                        if is_enum {
-                            self.below_enum = true;
-                        }
-
-                        let mut elements = Vec::with_capacity(num_elements);
-                        let mut formats = vec![];
-                        let mut cursor = 0;
-                        for i in 0..num_elements {
-                            let is_first = i == 0;
-
-                            let format = if is_enum {
-                                self.get_variant(format)?
-                            } else {
-                                format
-                            };
-
-                            let args = if let Some(list) = &mut self.format_list {
-                                match list {
-                                    FormatList::Use { .. } => self.decode_format(format)?,
-
-                                    FormatList::Build { formats } => {
-                                        if is_first {
-                                            cursor = formats.len();
-                                            self.decode_format(format)?
-                                        } else {
-                                            let formats = formats.clone();
-                                            let old = mem::replace(
-                                                &mut self.format_list,
-                                                Some(FormatList::Use { formats, cursor }),
-                                            );
-                                            let args = self.decode_format(format)?;
-                                            self.format_list = old;
-                                            args
-                                        }
-                                    }
-                                }
-                            } else {
-                                if is_first {
-                                    let mut old = mem::replace(
-                                        &mut self.format_list,
-                                        Some(FormatList::Build { formats }),
-                                    );
-                                    let args = self.decode_format(format)?;
-                                    mem::swap(&mut self.format_list, &mut old);
-                                    formats = match old {
-                                        Some(FormatList::Build { formats, .. }) => formats,
-                                        _ => unreachable!(),
-                                    };
-                                    args
-                                } else {
-                                    let formats = formats.clone();
-                                    let old = mem::replace(
-                                        &mut self.format_list,
-                                        Some(FormatList::Use { formats, cursor: 0 }),
-                                    );
-                                    let args = self.decode_format(format)?;
-                                    self.format_list = old;
-                                    args
-                                }
-                            };
-
-                            elements.push(FormatSliceElement { format, args });
-                        }
-
-                        if is_enum {
-                            self.below_enum = below_enum;
-                        }
-
-                        Arg::FormatSlice { elements }
-                    };
-
-                    args.push(arg);
+                    let elements = self.decode_format_slice(num_elements)?;
+                    args.push(Arg::FormatSlice { elements });
                 }
                 Type::Format => {
                     let format = self.get_format()?;
@@ -750,7 +753,7 @@ impl<'t, 'b> Decoder<'t, 'b> {
 
                     args.push(Arg::IStr(string));
                 }
-                Type::Slice => {
+                Type::U8Slice => {
                     // only supports byte slices
                     let num_elements = read_leb128(&mut self.bytes)? as usize;
                     let mut arg_slice = vec![];
@@ -761,7 +764,7 @@ impl<'t, 'b> Decoder<'t, 'b> {
                     }
                     args.push(Arg::Slice(arg_slice.to_vec()));
                 }
-                Type::Array(len) => {
+                Type::U8Array(len) => {
                     let mut arg_slice = vec![];
                     // note: went for the suboptimal but simple solution; optimize if necessary
                     for _ in 0..*len {
@@ -769,6 +772,10 @@ impl<'t, 'b> Decoder<'t, 'b> {
                     }
                     args.push(Arg::Slice(arg_slice.to_vec()));
                 }
+                Type::FormatArray(len) => {
+                    let elements = self.decode_format_slice(*len)?;
+                    args.push(Arg::FormatSlice { elements });
+                }
             }
         }
 
diff --git a/firmware/qemu/src/bin/log.rs b/firmware/qemu/src/bin/log.rs
index ae3802c..f151cc4 100644
--- a/firmware/qemu/src/bin/log.rs
+++ b/firmware/qemu/src/bin/log.rs
@@ -346,6 +346,22 @@ fn main() -> ! {
     defmt::info!("tuple of slices: {:?}", (&[1,2,3][..], &[4,5,6][..]));
     defmt::info!("tuple of [u8;4]: {:?}", ([1u8, 2, 3, 4], [5u8, 6, 7, 8]));
 
+    // Arrays of T: Format
+    defmt::info!("[u8;0]: {:[?;0]}", [0u8; 0]);
+    defmt::info!("[u8;4]: {:[?;4]}", [1u8, 2, 3, 4]);
+    defmt::info!("[i8;4]: {:[?;4]}", [-1i8, 2, 3, -4]);
+    defmt::info!("[(u32,u32);4]: {:[?;4]}", [(1u32, 2u32), (3, 4), (5, 6), (7, 8)]);
+
+    defmt::info!("[u8;0]: {:?}", [0u8; 0]);
+    defmt::info!("[u8;4]: {:?}", [1u8, 2, 3, 4]);
+    defmt::info!("[i8;4]: {:?}", [-1i8, 2, 3, -4]);
+    defmt::info!("[u32;4]: {:?}", [1u32, 2, 3, 4]);
+    defmt::info!("[i32;4]: {:?}", [-1i32, 2, 3, -4]);
+    defmt::info!("[[u32;4];4]: {:?}", [[1u32, 2, 3, 4], [2, 3, 4, 5], [3, 4, 5, 6], [4, 5, 6, 7]]);
+    defmt::info!("[Option<u32>;4]: {:?}", [Some(1u32), None, Some(3), None]);
+    defmt::info!("[(u32,u32);4]: {:?}", [(1u32, 2u32), (3, 4), (5, 6), (7, 8)]);
+
+
     #[cfg(feature = "alloc")]
     {
         use alloc::boxed::Box;
diff --git a/macros/src/lib.rs b/macros/src/lib.rs
index 282ca4c..9777bd3 100644
--- a/macros/src/lib.rs
+++ b/macros/src/lib.rs
@@ -752,10 +752,10 @@ impl Codegen {
                 defmt_parser::Type::Bool => {
                     exprs.push(quote!(_fmt_.bool(#arg)));
                 }
-                defmt_parser::Type::Slice => {
+                defmt_parser::Type::U8Slice => {
                     exprs.push(quote!(_fmt_.slice(#arg)));
                 }
-                defmt_parser::Type::Array(len) => {
+                defmt_parser::Type::U8Array(len) => {
                     // We cast to the expected array type (which should be a no-op cast) to provoke
                     // a type mismatch error on mismatched lengths:
                     // ```
@@ -768,11 +768,17 @@ impl Codegen {
                     //    |     expected an array with a fixed size of 3 elements, found one with 2 elements
                     //    |     expected due to this
                     // ```
-                    exprs.push(quote!(_fmt_.array({
+                    exprs.push(quote!(_fmt_.u8_array({
                         let tmp: &[u8; #len] = #arg;
                         tmp
                     })));
                 }
+                defmt_parser::Type::FormatArray(len) => {
+                    exprs.push(quote!(_fmt_.fmt_array({
+                        let tmp: &[_; #len] = #arg;
+                        tmp
+                    })));
+                }
                 defmt_parser::Type::F32 => {
                     exprs.push(quote!(_fmt_.f32(#arg)));
                 }
diff --git a/parser/src/lib.rs b/parser/src/lib.rs
index 7c478a8..4500634 100644
--- a/parser/src/lib.rs
+++ b/parser/src/lib.rs
@@ -44,8 +44,9 @@ pub enum Level {
 pub enum Type {
     BitField(Range<u8>),
     Bool,
-    Format,      // "{:?}"
-    FormatSlice, // "{:[?]}"
+    Format,             // "{:?}"
+    FormatSlice,        // "{:[?]}"
+    FormatArray(usize), // FIXME: This `usize` is not the target's `usize`; use `u64` instead?
     I8,
     I16,
     I32,
@@ -62,8 +63,8 @@ pub enum Type {
     U64,
     Usize,
     /// Byte slice `{:[u8]}`.
-    Slice,
-    Array(usize), // FIXME: This `usize` is not the target's `usize`; use `u64` instead?
+    U8Slice,
+    U8Array(usize), // FIXME: This `usize` is not the target's `usize`; use `u64` instead?
     F32,
 }
 
@@ -103,6 +104,24 @@ fn parse_range(mut s: &str) -> Option<(Range<u8>, usize /* consumed */)> {
     Some((start..end, start_digits + end_digits + 2))
 }
 
+fn parse_array(mut s: &str) -> Result<usize, Cow<'static, str>> {
+    // Skip spaces.
+    let len_pos = s
+        .find(|c: char| c != ' ')
+        .ok_or("invalid array specifier (missing length)")?;
+    s = &s[len_pos..];
+    let after_len = s
+        .find(|c: char| !c.is_digit(10))
+        .ok_or("invalid array specifier (missing `]`)")?;
+    let len = s[..after_len].parse::<usize>().map_err(|e| e.to_string())?;
+    s = &s[after_len..];
+    if s != "]" {
+        return Err("invalid array specifier (missing `]`)".into());
+    }
+
+    Ok(len)
+}
+
 fn parse_param(mut s: &str) -> Result<Param, Cow<'static, str>> {
     // First, optional argument index.
     // Then, mandatory `:`.
@@ -122,7 +141,8 @@ fn parse_param(mut s: &str) -> Result<Param, Cow<'static, str>> {
     // Then, type specifier.
     s = &s[colon_pos + 1..];
 
-    static ARRAY_START: &str = "[u8;";
+    static FORMAT_ARRAY_START: &str = "[?;";
+    static U8_ARRAY_START: &str = "[u8;";
     let ty = match s {
         "u8" => Type::U8,
         "u16" => Type::U16,
@@ -139,27 +159,18 @@ fn parse_param(mut s: &str) -> Result<Param, Cow<'static, str>> {
         "bool" => Type::Bool,
         "str" => Type::Str,
         "istr" => Type::IStr,
-        "[u8]" => Type::Slice,
+        "[u8]" => Type::U8Slice,
         "?" => Type::Format,
         "[?]" => Type::FormatSlice,
-        _ if s.starts_with(ARRAY_START) => {
-            s = &s[ARRAY_START.len()..];
-
-            // Skip spaces.
-            let len_pos = s
-                .find(|c: char| c != ' ')
-                .ok_or("invalid array specifier (missing length)")?;
-            s = &s[len_pos..];
-            let after_len = s
-                .find(|c: char| !c.is_digit(10))
-                .ok_or("invalid array specifier (missing `]`)")?;
-            let len = s[..after_len].parse::<usize>().map_err(|e| e.to_string())?;
-            s = &s[after_len..];
-            if s != "]" {
-                return Err("invalid array specifier (missing `]`)".into());
-            }
-
-            Type::Array(len)
+        _ if s.starts_with(U8_ARRAY_START) => {
+            s = &s[U8_ARRAY_START.len()..];
+            let len = parse_array(s)?;
+            Type::U8Array(len)
+        }
+        _ if s.starts_with(FORMAT_ARRAY_START) => {
+            s = &s[FORMAT_ARRAY_START.len()..];
+            let len = parse_array(s)?;
+            Type::FormatArray(len)
         }
         _ => {
             // Check for bitfield syntax.
@@ -460,7 +471,7 @@ mod tests {
             parse("{:[u8]}"),
             Ok(vec![Fragment::Parameter(Parameter {
                 index: 0,
-                ty: Type::Slice,
+                ty: Type::U8Slice,
             })])
         );
 
@@ -609,7 +620,7 @@ mod tests {
             parse("{:[u8; 0]}"),
             Ok(vec![Fragment::Parameter(Parameter {
                 index: 0,
-                ty: Type::Array(0),
+                ty: Type::U8Array(0),
             })])
         );
 
@@ -618,7 +629,7 @@ mod tests {
             parse("{:[u8;42]}"),
             Ok(vec![Fragment::Parameter(Parameter {
                 index: 0,
-                ty: Type::Array(42),
+                ty: Type::U8Array(42),
             })])
         );
 
@@ -627,7 +638,7 @@ mod tests {
             parse("{:[u8;    257]}"),
             Ok(vec![Fragment::Parameter(Parameter {
                 index: 0,
-                ty: Type::Array(257),
+                ty: Type::U8Array(257),
             })])
         );
 
diff --git a/src/impls.rs b/src/impls.rs
index 064bc1c..b30cd5a 100644
--- a/src/impls.rs
+++ b/src/impls.rs
@@ -173,52 +173,88 @@ impl Format for bool {
 
 macro_rules! arrays {
     ( $($len:literal $fmt:literal,)+ ) => { $(
-        impl Format for [u8; $len] {
+        impl<T> Format for [T; $len]
+        where
+            T: Format
+        {
             fn format(&self, fmt: &mut Formatter) {
                 if fmt.needs_tag() {
                     let t = internp!($fmt);
                     fmt.u8(&t);
                 }
-                fmt.array(self);
+                fmt.fmt_array(self);
             }
         }
     )+ };
 }
 
-arrays! {
-    0 "{:[u8; 0]}",
-    1 "{:[u8; 1]}",
-    2 "{:[u8; 2]}",
-    3 "{:[u8; 3]}",
-    4 "{:[u8; 4]}",
-    5 "{:[u8; 5]}",
-    6 "{:[u8; 6]}",
-    7 "{:[u8; 7]}",
-    8 "{:[u8; 8]}",
-    9 "{:[u8; 9]}",
-    10 "{:[u8; 10]}",
-    11 "{:[u8; 11]}",
-    12 "{:[u8; 12]}",
-    13 "{:[u8; 13]}",
-    14 "{:[u8; 14]}",
-    15 "{:[u8; 15]}",
-    16 "{:[u8; 16]}",
-    17 "{:[u8; 17]}",
-    18 "{:[u8; 18]}",
-    19 "{:[u8; 19]}",
-    20 "{:[u8; 20]}",
-    21 "{:[u8; 21]}",
-    22 "{:[u8; 22]}",
-    23 "{:[u8; 23]}",
-    24 "{:[u8; 24]}",
-    25 "{:[u8; 25]}",
-    26 "{:[u8; 26]}",
-    27 "{:[u8; 27]}",
-    28 "{:[u8; 28]}",
-    29 "{:[u8; 29]}",
-    30 "{:[u8; 30]}",
-    31 "{:[u8; 31]}",
-    32 "{:[u8; 32]}",
+arrays!{
+    0 "{:[?;0]}",
+    1 "{:[?;1]}",
+    2 "{:[?;2]}",
+    3 "{:[?;3]}",
+    4 "{:[?;4]}",
+    5 "{:[?;5]}",
+    6 "{:[?;6]}",
+    7 "{:[?;7]}",
+    8 "{:[?;8]}",
+    9 "{:[?;9]}",
+    10 "{:[?;10]}",
+    11 "{:[?;11]}",
+    12 "{:[?;12]}",
+    13 "{:[?;13]}",
+    14 "{:[?;14]}",
+    15 "{:[?;15]}",
+    16 "{:[?;16]}",
+    17 "{:[?;17]}",
+    18 "{:[?;18]}",
+    19 "{:[?;19]}",
+    20 "{:[?;20]}",
+    21 "{:[?;21]}",
+    22 "{:[?;22]}",
+    23 "{:[?;23]}",
+    24 "{:[?;24]}",
+    25 "{:[?;25]}",
+    26 "{:[?;26]}",
+    27 "{:[?;27]}",
+    28 "{:[?;28]}",
+    29 "{:[?;29]}",
+    30 "{:[?;30]}",
+    31 "{:[?;31]}",
+    32 "{:[?;32]}",
+    64 "{:[?;64]}",
+    128 "{:[?;128]}",
+    256 "{:[?;256]}",
+    512 "{:[?;512]}",
+    1024 "{:[?;1024]}",
+    2048 "{:[?;2048]}",
+    4096 "{:[?;4096]}",
+    8192 "{:[?;8192]}",
+    16384 "{:[?;16384]}",
+    32768 "{:[?;32768]}",
+    65536 "{:[?;65536]}",
+    131072 "{:[?;131072]}",
+    262144 "{:[?;262144]}",
+    524288 "{:[?;524288]}",
+    1048576 "{:[?;1048576]}",
+    2097152 "{:[?;2097152]}",
+    4194304 "{:[?;4194304]}",
+    8388608 "{:[?;8388608]}",
+    16777216 "{:[?;16777216]}",
+    33554432 "{:[?;33554432]}",
+    67108864 "{:[?;67108864]}",
+    134217728 "{:[?;134217728]}",
+    268435456 "{:[?;268435456]}",
+    536870912 "{:[?;536870912]}",
+    1073741824 "{:[?;1073741824]}",
+    100 "{:[?;100]}",
+    1000 "{:[?;1000]}",
+    10000 "{:[?;10000]}",
+    100000 "{:[?;100000]}",
+    1000000 "{:[?;1000000]}",
+    10000000 "{:[?;10000000]}",
+    100000000 "{:[?;100000000]}",
+    1000000000 "{:[?;1000000000]}",
 }
 
 impl<T> Format for Option<T>
diff --git a/src/lib.rs b/src/lib.rs
index 2382e47..6dc5d58 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -372,10 +372,21 @@ impl Formatter {
 
     // NOTE: This is passed `&[u8; N]` – it's just coerced to a slice.
     #[doc(hidden)]
-    pub fn array(&mut self, a: &[u8]) {
+    pub fn u8_array(&mut self, a: &[u8]) {
         self.write(a);
     }
 
+    // NOTE: This is passed `&[u8; N]` – it's just coerced to a slice.
+    #[doc(hidden)]
+    pub fn fmt_array(&mut self, a: &[impl Format]) {
+        let mut is_first = true;
+        for value in a {
+            let omit_tag = !is_first;
+            self.fmt(value, omit_tag);
+            is_first = false;
+        }
+    }
+
     /// Implementation detail
     #[doc(hidden)]
     pub fn istr(&mut self, s: &Str) {


Test patch:
diff --git a/tests/encode.rs b/tests/encode.rs
index 4700088..6b7971a 100644
--- a/tests/encode.rs
+++ b/tests/encode.rs
@@ -681,15 +681,26 @@ fn istr() {
 }
 
 #[test]
-fn arrays() {
+fn format_arrays() {
     let index = fetch_string_index();
-    check_format_implementation(&[], &[index]);
+    let array: [u16; 0] = [];
+    check_format_implementation(&array, &[index]);
 
     let index = fetch_string_index();
-    check_format_implementation(&[0], &[index, 0]);
-
-    let index = fetch_string_index();
-    check_format_implementation(&[0xff, 0xab, 0x1f], &[index, 0xff, 0xab, 0x1f]);
+    let array: [u16; 3] = [1, 256, 257];
+    check_format_implementation(
+        &array,
+        &[
+            index,             // "{:[?;3]}"
+            inc(index, 1),     // "{:u16}"
+            1,                 // [0].low
+            0,                 // [0].high
+            0,                 // [1].low
+            1,                 // [1].high
+            1,                 // [2].low
+            1,                 // [2].high
+        ],
+    );
 }
 
 #[test]