Merge #359

359: Implement precedence of inner display hint r=japaric a=justahero

This PR implements precedence of display hints inner types, e.g. `u8`, `u16` etc. It implements the display hint precedence rules given in #319.

When formating variables using the `defmt::Format` trait, either by implementing it or using the `derive` macro, the outer display hint is ignored depending on the inner type & its display hint.

For example, a struct that uses the `derive` marco

```rust
#[derive(Format)]
struct S { x: u8 }

// derive would expand to
impl defmt::Format for S {
    fn format(&self, f: defmt::Formatter) {
        write!(f, "S {{ x: {=u8:?} }}", self.x);
    }
}
```

This uses the `Debug` display hint (`:?`) for the inner type `x` of struct `S`. When displaying the above struct `S` with

```rust
info!("{:x}", S { x: 42 });
// -> INFO S { x: 0x2a }
```

the outer display hint `:x` is applied to the inner type `x`. In this case the outer display hint hexadecimal is used.

On the other hand if the inner type is one of `:x` (hex), `:b` (binary), the outer display hint will be ignored and the inner hint has precedence.

This time the implementation uses the `:b` (binary) display hint to format inner type `x`.

```rust
impl defmt::Format for S {
    fn format(&self, f: defmt::Formatter) {
        write!(f, "S {{ x: {=u8:b} }}", self.x);
    }
}
```

When logging the struct this time

```rust
info!("{:x}", S { x: 42 });
// -> INFO S { x: 0b101010 }
```

the outer display hint `:x` is ignored, the inner display hint `:b` is used, therefore the value `42` is formatted as a binary string `0b101010`.

Another precedence rule is related to `str` fields of structs. As given in the RFC #319, the following struct with an inner `str` field is deriving the `defmt::Format` trait as follows.

```rust
#[derive(Format)]
struct S { x: &'static str }

// derive now expands to
impl defmt::Format for S {
    fn format(&self, f: defmt::Formatter) {
        write!(f, "S {{ x: {=str:?} }}", self.x);
    }
}
```

The difference is `x: &'static str` is expanded to `{=str:?}` instead of a plain `{=str}`. This way string in `x` is enclosed by double quotes.

```rust
defmt::info!("{} world", S { x: "hello" });
// -> "hello" world
defmt::info("hello {:?}", S { x: "world" });
// -> hello "world"
```

Both display hints `{}` and `{:?}` now produce the same desired output.


Co-authored-by: Sebastian Ziebell <sebastian.ziebell@asquera.de>

decoder/src/frame.rs

@@ -287,13 +287,24 @@ fn format_args_real(
                                     format_u128(bitfields as u128, hint, &mut buf)?;
                                 }
                             }
-                            _ => format_u128(*x as u128, hint, &mut buf)?,
+                            _ => {
+                                if let Some(DisplayHint::Debug) = hint {
+                                    format_u128(*x as u128, parent_hint, &mut buf)?;
+                                } else {
+                                    format_u128(*x as u128, hint, &mut buf)?;
+                                }
+                            }
                         }
                     }
                     Arg::Ixx(x) => format_i128(*x as i128, hint, &mut buf)?,
                     Arg::Str(x) | Arg::Preformatted(x) => format_str(x, hint, &mut buf)?,
                     Arg::IStr(x) => format_str(x, hint, &mut buf)?,
-                    Arg::Format { format, args } => buf.push_str(&format_args(format, args, hint)),
+                    Arg::Format { format, args } => match parent_hint {
+                        Some(DisplayHint::Ascii) => {
+                            buf.push_str(&format_args(format, args, parent_hint));
+                        }
+                        _ => buf.push_str(&format_args(format, args, hint)),
+                    },
                     Arg::FormatSlice { elements } => {
                         match hint {
                             // Filter Ascii Hints, which contains u8 byte slices

decoder/src/lib.rs

@@ -549,6 +549,159 @@ mod tests {
         );
     }
 
+    #[test]
+    fn display_use_inner_type_hint() {
+        let mut entries = BTreeMap::new();
+
+        entries.insert(
+            0,
+            TableEntry::new_without_symbol(Tag::Info, "x={:b}".to_owned()),
+        );
+        entries.insert(
+            1,
+            TableEntry::new_without_symbol(Tag::Derived, "S {{ x: {=u8:x} }}".to_owned()),
+        );
+
+        let table = Table {
+            entries,
+            timestamp: Some(TableEntry::new_without_symbol(
+                Tag::Timestamp,
+                "{=u8:µs}".to_owned(),
+            )),
+        };
+
+        let bytes = [
+            0,  // index
+            2,  // timestamp
+            1,  // index of the struct
+            42, // value
+        ];
+
+        let frame = table.decode(&bytes).unwrap().0;
+        assert_eq!(
+            frame.display(false).to_string(),
+            "0.000002 INFO x=S { x: 0x2a }",
+        );
+    }
+
+    #[test]
+    fn display_use_outer_type_hint() {
+        let mut entries = BTreeMap::new();
+
+        entries.insert(
+            0,
+            TableEntry::new_without_symbol(Tag::Info, "x={:b}".to_owned()),
+        );
+        entries.insert(
+            1,
+            TableEntry::new_without_symbol(Tag::Derived, "S {{ x: {=u8:?} }}".to_owned()),
+        );
+
+        let table = Table {
+            entries,
+            timestamp: Some(TableEntry::new_without_symbol(
+                Tag::Timestamp,
+                "{=u8:µs}".to_owned(),
+            )),
+        };
+
+        let bytes = [
+            0,  // index
+            2,  // timestamp
+            1,  // index of the struct
+            42, // value
+        ];
+
+        let frame = table.decode(&bytes).unwrap().0;
+        assert_eq!(
+            frame.display(false).to_string(),
+            "0.000002 INFO x=S { x: 0b101010 }",
+        );
+    }
+
+    #[test]
+    fn display_inner_str_in_struct() {
+        let mut entries = BTreeMap::new();
+
+        entries.insert(
+            0,
+            TableEntry::new_without_symbol(Tag::Info, "{}".to_owned()),
+        );
+        entries.insert(
+            1,
+            TableEntry::new_without_symbol(Tag::Derived, "S {{ x: {=str:?} }}".to_owned()),
+        );
+
+        let table = Table {
+            entries,
+            timestamp: Some(TableEntry::new_without_symbol(
+                Tag::Timestamp,
+                "{=u8:µs}".to_owned(),
+            )),
+        };
+
+        let bytes = [
+            0, // index
+            2, // timestamp
+            1, // index into the struct
+            5, // length of the string
+            b'H', b'e', b'l', b'l', b'o', // string "Hello"
+        ];
+        let frame = table.decode(&bytes).unwrap().0;
+        assert_eq!(
+            frame.display(false).to_string(),
+            "0.000002 INFO S { x: \"Hello\" }",
+        );
+    }
+
+    #[test]
+    fn display_u8_vec() {
+        let mut entries = BTreeMap::new();
+
+        entries.insert(
+            0,
+            TableEntry::new_without_symbol(Tag::Prim, "{=u8}".to_owned()),
+        );
+        entries.insert(
+            1,
+            TableEntry::new_without_symbol(Tag::Prim, "{=[?]}".to_owned()),
+        );
+        entries.insert(
+            2,
+            TableEntry::new_without_symbol(Tag::Derived, "Data {{ name: {=?:?} }}".to_owned()),
+        );
+        entries.insert(
+            3,
+            TableEntry::new_without_symbol(Tag::Info, "{=[?]:a}".to_owned()),
+        );
+
+        let table = Table {
+            entries,
+            timestamp: Some(TableEntry::new_without_symbol(
+                Tag::Timestamp,
+                "{=u8:µs}".to_owned(),
+            )),
+        };
+
+        let bytes = [
+            3,   // frame index
+            2,   // timestamp value of type `u8`
+            1,   // number of elements in `FormatSlice`
+            2,   // index to `Data` struct
+            1,   // Format index to table entry: `{=[?]}`
+            2,   // inner FormatSlice, number of elements in `name` field
+            0,   // Format index to table entry: `{=u8}`
+            72,  // "H"
+            105, // "i"
+        ];
+        let frame = table.decode(&bytes).unwrap().0;
+        dbg!(&frame);
+        assert_eq!(
+            frame.display(false).to_string(),
+            "0.000002 INFO [Data { name: b\"Hi\" }]",
+        );
+    }
+
     #[test]
     fn bools_simple() {
         let bytes = [

firmware/qemu/src/bin/hints_inner.out

@@ -0,0 +1,10 @@
+0.000000 INFO S1 { x: "hi", y: 0x2a }
+0.000001 INFO S2 { x: 0x2a }
+0.000002 WARN Debug hint: S { x: "hello", y: 512 }
+0.000003 WARN    no hint: S { x: "hello", y: 1024 }
+0.000004 INFO S2 { s: S1 { x: 0b100, y: 12 }, z: 20 }
+0.000005 INFO S2 { s: S1 { x: 0b100, y: 12 }, z: 20 }
+0.000006 INFO S2 { s: S1 { x: 0b100, y: 12 }, z: 0x14 }
+0.000007 INFO S2 { s: S1 { x: 0b100, y: 12 }, z: 0b10100 }
+0.000008 INFO S1 { x: "hi", y: 42 }
+0.000009 INFO S1 { x: "hi", y: 0x2a }

firmware/qemu/src/bin/hints_inner.release.out

@@ -0,0 +1,10 @@
+0.000000 INFO S1 { x: "hi", y: 0x2a }
+0.000001 INFO S2 { x: 0x2a }
+0.000002 WARN Debug hint: S { x: "hello", y: 512 }
+0.000003 WARN    no hint: S { x: "hello", y: 1024 }
+0.000004 INFO S2 { s: S1 { x: 0b100, y: 12 }, z: 20 }
+0.000005 INFO S2 { s: S1 { x: 0b100, y: 12 }, z: 20 }
+0.000006 INFO S2 { s: S1 { x: 0b100, y: 12 }, z: 0x14 }
+0.000007 INFO S2 { s: S1 { x: 0b100, y: 12 }, z: 0b10100 }
+0.000008 INFO S1 { x: "hi", y: 42 }
+0.000009 INFO S1 { x: "hi", y: 0x2a }

firmware/qemu/src/bin/hints_inner.rs

@@ -0,0 +1,142 @@
+#![no_std]
+#![no_main]
+
+use core::sync::atomic::{AtomicU32, Ordering};
+
+use cortex_m_rt::entry;
+use cortex_m_semihosting::debug;
+
+use defmt::write;
+use defmt_semihosting as _; // global logger
+
+#[entry]
+fn main() -> ! {
+    {
+        struct S1 {
+            x: &'static str,
+            y: u8,
+        }
+        impl defmt::Format for S1 {
+            fn format(&self, f: defmt::Formatter) {
+                write!(f, "S1 {{ x: {=str:?}, y: {=u8:?} }}", self.x, self.y)
+            }
+        }
+
+        // outputs: "S { x: "hi", y: 0x2a }"
+        defmt::info!("{:x}", S1 { x: "hi", y: 42 });
+    }
+
+    {
+        struct S2 {
+            x: u8,
+        }
+        impl defmt::Format for S2 {
+            fn format(&self, f: defmt::Formatter) {
+                write!(f, "S2 {{ x: {=u8:x} }}", self.x)
+            }
+        }
+
+        // ignores outer :b hint, should output: "S { x: 0x2a }"
+        defmt::info!("{:b}", S2 { x: 42 });
+    }
+
+    {
+        #[derive(defmt::Format)]
+        struct S {
+            x: &'static str,
+            y: u32,
+        }
+
+        // 0.1.x version
+        defmt::warn!("Debug hint: {:?}", S { x: "hello", y: 512 });
+        // 0.2.x version, results in same output
+        defmt::warn!(
+            "   no hint: {}",
+            S {
+                x: "hello",
+                y: 1024
+            }
+        );
+    }
+
+    {
+        // nested struct
+        struct S1 {
+            x: u16,
+            y: u32,
+        }
+        impl defmt::Format for S1 {
+            fn format(&self, f: defmt::Formatter) {
+                write!(f, "S1 {{ x: {=u16:b}, y: {} }}", self.x, self.y);
+            }
+        }
+
+        struct S2 {
+            s: S1,
+            z: u8,
+        }
+        impl defmt::Format for S2 {
+            fn format(&self, f: defmt::Formatter) {
+                write!(f, "S2 {{ s: {:?}, z: {} }}", self.s, self.z);
+            }
+        }
+
+        defmt::info!(
+            "{}",
+            S2 {
+                s: S1 { x: 4, y: 12 },
+                z: 20
+            }
+        );
+        defmt::info!(
+            "{:?}",
+            S2 {
+                s: S1 { x: 4, y: 12 },
+                z: 20
+            }
+        );
+        defmt::info!(
+            "{:x}",
+            S2 {
+                s: S1 { x: 4, y: 12 },
+                z: 20
+            }
+        );
+        defmt::info!(
+            "{:b}",
+            S2 {
+                s: S1 { x: 4, y: 12 },
+                z: 20
+            }
+        );
+    }
+
+    {
+        #[derive(defmt::Format)]
+        struct S1<T> {
+            x: T,
+            y: u8,
+        }
+
+        // outputs: "S { x: "hi", y: 42 }"
+        defmt::info!("{}", S1 { x: "hi", y: 42 });
+        // outputs: "S { x: "hi", y: 0x2a }"
+        defmt::info!("{:x}", S1 { x: "hi", y: 42 });
+    }
+
+    loop {
+        debug::exit(debug::EXIT_SUCCESS)
+    }
+}
+
+static COUNT: AtomicU32 = AtomicU32::new(0);
+defmt::timestamp!("{=u32:µs}", COUNT.fetch_add(1, Ordering::Relaxed));
+
+// like `panic-semihosting` but doesn't print to stdout (that would corrupt the defmt stream)
+#[cfg(target_os = "none")]
+#[panic_handler]
+fn panic(_: &core::panic::PanicInfo) -> ! {
+    loop {
+        debug::exit(debug::EXIT_FAILURE)
+    }
+}

macros/src/lib.rs

@@ -415,7 +415,7 @@ fn fields(
                         "?".to_string()
                     });
                     if let Some(ident) = f.ident.as_ref() {
-                        core::write!(format, "{}: {{={}}}", ident, ty).ok();
+                        core::write!(format, "{}: {{={}:?}}", ident, ty).ok();
 
                         if ty == "?" {
                             list.push(quote!(f.inner.fmt(#ident, false)));