From 7a7f665543179ce57509f82dcf90af5457642e8f Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 30 Jun 2021 03:50:22 +0200 Subject: [PATCH 1/9] test: do not depend on custom InternalFormatter --- macros/src/lib.rs | 56 +++++++++++-------------------- src/export.rs | 17 +++++++--- src/formatter.rs | 24 -------------- tests/encode.rs | 84 +++++++++++++++++++---------------------------- 4 files changed, 65 insertions(+), 116 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 1f7cedd1..e9332cd0 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -169,47 +169,29 @@ pub fn timestamp(ts: TokenStream) -> TokenStream { Err(e) => return e.to_compile_error().into(), }; - if cfg!(feature = "unstable-test") { - // Only check that the formatting arguments compile. - quote!( - const _: () = { - #[allow(unused)] - fn defmt_timestamp(fmt: ::defmt::Formatter<'_>) { - match (fmt.inner, #(&(#args)),*) { - (_fmt_, #(#pats),*) => { - // NOTE: No format string index, and no finalize call. - #(#exprs;)* - } - } - } - }; - ) - .into() - } else { - quote!( - const _: () = { - #[export_name = "_defmt_timestamp"] - fn defmt_timestamp(fmt: ::defmt::Formatter<'_>) { - match (fmt.inner, #(&(#args)),*) { - (_fmt_, #(#pats),*) => { - // NOTE: No format string index, and no finalize call. - #(#exprs;)* - } + quote!( + const _: () = { + #[export_name = "_defmt_timestamp"] + fn defmt_timestamp(fmt: ::defmt::Formatter<'_>) { + match (fmt.inner, #(&(#args)),*) { + (_fmt_, #(#pats),*) => { + // NOTE: No format string index, and no finalize call. + #(#exprs;)* } } + } - #sym; + #sym; - // Unique symbol name to prevent multiple `timestamp!` invocations in the crate graph. - // Uses `#symname` to ensure it is not discarded by the linker. - #[no_mangle] - #[cfg_attr(target_os = "macos", link_section = ".defmt,end.timestamp")] - #[cfg_attr(not(target_os = "macos"), link_section = ".defmt.end.timestamp")] - static __DEFMT_MARKER_TIMESTAMP_WAS_DEFINED: &u8 = &#symname; - }; - ) - .into() - } + // Unique symbol name to prevent multiple `timestamp!` invocations in the crate graph. + // Uses `#symname` to ensure it is not discarded by the linker. + #[no_mangle] + #[cfg_attr(target_os = "macos", link_section = ".defmt,end.timestamp")] + #[cfg_attr(not(target_os = "macos"), link_section = ".defmt.end.timestamp")] + static __DEFMT_MARKER_TIMESTAMP_WAS_DEFINED: &u8 = &#symname; + }; + ) + .into() } /// Returns a list of features of which one has to be enabled for `level` to be active diff --git a/src/export.rs b/src/export.rs index bb297ca9..aef94c4d 100644 --- a/src/export.rs +++ b/src/export.rs @@ -2,10 +2,8 @@ use crate::Str; #[cfg(feature = "unstable-test")] thread_local! { - static I: core::sync::atomic::AtomicU16 = - core::sync::atomic::AtomicU16::new(0); - static T: core::sync::atomic::AtomicU16 = - core::sync::atomic::AtomicU16::new(0); + static I: core::sync::atomic::AtomicU16 = core::sync::atomic::AtomicU16::new(0); + static BYTES: core::cell::RefCell> = core::cell::RefCell::new(Vec::new()); } /// For testing purposes @@ -20,6 +18,12 @@ pub fn fetch_add_string_index() -> usize { (I.with(|i| i.fetch_add(1, core::sync::atomic::Ordering::Relaxed))) as usize } +/// Get and clear the logged bytes +#[cfg(feature = "unstable-test")] +pub fn fetch_bytes() -> Vec { + BYTES.with(|b| core::mem::replace(&mut *b.borrow_mut(), Vec::new())) +} + #[cfg(feature = "unstable-test")] pub fn acquire() {} @@ -44,6 +48,11 @@ pub fn release() { unsafe { _defmt_release() } } +#[cfg(feature = "unstable-test")] +pub fn write(bytes: &[u8]) { + BYTES.with(|b| b.borrow_mut().extend(bytes)) +} + #[cfg(not(feature = "unstable-test"))] #[inline(never)] pub fn write(bytes: &[u8]) { diff --git a/src/formatter.rs b/src/formatter.rs index e90f83b9..52462e8e 100644 --- a/src/formatter.rs +++ b/src/formatter.rs @@ -11,8 +11,6 @@ pub struct Formatter<'a> { #[doc(hidden)] pub struct InternalFormatter { - #[cfg(feature = "unstable-test")] - bytes: Vec, /// Whether to omit the tag of a `Format` value /// /// * this is disabled while formatting a `{:[?]}` value (second element on-wards) @@ -22,7 +20,6 @@ pub struct InternalFormatter { #[doc(hidden)] impl InternalFormatter { - #[cfg(not(feature = "unstable-test"))] pub fn write(&mut self, bytes: &[u8]) { export::write(bytes) } @@ -32,7 +29,6 @@ impl InternalFormatter { /// # Safety /// /// Must only be called when the current execution context has acquired the logger with [Logger::acquire] - #[cfg(not(feature = "unstable-test"))] #[allow(clippy::new_without_default)] pub fn new() -> Self { Self { omit_tag: false } @@ -241,23 +237,3 @@ impl fmt::Write for FmtWrite<'_> { Ok(()) } } - -#[doc(hidden)] -#[cfg(feature = "unstable-test")] -impl super::InternalFormatter { - #[allow(clippy::new_without_default)] - pub fn new() -> Self { - Self { - bytes: vec![], - omit_tag: false, - } - } - - pub fn bytes(&mut self) -> &[u8] { - &self.bytes - } - - pub fn write(&mut self, bytes: &[u8]) { - self.bytes.extend_from_slice(bytes) - } -} diff --git a/tests/encode.rs b/tests/encode.rs index 45fca3cb..36dff2fd 100644 --- a/tests/encode.rs +++ b/tests/encode.rs @@ -21,11 +21,11 @@ // let mut f = InternalFormatter::new(); // let index = defmt::export::fetch_string_index(); // foo(&mut f); // NOTE increases the interner index -// assert_eq!(f.bytes(), [index]); +// assert_eq!(fetch_bytes(), [index]); // // let mut f = InternalFormatter::new(); // foo(&mut f); -// assert_eq!(f.bytes(), [index.wrapping_add(1)]); +// assert_eq!(fetch_bytes(), [index.wrapping_add(1)]); // ^^^^^^^^^^^^^^^ account for the previous `foo` call // ``` // @@ -50,17 +50,17 @@ fn check_format_implementation(val: &(impl Format + ?Sized), expected_encoding: let mut f = InternalFormatter::new(); let g = Formatter { inner: &mut f }; val.format(g); - assert_eq!(f.bytes(), expected_encoding); + assert_eq!(defmt::export::fetch_bytes(), expected_encoding); } macro_rules! check { - ($got:expr, [$($x:expr),* $(,)?]) => { + ([$($x:expr),* $(,)?]) => { { let mut v = Vec::::new(); $( v.extend(&($x).to_le_bytes()); )* - assert_eq!($got, &v); + assert_eq!(defmt::export::fetch_bytes(), v); } }; } @@ -84,25 +84,19 @@ fn write() { let g = Formatter { inner: f }; write!(g, "The answer is {=u8}", 42); - check!( - f.bytes(), - [ - index, // "The answer is {=u8}", - 42u8, // u8 value - ] - ); + check!([ + index, // "The answer is {=u8}", + 42u8, // u8 value + ]); let f2 = &mut InternalFormatter::new(); let g2 = Formatter { inner: f2 }; write!(g2, "The answer is {=?}", 42u8); - check!( - f2.bytes(), - [ - inc(index, 1), // "The answer is {=?}" - inc(index, 2), // "{=u8}" / impl Format for u8 - 42u8, // u8 value - ] - ); + check!([ + inc(index, 1), // "The answer is {=?}" + inc(index, 2), // "{=u8}" / impl Format for u8 + 42u8, // u8 value + ]); } #[test] @@ -116,15 +110,12 @@ fn bitfields_mixed() { "bitfields {0=7..12}, {1=0..5}", 0b1110_0101_1111_0000u16, 0b1111_0000u8 ); - check!( - f.bytes(), - [ - index, // bitfields {0=7..12}, {1=0..5}", - 0b1111_0000u8, - 0b1110_0101u8, // u16 - 0b1111_0000u8, // u8 - ] - ); + check!([ + index, // bitfields {0=7..12}, {1=0..5}", + 0b1111_0000u8, + 0b1110_0101u8, // u16 + 0b1111_0000u8, // u8 + ]); } #[test] @@ -134,14 +125,11 @@ fn bitfields_across_octets() { let g = Formatter { inner: f }; write!(g, "bitfields {0=0..7} {0=9..14}", 0b0110_0011_1101_0010u16); - check!( - f.bytes(), - [ - index, // bitfields {0=0..7} {0=9..14}", - 0b1101_0010u8, - 0b0110_0011u8, // u16 - ] - ); + check!([ + index, // bitfields {0=0..7} {0=9..14}", + 0b1101_0010u8, + 0b0110_0011u8, // u16 + ]); } #[test] @@ -155,13 +143,10 @@ fn bitfields_truncate_lower() { "bitfields {0=9..14}", 0b0000_0000_0000_1111_0110_0011_1101_0010u32 ); - check!( - f.bytes(), - [ - index, // bitfields {0=9..14}", - 0b0110_0011u8, // the first octet should have been truncated away - ] - ); + check!([ + index, // bitfields {0=9..14}", + 0b0110_0011u8, // the first octet should have been truncated away + ]); } #[test] @@ -171,13 +156,10 @@ fn bitfields_assert_range_exclusive() { let g = Formatter { inner: f }; write!(g, "bitfields {0=6..8}", 0b1010_0101u8,); - check!( - f.bytes(), - [ - index, // "bitfields {0=6..8}" - 0b1010_0101u8 - ] - ); + check!([ + index, // "bitfields {0=6..8}" + 0b1010_0101u8 + ]); } #[test] From f8e68fb5918093555f0013b07ef9e2622fc27a09 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 14 Jun 2021 18:28:08 +0200 Subject: [PATCH 2/9] Format trait v2 --- book/src/ser-format.md | 6 +- decoder/src/decoder.rs | 133 +++++-------------- decoder/src/lib.rs | 54 ++++++++ defmt.x.in | 11 +- macros/src/lib.rs | 43 +++---- parser/src/types.rs | 2 + src/export.rs | 14 +- src/formatter.rs | 53 ++------ src/{ => impls}/adapter.rs | 24 ++-- src/impls/alloc_.rs | 28 +--- src/impls/arrays.rs | 14 +- src/impls/core_/mod.rs | 70 ++++++---- src/impls/mod.rs | 30 +++++ src/impls/primitives.rs | 253 +++++++++---------------------------- src/impls/tuples.rs | 32 +++-- src/lib.rs | 3 +- src/traits.rs | 15 +++ tests/encode.rs | 5 +- 18 files changed, 337 insertions(+), 453 deletions(-) rename src/{ => impls}/adapter.rs (85%) diff --git a/book/src/ser-format.md b/book/src/ser-format.md index acefe1bb..cdbaea9c 100644 --- a/book/src/ser-format.md +++ b/book/src/ser-format.md @@ -10,10 +10,8 @@ First let's see how a primitive implements the `Format` trait: # trait Format { fn format(&self, fmt: defmt::Formatter); } impl Format for u8 { fn format(&self, fmt: defmt::Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=u8}"); - fmt.inner.tag(&t); - } + let t = internp!("{=u8}"); + fmt.inner.tag(&t); fmt.inner.u8(self) // on the wire: [1, 42] // string index ^ ^^ `self` diff --git a/decoder/src/decoder.rs b/decoder/src/decoder.rs index af0d6349..9f363a2d 100644 --- a/decoder/src/decoder.rs +++ b/decoder/src/decoder.rs @@ -8,34 +8,14 @@ use crate::{Arg, DecodeError, FormatSliceElement, Table}; use byteorder::{ReadBytesExt, LE}; use defmt_parser::{get_max_bitfield_range, Fragment, Parameter, Type}; -/// List of format strings; used when decoding a `FormatSlice` (`{:[?]}`) argument -#[derive(Debug)] -enum FormatList<'t> { - /// Build the list; used when decoding the first element - Build { formats: Vec<&'t str> }, - /// Use the list; used when decoding the rest of elements - Use { - formats: Vec<&'t str>, - cursor: usize, - }, -} - pub(crate) struct Decoder<'t, 'b> { table: &'t Table, pub bytes: &'b [u8], - format_list: Option>, - // below an enum tags must be included - below_enum: bool, } impl<'t, 'b> Decoder<'t, 'b> { pub fn new(table: &'t Table, bytes: &'b [u8]) -> Self { - Self { - table, - bytes, - format_list: None, - below_enum: false, - } + Self { table, bytes } } /// Sort and deduplicate `params` so that they can be interpreted correctly during decoding @@ -48,28 +28,14 @@ impl<'t, 'b> Decoder<'t, 'b> { params.dedup_by(|a, b| a.index == b.index); } - /// Gets a format string from - /// - the `FormatList`, if it's in `Use` mode, or - /// - from `bytes` and `table` if the `FormatList` is in `Build` mode or was not provided + /// Gets a format string from `bytes` and `table` fn get_format(&mut self) -> Result<&'t str, DecodeError> { - if let Some(FormatList::Use { formats, cursor }) = self.format_list.as_mut() { - if let Some(format) = formats.get(*cursor) { - *cursor += 1; - return Ok(format); - } - } - let index = self.bytes.read_u16::()? as usize; let format = self .table .get_without_level(index as usize) .map_err(|_| DecodeError::Malformed)?; - if let Some(FormatList::Build { formats }) = self.format_list.as_mut() { - if !self.below_enum { - formats.push(format) - } - } Ok(format) } @@ -111,76 +77,19 @@ impl<'t, 'b> Decoder<'t, 'b> { 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 = self.get_format()?; + let is_enum = format.contains('|'); 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 - }; - + let args = self.decode_format(format)?; elements.push(FormatSliceElement { format, args }); } - if is_enum { - self.below_enum = below_enum; - } - Ok(elements) } @@ -230,10 +139,7 @@ impl<'t, 'b> Decoder<'t, 'b> { if format.contains('|') { // enum let variant = self.get_variant(format)?; - let below_enum = self.below_enum; - self.below_enum = true; let inner_args = self.decode_format(variant)?; - self.below_enum = below_enum; args.push(Arg::Format { format: variant, args: inner_args, @@ -332,6 +238,35 @@ impl<'t, 'b> Decoder<'t, 'b> { args.push(Arg::Preformatted(data.into())); } + Type::FormatSequence => { + loop { + let index = self.bytes.read_u16::()? as usize; + if index == 0 { + break; + } + + let format = self + .table + .get_without_level(index as usize) + .map_err(|_| DecodeError::Malformed)?; + + if format.contains('|') { + // enum + let variant = self.get_variant(format)?; + let inner_args = self.decode_format(variant)?; + args.push(Arg::Format { + format: variant, + args: inner_args, + }); + } else { + let inner_args = self.decode_format(format)?; + args.push(Arg::Format { + format, + args: inner_args, + }); + } + } + } } } diff --git a/decoder/src/lib.rs b/decoder/src/lib.rs index cfbad173..8a3e268e 100644 --- a/decoder/src/lib.rs +++ b/decoder/src/lib.rs @@ -515,6 +515,60 @@ mod tests { ); } + #[test] + fn format_sequence() { + let mut entries = BTreeMap::new(); + entries.insert( + 0, + TableEntry::new_without_symbol(Tag::Info, "{=__internal_FormatSequence}".to_owned()), + ); + entries.insert( + 1, + TableEntry::new_without_symbol(Tag::Derived, "Foo".to_owned()), + ); + entries.insert( + 2, + TableEntry::new_without_symbol(Tag::Derived, "Bar({=u8})".to_owned()), + ); + + let table = Table { + entries, + timestamp: None, + }; + + let bytes = [ + 0, 0, // index + 1, 0, // index of Foo + 2, 0, // index of Bar + 42, // bar.x + 0, 0, // terminator + ]; + + assert_eq!( + table.decode(&bytes), + Ok(( + Frame::new( + Level::Info, + 0, + None, + vec![], + "{=__internal_FormatSequence}", + vec![ + Arg::Format { + format: "Foo", + args: vec![] + }, + Arg::Format { + format: "Bar({=u8})", + args: vec![Arg::Uxx(42)] + } + ], + ), + bytes.len(), + )) + ); + } + #[test] fn display() { let mut entries = BTreeMap::new(); diff --git a/defmt.x.in b/defmt.x.in index 40090861..707f249e 100644 --- a/defmt.x.in +++ b/defmt.x.in @@ -8,9 +8,14 @@ PROVIDE(_defmt_panic = __defmt_default_panic); SECTIONS { - /* `0` specifies the start address of this virtual (`(INFO)`) section */ - .defmt 0 (INFO) : + + /* `1` specifies the start address of this virtual (`(INFO)`) section */ + /* Tag number 0 is reserved for special uses, like as a format sequence terminator. */ + .defmt 1 (INFO) : { + /* For some reason the `1` above has no effect, but this does */ + . = 1; + /* Format implementations for primitives like u8 */ *(.defmt.prim.*); @@ -26,4 +31,4 @@ SECTIONS } } -ASSERT(SIZEOF(.defmt) < 16384, ".defmt section cannot contain more than (1<<14) interned strings"); +ASSERT(SIZEOF(.defmt) < 65534, ".defmt section cannot contain more than 65534 interned strings"); diff --git a/macros/src/lib.rs b/macros/src/lib.rs index e9332cd0..5c9080ae 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -234,10 +234,12 @@ pub fn format(ts: TokenStream) -> TokenStream { let mut fs = String::new(); let mut field_types = vec![]; let mut exprs = vec![]; + let sym: TokenStream2; match input.data { Data::Enum(de) => { if de.variants.is_empty() { // For zero-variant enums, this is unreachable code. + sym = quote!(0); exprs.push(quote!(match *self {})); } else { let mut arms = vec![]; @@ -289,24 +291,12 @@ pub fn format(ts: TokenStream) -> TokenStream { arms.push(quote!( #ident::#vident #pats => { #encode_discriminant - - // When descending into an enum variant, force all discriminants to be - // encoded. This is required when encoding arrays like `[None, Some(x)]` - // with `{:?}`, since the format string of `x` won't appear for the - // first element. - f.inner.with_tag(|f| { - #(#exprs;)* - }); + #(#exprs;)* } )) } - let sym = mksym(&fs, "derived", false); - exprs.push(quote!( - if f.inner.needs_tag() { - f.inner.istr(&defmt::export::istr(#sym)); - } - )); + sym = mksym(&fs, "derived", false); exprs.push(quote!(match self { #(#arms)* })); @@ -318,12 +308,7 @@ pub fn format(ts: TokenStream) -> TokenStream { let mut pats = vec![]; let args = fields(&ds.fields, &mut fs, &mut field_types, &mut pats); - let sym = mksym(&fs, "derived", false); - exprs.push(quote!( - if f.inner.needs_tag() { - f.inner.istr(&defmt::export::istr(#sym)); - } - )); + sym = mksym(&fs, "derived", false); exprs.push(quote!(match self { Self { #(#pats),* } => { #(#args;)* @@ -355,6 +340,12 @@ pub fn format(ts: TokenStream) -> TokenStream { quote!( impl #impl_generics defmt::Format for #ident #type_generics #where_clause { fn format(&self, f: defmt::Formatter) { + unreachable!() + } + fn _format_tag() -> u16 { + (#sym) as u16 + } + fn _format_data(&self, f: defmt::Formatter) { #(#exprs)* } } @@ -400,7 +391,7 @@ fn fields( core::write!(format, "{}: {{={}}}", ident, ty).ok(); if ty == "?" { - list.push(quote!(f.inner.fmt(#ident, false))); + list.push(quote!(f.inner.fmt(#ident))); } else { let method = format_ident!("{}", ty); list.push(quote!(f.inner.#method(#ident))); @@ -413,7 +404,7 @@ fn fields( let ident = format_ident!("arg{}", i); if ty == "?" { - list.push(quote!(f.inner.fmt(#ident, false))); + list.push(quote!(f.inner.fmt(#ident))); } else { let method = format_ident!("{}", ty); list.push(quote!(f.inner.#method(#ident))); @@ -1017,10 +1008,7 @@ pub fn write(ts: TokenStream) -> TokenStream { let fmt: defmt::Formatter<'_> = #fmt; match (fmt.inner, #(&(#args)),*) { (_fmt_, #(#pats),*) => { - // HACK conditional should not be here; see FIXME in `format` - if _fmt_.needs_tag() { - _fmt_.istr(&defmt::export::istr(#sym)); - } + _fmt_.istr(&defmt::export::istr(#sym)); #(#exprs;)* } } @@ -1151,7 +1139,7 @@ impl Codegen { defmt_parser::Type::IStr => exprs.push(quote!(_fmt_.istr(#arg))), defmt_parser::Type::Char => exprs.push(quote!(_fmt_.u32(&(*#arg as u32)))), - defmt_parser::Type::Format => exprs.push(quote!(_fmt_.fmt(#arg, false))), + defmt_parser::Type::Format => exprs.push(quote!(_fmt_.fmt(#arg))), defmt_parser::Type::FormatSlice => exprs.push(quote!(_fmt_.fmt_slice(#arg))), defmt_parser::Type::FormatArray(len) => exprs.push(quote!(_fmt_.fmt_array({ let tmp: &[_; #len] = #arg; @@ -1160,6 +1148,7 @@ impl Codegen { defmt_parser::Type::Debug => exprs.push(quote!(_fmt_.debug(#arg))), defmt_parser::Type::Display => exprs.push(quote!(_fmt_.display(#arg))), + defmt_parser::Type::FormatSequence => unreachable!(), defmt_parser::Type::U8Slice => exprs.push(quote!(_fmt_.slice(#arg))), // We cast to the expected array type (which should be a no-op cast) to provoke diff --git a/parser/src/types.rs b/parser/src/types.rs index 3b7d1b73..cf1f7c49 100644 --- a/parser/src/types.rs +++ b/parser/src/types.rs @@ -9,6 +9,7 @@ pub enum Type { Debug, Display, + FormatSequence, F32, F64, @@ -67,6 +68,7 @@ impl FromStr for Type { "istr" => Type::IStr, "__internal_Debug" => Type::Debug, "__internal_Display" => Type::Display, + "__internal_FormatSequence" => Type::FormatSequence, "[u8]" => Type::U8Slice, "?" => Type::Format, "[?]" => Type::FormatSlice, diff --git a/src/export.rs b/src/export.rs index aef94c4d..9a0f8c46 100644 --- a/src/export.rs +++ b/src/export.rs @@ -86,7 +86,6 @@ mod sealed { #[allow(unused_imports)] use crate as defmt; use crate::{Format, Formatter}; - use defmt_macros::internp; pub trait Truncate { fn truncate(self) -> U; @@ -190,12 +189,15 @@ mod sealed { pub struct NoneError; impl Format for NoneError { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("Unwrap of a None option value"); - fmt.inner.tag(&t); - } + fn format(&self, _fmt: Formatter) { + unreachable!(); } + + fn _format_tag() -> u16 { + defmt_macros::internp!("Unwrap of a None option value") + } + + fn _format_data(&self, _fmt: Formatter) {} } pub trait IntoResult { diff --git a/src/formatter.rs b/src/formatter.rs index 52462e8e..a4cf7b84 100644 --- a/src/formatter.rs +++ b/src/formatter.rs @@ -10,13 +10,7 @@ pub struct Formatter<'a> { } #[doc(hidden)] -pub struct InternalFormatter { - /// Whether to omit the tag of a `Format` value - /// - /// * this is disabled while formatting a `{:[?]}` value (second element on-wards) - /// * this is force-enabled while formatting enums - omit_tag: bool, -} +pub struct InternalFormatter {} #[doc(hidden)] impl InternalFormatter { @@ -31,44 +25,19 @@ impl InternalFormatter { /// Must only be called when the current execution context has acquired the logger with [Logger::acquire] #[allow(clippy::new_without_default)] pub fn new() -> Self { - Self { omit_tag: false } + Self {} } // TODO turn these public methods in `export` free functions /// Implementation detail - pub fn fmt(&mut self, f: &impl Format, omit_tag: bool) { - let old_omit_tag = self.omit_tag; - if omit_tag { - self.omit_tag = true; - } - - let formatter = Formatter { inner: self }; - f.format(formatter); - - if omit_tag { - // restore - self.omit_tag = old_omit_tag; - } - } - - /// Implementation detail - pub fn needs_tag(&self) -> bool { - !self.omit_tag - } - - /// Implementation detail - pub fn with_tag(&mut self, f: impl FnOnce(Formatter)) { - let omit_tag = self.omit_tag; - self.omit_tag = false; - + pub fn fmt(&mut self, f: &T) { + self.tag(T::_format_tag()); let formatter = Formatter { inner: self }; - f(formatter); - // restore - self.omit_tag = omit_tag; + f._format_data(formatter); } /// Implementation detail - pub fn tag(&mut self, b: &u16) { + pub fn tag(&mut self, b: u16) { self.write(&b.to_le_bytes()) } @@ -105,11 +74,8 @@ impl InternalFormatter { /// Implementation detail pub fn fmt_slice(&mut self, values: &[impl Format]) { self.usize(&values.len()); - let mut is_first = true; for value in values { - let omit_tag = !is_first; - self.fmt(value, omit_tag); - is_first = false; + self.fmt(value); } } @@ -181,11 +147,8 @@ impl InternalFormatter { // NOTE: This is passed `&[u8; N]` – it's just coerced to a slice. 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; + self.fmt(value); } } diff --git a/src/adapter.rs b/src/impls/adapter.rs similarity index 85% rename from src/adapter.rs rename to src/impls/adapter.rs index 49590fd3..70676481 100644 --- a/src/adapter.rs +++ b/src/impls/adapter.rs @@ -26,11 +26,13 @@ use crate::{Format, Formatter}; pub struct Debug2Format<'a, T: fmt::Debug + ?Sized>(pub &'a T); impl Format for Debug2Format<'_, T> { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = defmt_macros::internp!("{=__internal_Debug}"); - fmt.inner.tag(&t); - } + default_format!(); + + fn _format_tag() -> u16 { + defmt_macros::internp!("{=__internal_Debug}") + } + + fn _format_data(&self, fmt: Formatter) { fmt.inner.debug(&self.0); } } @@ -62,11 +64,13 @@ impl Format for Debug2Format<'_, T> { pub struct Display2Format<'a, T: fmt::Display + ?Sized>(pub &'a T); impl Format for Display2Format<'_, T> { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = defmt_macros::internp!("{=__internal_Display}"); - fmt.inner.tag(&t); - } + default_format!(); + + fn _format_tag() -> u16 { + defmt_macros::internp!("{=__internal_Display}") + } + + fn _format_data(&self, fmt: Formatter) { fmt.inner.display(&self.0); } } diff --git a/src/impls/alloc_.rs b/src/impls/alloc_.rs index 7fd0c71e..3e3e711a 100644 --- a/src/impls/alloc_.rs +++ b/src/impls/alloc_.rs @@ -4,18 +4,14 @@ impl Format for alloc::boxed::Box where T: ?Sized + Format, { - fn format(&self, f: Formatter) { - T::format(&*self, f) - } + delegate_format!(T, self, &*self); } impl Format for alloc::rc::Rc where T: ?Sized + Format, { - fn format(&self, f: Formatter) { - T::format(&*self, f) - } + delegate_format!(T, self, &*self); } #[cfg(not(no_cas))] @@ -23,24 +19,18 @@ impl Format for alloc::sync::Arc where T: ?Sized + Format, { - fn format(&self, f: Formatter) { - T::format(&*self, f) - } + delegate_format!(T, self, &*self); } impl Format for alloc::vec::Vec where T: Format, { - fn format(&self, f: Formatter) { - self.as_slice().format(f) - } + delegate_format!([T], self, self.as_slice()); } impl Format for alloc::string::String { - fn format(&self, f: Formatter) { - self.as_str().format(f) - } + delegate_format!(str, self, self.as_str()); } impl<'a, T> Format for alloc::borrow::Cow<'a, [T]> @@ -48,13 +38,9 @@ where T: 'a + Format, [T]: alloc::borrow::ToOwned>, { - fn format(&self, f: Formatter) { - self.as_ref().format(f) - } + delegate_format!([T], self, self.as_ref()); } impl<'a> Format for alloc::borrow::Cow<'a, str> { - fn format(&self, f: Formatter) { - self.as_ref().format(f) - } + delegate_format!(str, self, self.as_ref()); } diff --git a/src/impls/arrays.rs b/src/impls/arrays.rs index 2f46915a..d4961fec 100644 --- a/src/impls/arrays.rs +++ b/src/impls/arrays.rs @@ -6,11 +6,15 @@ macro_rules! arrays { where T: Format { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!($fmt); - fmt.inner.tag(&t); - } + default_format!(); + + #[inline] + fn _format_tag() -> u16 { + internp!($fmt) + } + + #[inline] + fn _format_data(&self, fmt: Formatter) { fmt.inner.fmt_array(self); } } diff --git a/src/impls/core_/mod.rs b/src/impls/core_/mod.rs index 2ba00cee..3289d3e7 100644 --- a/src/impls/core_/mod.rs +++ b/src/impls/core_/mod.rs @@ -15,16 +15,21 @@ impl Format for Option where T: Format, { - fn format(&self, f: Formatter) { - if f.inner.needs_tag() { - let t = internp!("None|Some({=?})"); - f.inner.tag(&t); - } + default_format!(); + + #[inline] + fn _format_tag() -> u16 { + internp!("None|Some({=?})") + } + + #[inline] + fn _format_data(&self, fmt: Formatter) { match self { - None => f.inner.u8(&0), + None => fmt.inner.u8(&0), Some(x) => { - f.inner.u8(&1); - f.inner.with_tag(|f| x.format(f)) + fmt.inner.u8(&1); + fmt.inner.tag(T::_format_tag()); + x._format_data(fmt) } } } @@ -35,38 +40,53 @@ where T: Format, E: Format, { - fn format(&self, f: Formatter) { - if f.inner.needs_tag() { - let t = internp!("Err({=?})|Ok({=?})"); - f.inner.tag(&t); - } + default_format!(); + + #[inline] + fn _format_tag() -> u16 { + internp!("Err({=?})|Ok({=?})") + } + + #[inline] + fn _format_data(&self, fmt: Formatter) { match self { Err(e) => { - f.inner.u8(&0); - f.inner.with_tag(|f| e.format(f)) + fmt.inner.u8(&0); + fmt.inner.tag(E::_format_tag()); + e._format_data(fmt) } Ok(x) => { - f.inner.u8(&1); - f.inner.with_tag(|f| x.format(f)) + fmt.inner.u8(&1); + fmt.inner.tag(T::_format_tag()); + x._format_data(fmt) } } } } impl Format for core::marker::PhantomData { - fn format(&self, f: Formatter) { - if f.inner.needs_tag() { - let t = internp!("PhantomData"); - f.inner.tag(&t); - } + default_format!(); + + #[inline] + fn _format_tag() -> u16 { + internp!("PhantomData") } + + #[inline] + fn _format_data(&self, _fmt: Formatter) {} } impl Format for core::convert::Infallible { + default_format!(); + + #[inline] + fn _format_tag() -> u16 { + unreachable!(); + } + #[inline] - fn format(&self, _: Formatter) { - // type cannot be instantiated so nothing to do here - match *self {} + fn _format_data(&self, _fmt: Formatter) { + unreachable!(); } } diff --git a/src/impls/mod.rs b/src/impls/mod.rs index 57fd0234..d3d78a8e 100644 --- a/src/impls/mod.rs +++ b/src/impls/mod.rs @@ -1,3 +1,33 @@ +macro_rules! default_format { + () => { + #[inline] + fn format(&self, fmt: Formatter) { + fmt.inner.tag(Self::_format_tag()); + self._format_data(fmt); + } + }; +} + +macro_rules! delegate_format { + ($ty:ty, $self_:ident, $val:expr) => { + #[inline] + fn format(&$self_, fmt: Formatter) { + <$ty as Format>::format($val, fmt) + } + + #[inline] + fn _format_tag() -> u16 { + <$ty as Format>::_format_tag() + } + + #[inline] + fn _format_data(&$self_, fmt: Formatter) { + <$ty as Format>::_format_data($val, fmt) + } + }; +} + +pub mod adapter; #[cfg(feature = "alloc")] mod alloc_; mod arrays; diff --git a/src/impls/primitives.rs b/src/impls/primitives.rs index 0aa2a661..ffddc640 100644 --- a/src/impls/primitives.rs +++ b/src/impls/primitives.rs @@ -1,175 +1,51 @@ use super::*; -impl Format for i8 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=i8}"); - fmt.inner.tag(&t); - } - fmt.inner.u8(&(*self as u8)); - } -} - -impl Format for i16 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=i16}"); - fmt.inner.tag(&t); - } - fmt.inner.u16(&(*self as u16)) - } -} - -impl Format for i32 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=i32}"); - fmt.inner.tag(&t); - } - fmt.inner.i32(self); - } -} - -impl Format for i64 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=i64}"); - fmt.inner.tag(&t); - } - fmt.inner.i64(self); - } -} - -impl Format for i128 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=i128}"); - fmt.inner.tag(&t); - } - fmt.inner.i128(self); - } -} - -impl Format for isize { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=isize}"); - fmt.inner.tag(&t); - } - fmt.inner.isize(self); - } -} +macro_rules! prim { + ($ty:ty, $method:ident, $fmt: literal) => { + impl Format for $ty { + default_format!(); + + #[inline] + fn _format_tag() -> u16 { + internp!($fmt) + } + + #[inline] + fn _format_data(&self, fmt: Formatter) { + fmt.inner.$method(self); + } + } + }; +} + +prim!(i8, i8, "{=i8}"); +prim!(i16, i16, "{=i16}"); +prim!(i32, i32, "{=i32}"); +prim!(i64, i64, "{=i64}"); +prim!(i128, i128, "{=i128}"); +prim!(isize, isize, "{=isize}"); +prim!(u8, u8, "{=u8}"); +prim!(u16, u16, "{=u16}"); +prim!(u32, u32, "{=u32}"); +prim!(u64, u64, "{=u64}"); +prim!(u128, u128, "{=u128}"); +prim!(usize, usize, "{=usize}"); +prim!(f32, f32, "{=f32}"); +prim!(f64, f64, "{=f64}"); +prim!(str, str, "{=str}"); +prim!(bool, bool, "{=bool}"); +prim!(Str, istr, "{=istr}"); -impl Format for u8 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=u8}"); - fmt.inner.tag(&t); - } - fmt.inner.u8(self) - } -} - -impl Format for u16 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=u16}"); - fmt.inner.tag(&t); - } - fmt.inner.u16(self); - } -} - -impl Format for u32 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=u32}"); - fmt.inner.tag(&t); - } - fmt.inner.u32(self); - } -} - -impl Format for u64 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=u64}"); - fmt.inner.tag(&t); - } - fmt.inner.u64(self); - } -} - -impl Format for u128 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=u128}"); - fmt.inner.tag(&t); - } - fmt.inner.u128(self); - } -} - -impl Format for usize { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=usize}"); - fmt.inner.tag(&t); - } - fmt.inner.usize(self); - } -} - -impl Format for f32 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=f32}"); - fmt.inner.tag(&t); - } - fmt.inner.f32(self); - } -} - -impl Format for f64 { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=f64}"); - fmt.inner.tag(&t); - } - fmt.inner.f64(self); - } -} - -impl Format for str { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = str_tag(); - fmt.inner.tag(&t); - } - fmt.inner.str(self); - } -} - -pub(crate) fn str_tag() -> u16 { - internp!("{=str}") -} +impl Format for char { + default_format!(); -impl Format for Str { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=istr}"); - fmt.inner.tag(&t); - } - fmt.inner.istr(self); + #[inline] + fn _format_tag() -> u16 { + internp!("{=char}") } -} -impl Format for char { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=char}"); - fmt.inner.tag(&t); - } + #[inline] + fn _format_data(&self, fmt: Formatter) { fmt.inner.u32(&(*self as u32)); } } @@ -178,12 +54,20 @@ impl Format for [T] where T: Format, { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=[?]}"); - fmt.inner.tag(&t); + default_format!(); + + #[inline] + fn _format_tag() -> u16 { + internp!("{=[?]}") + } + + #[inline] + fn _format_data(&self, fmt: Formatter) { + fmt.inner.usize(&self.len()); + for value in self { + fmt.inner.tag(T::_format_tag()); + value._format_data(Formatter { inner: fmt.inner }); } - fmt.inner.fmt_slice(self) } } @@ -191,37 +75,14 @@ impl Format for &'_ T where T: Format + ?Sized, { - fn format(&self, fmt: Formatter) { - T::format(self, fmt) - } + delegate_format!(T, self, self); } impl Format for &'_ mut T where T: Format + ?Sized, { - fn format(&self, fmt: Formatter) { - T::format(self, fmt) - } -} - -impl Format for bool { - fn format(&self, fmt: Formatter) { - if fmt.inner.needs_tag() { - let t = internp!("{=bool}"); - fmt.inner.tag(&t); - } - fmt.inner.bool(self); - } -} - -impl Format for () { - fn format(&self, f: Formatter) { - if f.inner.needs_tag() { - let t = internp!("()"); - f.inner.tag(&t); - } - } + delegate_format!(T, self, self); } // Format raw pointer as hexadecimal diff --git a/src/impls/tuples.rs b/src/impls/tuples.rs index d3602b64..7b428a64 100644 --- a/src/impls/tuples.rs +++ b/src/impls/tuples.rs @@ -1,19 +1,35 @@ use super::*; +impl Format for () { + default_format!(); + + #[inline] + fn _format_tag() -> u16 { + internp!("()") + } + + #[inline] + fn _format_data(&self, _fmt: Formatter) {} +} + macro_rules! tuple { ( $format:expr, ($($name:ident),+) ) => ( impl<$($name:Format),+> Format for ($($name,)+) where last_type!($($name,)+): ?Sized { - #[allow(non_snake_case, unused_assignments)] - fn format(&self, f: Formatter) { - if f.inner.needs_tag() { - let t = internp!($format); - f.inner.tag(&t); - } + default_format!(); + + #[inline] + fn _format_tag() -> u16 { + internp!($format) + } + #[inline] + #[allow(non_snake_case, unused_assignments)] + fn _format_data(&self, fmt: Formatter) { let ($(ref $name,)+) = *self; $( - let formatter = Formatter { inner: f.inner }; - $name.format(formatter); + fmt.inner.tag($name::_format_tag()); + let formatter = Formatter { inner: fmt.inner }; + $name._format_data(formatter); )+ } } diff --git a/src/lib.rs b/src/lib.rs index 108ab206..8ab010df 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,7 +18,6 @@ #[cfg(feature = "alloc")] extern crate alloc; -mod adapter; #[doc(hidden)] pub mod export; mod formatter; @@ -28,8 +27,8 @@ mod tests; mod traits; pub use crate::{ - adapter::{Debug2Format, Display2Format}, formatter::{Formatter, InternalFormatter, Str}, + impls::adapter::{Debug2Format, Display2Format}, traits::{Format, Logger}, }; diff --git a/src/traits.rs b/src/traits.rs index 2f8f330d..2b684e16 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -1,3 +1,7 @@ +use defmt_macros::internp; + +#[allow(unused_imports)] +use crate as defmt; use crate::Formatter; /// Trait for types that can be formatted via defmt. @@ -43,6 +47,17 @@ use crate::Formatter; pub trait Format { /// Writes the defmt representation of `self` to `fmt`. fn format(&self, fmt: Formatter); + + #[doc(hidden)] + fn _format_tag() -> u16 { + internp!("{=__internal_FormatSequence}") + } + + #[doc(hidden)] + fn _format_data(&self, fmt: Formatter) { + self.format(Formatter { inner: fmt.inner }); + fmt.inner.tag(0); // terminator + } } /// Global logger acquire-release mechanism diff --git a/tests/encode.rs b/tests/encode.rs index 36dff2fd..bcf5167c 100644 --- a/tests/encode.rs +++ b/tests/encode.rs @@ -46,10 +46,11 @@ fn inc(index: u16, n: u16) -> u16 { index.wrapping_add(n) } -fn check_format_implementation(val: &(impl Format + ?Sized), expected_encoding: &[u8]) { +fn check_format_implementation(val: &T, expected_encoding: &[u8]) { let mut f = InternalFormatter::new(); + f.tag(T::_format_tag()); let g = Formatter { inner: &mut f }; - val.format(g); + val._format_data(g); assert_eq!(defmt::export::fetch_bytes(), expected_encoding); } From 5cac3902f0c6c1ca565f518bed120f816e6360e5 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 30 Jun 2021 06:01:39 +0200 Subject: [PATCH 3/9] Remove InternalFormatter --- book/src/ser-format.md | 10 +-- macros/src/lib.rs | 141 +++++++++++++++-------------- src/export.rs | 173 ++++++++++++++++++++++++++++++++++-- src/formatter.rs | 191 +--------------------------------------- src/impls/adapter.rs | 15 ++-- src/impls/arrays.rs | 7 +- src/impls/core_/mod.rs | 23 ++--- src/impls/mod.rs | 4 +- src/impls/primitives.rs | 69 ++++++--------- src/impls/tuples.rs | 11 +-- src/lib.rs | 2 +- src/traits.rs | 10 +-- tests/encode.rs | 35 +++----- 13 files changed, 323 insertions(+), 368 deletions(-) diff --git a/book/src/ser-format.md b/book/src/ser-format.md index cdbaea9c..096898ca 100644 --- a/book/src/ser-format.md +++ b/book/src/ser-format.md @@ -4,15 +4,15 @@ The untyped argument (`=?`) requires one level of indirection during serializati First let's see how a primitive implements the `Format` trait: -``` rust +```rust # extern crate defmt; -# macro_rules! internp { ($l:literal) => { 0 } } +# macro_rules! internp { ($l:literal) => { defmt::export::make_istr(0) } } # trait Format { fn format(&self, fmt: defmt::Formatter); } impl Format for u8 { fn format(&self, fmt: defmt::Formatter) { let t = internp!("{=u8}"); - fmt.inner.tag(&t); - fmt.inner.u8(self) + defmt::export::istr(&t); + defmt::export::u8(self) // on the wire: [1, 42] // string index ^ ^^ `self` } @@ -25,7 +25,7 @@ In general, `write!` can use `{=?}` so `Format` nesting is possible. Now let's look into a log invocation: -``` rust +```rust # extern crate defmt; defmt::error!("The answer is {=?}!", 42u8); // on the wire: [2, 1, 42] diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 5c9080ae..45fbc3d6 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -173,9 +173,9 @@ pub fn timestamp(ts: TokenStream) -> TokenStream { const _: () = { #[export_name = "_defmt_timestamp"] fn defmt_timestamp(fmt: ::defmt::Formatter<'_>) { - match (fmt.inner, #(&(#args)),*) { - (_fmt_, #(#pats),*) => { - // NOTE: No format string index, and no finalize call. + match (#(&(#args)),*) { + (#(#pats),*) => { + // NOTE: No format string index, and no finalize call. #(#exprs;)* } } @@ -239,7 +239,7 @@ pub fn format(ts: TokenStream) -> TokenStream { Data::Enum(de) => { if de.variants.is_empty() { // For zero-variant enums, this is unreachable code. - sym = quote!(0); + sym = mksym("!", "derived", false); exprs.push(quote!(match *self {})); } else { let mut arms = vec![]; @@ -264,19 +264,19 @@ pub fn format(ts: TokenStream) -> TokenStream { quote!() } else if let (Ok(_), Ok(i)) = (u8::try_from(len), u8::try_from(i)) { quote!( - f.inner.u8(&#i); + defmt::export::u8(&#i); ) } else if let (Ok(_), Ok(i)) = (u16::try_from(len), u16::try_from(i)) { quote!( - f.inner.u16(&#i); + defmt::export::u16(&#i); ) } else if let (Ok(_), Ok(i)) = (u32::try_from(len), u32::try_from(i)) { quote!( - f.inner.u32(&#i); + defmt::export::u32(&#i); ) } else if let (Ok(_), Ok(i)) = (u64::try_from(len), u64::try_from(i)) { quote!( - f.inner.u64(&#i); + defmt::export::u64(&#i); ) } else { // u128 case is omitted with the assumption, that usize is never greater than u64 @@ -342,8 +342,8 @@ pub fn format(ts: TokenStream) -> TokenStream { fn format(&self, f: defmt::Formatter) { unreachable!() } - fn _format_tag() -> u16 { - (#sym) as u16 + fn _format_tag() -> defmt::Str { + #sym } fn _format_data(&self, f: defmt::Formatter) { #(#exprs)* @@ -391,10 +391,10 @@ fn fields( core::write!(format, "{}: {{={}}}", ident, ty).ok(); if ty == "?" { - list.push(quote!(f.inner.fmt(#ident))); + list.push(quote!(defmt::export::fmt(#ident))); } else { let method = format_ident!("{}", ty); - list.push(quote!(f.inner.#method(#ident))); + list.push(quote!(defmt::export::#method(#ident))); } pats.push(quote!( #ident )); } else { @@ -404,10 +404,10 @@ fn fields( let ident = format_ident!("arg{}", i); if ty == "?" { - list.push(quote!(f.inner.fmt(#ident))); + list.push(quote!(defmt::export::fmt(#ident))); } else { let method = format_ident!("{}", ty); - list.push(quote!(f.inner.#method(#ident))); + list.push(quote!(defmt::export::#method(#ident))); } let i = syn::Index::from(i); @@ -493,8 +493,7 @@ fn log(level: Level, log: FormatArgs) -> TokenStream2 { match (#(&(#args)),*) { (#(#pats),*) => { defmt::export::acquire(); - let mut _fmt_ = defmt::InternalFormatter::new(); - _fmt_.header(&defmt::export::istr(#sym)); + defmt::export::header(&#sym); #(#exprs;)* defmt::export::release() } @@ -948,11 +947,7 @@ pub fn intern(ts: TokenStream) -> TokenStream { let lit = parse_macro_input!(ts as LitStr); let ls = lit.value(); - let sym = mksym(&ls, "str", false); - quote!({ - defmt::export::istr(#sym) - }) - .into() + mksym(&ls, "str", false).into() } #[proc_macro] @@ -965,7 +960,7 @@ pub fn internp(ts: TokenStream) -> TokenStream { let section = mksection(false, "prim.", &sym); let section_macos = mksection(true, "prim.", &sym); - if cfg!(feature = "unstable-test") { + let sym = if cfg!(feature = "unstable-test") { quote!({ defmt::export::fetch_add_string_index() as u16 }) } else { quote!({ @@ -975,7 +970,11 @@ pub fn internp(ts: TokenStream) -> TokenStream { static S: u8 = 0; &S as *const u8 as u16 }) - } + }; + + quote!({ + defmt::export::make_istr(#sym) + }) .into() } @@ -1006,9 +1005,9 @@ pub fn write(ts: TokenStream) -> TokenStream { let sym = mksym(&ls, "write", false); quote!({ let fmt: defmt::Formatter<'_> = #fmt; - match (fmt.inner, #(&(#args)),*) { - (_fmt_, #(#pats),*) => { - _fmt_.istr(&defmt::export::istr(#sym)); + match (#(&(#args)),*) { + (#(#pats),*) => { + defmt::export::istr(&#sym); #(#exprs;)* } } @@ -1054,15 +1053,19 @@ fn mksym(string: &str, tag: &str, is_log_statement: bool) -> TokenStream2 { } else { format_ident!("S") }; - if cfg!(feature = "unstable-test") { + let sym = if cfg!(feature = "unstable-test") { quote!({ defmt::export::fetch_add_string_index() }) } else { let statik = mkstatic(varname.clone(), string, tag); quote!({ #statik - &#varname as *const u8 as usize + &#varname as *const u8 as u16 }) - } + }; + + quote!({ + defmt::export::make_istr(#sym) + }) } struct Write { @@ -1116,41 +1119,45 @@ impl Codegen { // find first use of this argument and return its type let param = parsed_params.iter().find(|param| param.index == i).unwrap(); match param.ty { - defmt_parser::Type::I8 => exprs.push(quote!(_fmt_.i8(#arg))), - defmt_parser::Type::I16 => exprs.push(quote!(_fmt_.i16(#arg))), - defmt_parser::Type::I32 => exprs.push(quote!(_fmt_.i32(#arg))), - defmt_parser::Type::I64 => exprs.push(quote!(_fmt_.i64(#arg))), - defmt_parser::Type::I128 => exprs.push(quote!(_fmt_.i128(#arg))), - defmt_parser::Type::Isize => exprs.push(quote!(_fmt_.isize(#arg))), - - defmt_parser::Type::U8 => exprs.push(quote!(_fmt_.u8(#arg))), - defmt_parser::Type::U16 => exprs.push(quote!(_fmt_.u16(#arg))), - defmt_parser::Type::U32 => exprs.push(quote!(_fmt_.u32(#arg))), - defmt_parser::Type::U64 => exprs.push(quote!(_fmt_.u64(#arg))), - defmt_parser::Type::U128 => exprs.push(quote!(_fmt_.u128(#arg))), - defmt_parser::Type::Usize => exprs.push(quote!(_fmt_.usize(#arg))), - - defmt_parser::Type::F32 => exprs.push(quote!(_fmt_.f32(#arg))), - defmt_parser::Type::F64 => exprs.push(quote!(_fmt_.f64(#arg))), - - defmt_parser::Type::Bool => exprs.push(quote!(_fmt_.bool(#arg))), - - defmt_parser::Type::Str => exprs.push(quote!(_fmt_.str(#arg))), - defmt_parser::Type::IStr => exprs.push(quote!(_fmt_.istr(#arg))), - defmt_parser::Type::Char => exprs.push(quote!(_fmt_.u32(&(*#arg as u32)))), - - defmt_parser::Type::Format => exprs.push(quote!(_fmt_.fmt(#arg))), - defmt_parser::Type::FormatSlice => exprs.push(quote!(_fmt_.fmt_slice(#arg))), - defmt_parser::Type::FormatArray(len) => exprs.push(quote!(_fmt_.fmt_array({ - let tmp: &[_; #len] = #arg; - tmp - }))), + defmt_parser::Type::I8 => exprs.push(quote!(defmt::export::i8(#arg))), + defmt_parser::Type::I16 => exprs.push(quote!(defmt::export::i16(#arg))), + defmt_parser::Type::I32 => exprs.push(quote!(defmt::export::i32(#arg))), + defmt_parser::Type::I64 => exprs.push(quote!(defmt::export::i64(#arg))), + defmt_parser::Type::I128 => exprs.push(quote!(defmt::export::i128(#arg))), + defmt_parser::Type::Isize => exprs.push(quote!(defmt::export::isize(#arg))), + + defmt_parser::Type::U8 => exprs.push(quote!(defmt::export::u8(#arg))), + defmt_parser::Type::U16 => exprs.push(quote!(defmt::export::u16(#arg))), + defmt_parser::Type::U32 => exprs.push(quote!(defmt::export::u32(#arg))), + defmt_parser::Type::U64 => exprs.push(quote!(defmt::export::u64(#arg))), + defmt_parser::Type::U128 => exprs.push(quote!(defmt::export::u128(#arg))), + defmt_parser::Type::Usize => exprs.push(quote!(defmt::export::usize(#arg))), + + defmt_parser::Type::F32 => exprs.push(quote!(defmt::export::f32(#arg))), + defmt_parser::Type::F64 => exprs.push(quote!(defmt::export::f64(#arg))), + + defmt_parser::Type::Bool => exprs.push(quote!(defmt::export::bool(#arg))), + + defmt_parser::Type::Str => exprs.push(quote!(defmt::export::str(#arg))), + defmt_parser::Type::IStr => exprs.push(quote!(defmt::export::istr(#arg))), + defmt_parser::Type::Char => exprs.push(quote!(defmt::export::char(#arg))), + + defmt_parser::Type::Format => exprs.push(quote!(defmt::export::fmt(#arg))), + defmt_parser::Type::FormatSlice => { + exprs.push(quote!(defmt::export::fmt_slice(#arg))) + } + defmt_parser::Type::FormatArray(len) => { + exprs.push(quote!(defmt::export::fmt_array({ + let tmp: &[_; #len] = #arg; + tmp + }))) + } - defmt_parser::Type::Debug => exprs.push(quote!(_fmt_.debug(#arg))), - defmt_parser::Type::Display => exprs.push(quote!(_fmt_.display(#arg))), + defmt_parser::Type::Debug => exprs.push(quote!(defmt::export::debug(#arg))), + defmt_parser::Type::Display => exprs.push(quote!(defmt::export::display(#arg))), defmt_parser::Type::FormatSequence => unreachable!(), - defmt_parser::Type::U8Slice => exprs.push(quote!(_fmt_.slice(#arg))), + defmt_parser::Type::U8Slice => exprs.push(quote!(defmt::export::slice(#arg))), // We cast to the expected array type (which should be a no-op cast) to provoke // a type mismatch error on mismatched lengths: // ``` @@ -1163,7 +1170,7 @@ impl Codegen { // | expected an array with a fixed size of 3 elements, found one with 2 elements // | expected due to this // ``` - defmt_parser::Type::U8Array(len) => exprs.push(quote!(_fmt_.u8_array({ + defmt_parser::Type::U8Array(len) => exprs.push(quote!(defmt::export::u8_array({ let tmp: &[u8; #len] = #arg; tmp }))), @@ -1180,11 +1187,11 @@ impl Codegen { // shift away unneeded lower octet // TODO: create helper for shifting because readability match truncated_sz { - 1 => exprs.push(quote!(_fmt_.u8(&defmt::export::truncate((*#arg) >> (#lowest_byte * 8))))), - 2 => exprs.push(quote!(_fmt_.u16(&defmt::export::truncate((*#arg) >> (#lowest_byte * 8))))), - 3..=4 => exprs.push(quote!(_fmt_.u32(&defmt::export::truncate((*#arg) >> (#lowest_byte * 8))))), - 5..=8 => exprs.push(quote!(_fmt_.u64(&defmt::export::truncate((*#arg) >> (#lowest_byte * 8))))), - 9..=16 => exprs.push(quote!(_fmt_.u128(&defmt::export::truncate((*#arg) >> (#lowest_byte * 8))))), + 1 => exprs.push(quote!(defmt::export::u8(&defmt::export::truncate((*#arg) >> (#lowest_byte * 8))))), + 2 => exprs.push(quote!(defmt::export::u16(&defmt::export::truncate((*#arg) >> (#lowest_byte * 8))))), + 3..=4 => exprs.push(quote!(defmt::export::u32(&defmt::export::truncate((*#arg) >> (#lowest_byte * 8))))), + 5..=8 => exprs.push(quote!(defmt::export::u64(&defmt::export::truncate((*#arg) >> (#lowest_byte * 8))))), + 9..=16 => exprs.push(quote!(defmt::export::u128(&defmt::export::truncate((*#arg) >> (#lowest_byte * 8))))), _ => unreachable!(), } } diff --git a/src/export.rs b/src/export.rs index 9a0f8c46..56aa6f2f 100644 --- a/src/export.rs +++ b/src/export.rs @@ -1,4 +1,6 @@ -use crate::Str; +use core::fmt::Write as _; + +use crate::{Format, Formatter, Str}; #[cfg(feature = "unstable-test")] thread_local! { @@ -14,8 +16,8 @@ pub fn fetch_string_index() -> u16 { /// For testing purposes #[cfg(feature = "unstable-test")] -pub fn fetch_add_string_index() -> usize { - (I.with(|i| i.fetch_add(1, core::sync::atomic::Ordering::Relaxed))) as usize +pub fn fetch_add_string_index() -> u16 { + I.with(|i| i.fetch_add(1, core::sync::atomic::Ordering::Relaxed)) } /// Get and clear the logged bytes @@ -75,17 +77,21 @@ pub fn timestamp(fmt: crate::Formatter<'_>) { } /// Returns the interned string at `address`. -pub fn istr(address: usize) -> Str { - Str { - // NOTE address is limited to 14 bits in the linker script - address: address as *const u8 as u16, +pub fn make_istr(address: u16) -> Str { + Str { address } +} + +/// Create a Formatter. +pub fn make_formatter<'a>() -> Formatter<'a> { + Formatter { + _phantom: core::marker::PhantomData, } } mod sealed { #[allow(unused_imports)] use crate as defmt; - use crate::{Format, Formatter}; + use crate::{Format, Formatter, Str}; pub trait Truncate { fn truncate(self) -> U; @@ -193,7 +199,7 @@ mod sealed { unreachable!(); } - fn _format_tag() -> u16 { + fn _format_tag() -> Str { defmt_macros::internp!("Unwrap of a None option value") } @@ -248,3 +254,152 @@ pub fn panic() -> ! { } unsafe { _defmt_panic() } } + +/// Implementation detail +pub fn fmt(f: &T) { + istr(&T::_format_tag()); + let formatter = make_formatter(); + f._format_data(formatter); +} + +/// Implementation detail +pub fn i8(b: &i8) { + write(&b.to_le_bytes()) +} + +/// Implementation detail +pub fn i16(b: &i16) { + write(&b.to_le_bytes()) +} + +/// Implementation detail +pub fn i32(b: &i32) { + write(&b.to_le_bytes()) +} + +/// Implementation detail +pub fn i64(b: &i64) { + write(&b.to_le_bytes()) +} + +/// Implementation detail +pub fn i128(b: &i128) { + write(&b.to_le_bytes()) +} + +/// Implementation detail +pub fn isize(b: &isize) { + write(&(*b as i32).to_le_bytes()) +} + +/// Implementation detail +pub fn fmt_slice(values: &[impl Format]) { + usize(&values.len()); + for value in values { + fmt(value); + } +} + +/// Implementation detail +pub fn u8(b: &u8) { + write(&[*b]) +} + +/// Implementation detail +pub fn u16(b: &u16) { + write(&b.to_le_bytes()) +} + +/// Implementation detail +pub fn u32(b: &u32) { + write(&b.to_le_bytes()) +} + +/// Implementation detail +pub fn u64(b: &u64) { + write(&b.to_le_bytes()) +} + +/// Implementation detail +pub fn u128(b: &u128) { + write(&b.to_le_bytes()) +} + +/// Implementation detail +pub fn usize(b: &usize) { + write(&(*b as u32).to_le_bytes()) +} + +/// Implementation detail +pub fn f32(b: &f32) { + write(&f32::to_bits(*b).to_le_bytes()) +} + +/// Implementation detail +pub fn f64(b: &f64) { + write(&f64::to_bits(*b).to_le_bytes()) +} + +/// Implementation detail +pub fn char(b: &char) { + write(&(*b as u32).to_le_bytes()) +} + +pub fn str(s: &str) { + usize(&s.len()); + write(s.as_bytes()); +} + +pub fn slice(s: &[u8]) { + usize(&s.len()); + write(s); +} + +// NOTE: This is passed `&[u8; N]` – it's just coerced to a slice. +pub fn u8_array(a: &[u8]) { + write(a); +} + +// NOTE: This is passed `&[u8; N]` – it's just coerced to a slice. +pub fn fmt_array(a: &[impl Format]) { + for value in a { + fmt(value); + } +} + +/// Implementation detail +pub fn istr(s: &Str) { + write(&s.address.to_le_bytes()) +} + +/// Implementation detail +pub fn bool(b: &bool) { + u8(&(*b as u8)); +} + +/// Implementation detail +pub fn debug(val: &dyn core::fmt::Debug) { + core::write!(FmtWrite, "{:?}", val).ok(); + write(&[0xff]); +} + +/// Implementation detail +pub fn display(val: &dyn core::fmt::Display) { + core::write!(FmtWrite, "{}", val).ok(); + write(&[0xff]); +} + +#[inline(never)] +pub fn header(s: &Str) { + istr(s); + timestamp(make_formatter()); +} + +struct FmtWrite; + +impl core::fmt::Write for FmtWrite { + fn write_str(&mut self, s: &str) -> core::fmt::Result { + write(s.as_bytes()); + Ok(()) + } +} diff --git a/src/formatter.rs b/src/formatter.rs index a4cf7b84..a2aa66bc 100644 --- a/src/formatter.rs +++ b/src/formatter.rs @@ -1,184 +1,8 @@ -use core::fmt::{self, Write as _}; - -use crate::{export, Format}; +use core::marker::PhantomData; /// Handle to a defmt logger. pub struct Formatter<'a> { - /// Keep the formatter alive - #[doc(hidden)] - pub inner: &'a mut InternalFormatter, -} - -#[doc(hidden)] -pub struct InternalFormatter {} - -#[doc(hidden)] -impl InternalFormatter { - pub fn write(&mut self, bytes: &[u8]) { - export::write(bytes) - } - - /// Implementation detail - /// - /// # Safety - /// - /// Must only be called when the current execution context has acquired the logger with [Logger::acquire] - #[allow(clippy::new_without_default)] - pub fn new() -> Self { - Self {} - } - - // TODO turn these public methods in `export` free functions - /// Implementation detail - pub fn fmt(&mut self, f: &T) { - self.tag(T::_format_tag()); - let formatter = Formatter { inner: self }; - f._format_data(formatter); - } - - /// Implementation detail - pub fn tag(&mut self, b: u16) { - self.write(&b.to_le_bytes()) - } - - /// Implementation detail - pub fn i8(&mut self, b: &i8) { - self.write(&b.to_le_bytes()) - } - - /// Implementation detail - pub fn i16(&mut self, b: &i16) { - self.write(&b.to_le_bytes()) - } - - /// Implementation detail - pub fn i32(&mut self, b: &i32) { - self.write(&b.to_le_bytes()) - } - - /// Implementation detail - pub fn i64(&mut self, b: &i64) { - self.write(&b.to_le_bytes()) - } - - /// Implementation detail - pub fn i128(&mut self, b: &i128) { - self.write(&b.to_le_bytes()) - } - - /// Implementation detail - pub fn isize(&mut self, b: &isize) { - self.write(&(*b as i32).to_le_bytes()) - } - - /// Implementation detail - pub fn fmt_slice(&mut self, values: &[impl Format]) { - self.usize(&values.len()); - for value in values { - self.fmt(value); - } - } - - // TODO remove - /// Implementation detail - pub fn prim(&mut self, s: &Str) { - self.write(&[s.address as u8]) - } - - /// Implementation detail - pub fn u8(&mut self, b: &u8) { - self.write(&[*b]) - } - - /// Implementation detail - pub fn u16(&mut self, b: &u16) { - self.write(&b.to_le_bytes()) - } - - /// Implementation detail - pub fn u24(&mut self, b: &u32) { - self.write(&b.to_le_bytes()[..3]) - } - - /// Implementation detail - pub fn u32(&mut self, b: &u32) { - self.write(&b.to_le_bytes()) - } - - /// Implementation detail - pub fn u64(&mut self, b: &u64) { - self.write(&b.to_le_bytes()) - } - - /// Implementation detail - pub fn u128(&mut self, b: &u128) { - self.write(&b.to_le_bytes()) - } - - /// Implementation detail - pub fn usize(&mut self, b: &usize) { - self.write(&(*b as u32).to_le_bytes()) - } - - /// Implementation detail - pub fn f32(&mut self, b: &f32) { - self.write(&f32::to_bits(*b).to_le_bytes()) - } - - /// Implementation detail - pub fn f64(&mut self, b: &f64) { - self.write(&f64::to_bits(*b).to_le_bytes()) - } - - pub fn str(&mut self, s: &str) { - self.usize(&s.len()); - self.write(s.as_bytes()); - } - - pub fn slice(&mut self, s: &[u8]) { - self.usize(&s.len()); - self.write(s); - } - - // NOTE: This is passed `&[u8; N]` – it's just coerced to a slice. - pub fn u8_array(&mut self, a: &[u8]) { - self.write(a); - } - - // NOTE: This is passed `&[u8; N]` – it's just coerced to a slice. - pub fn fmt_array(&mut self, a: &[impl Format]) { - for value in a { - self.fmt(value); - } - } - - /// Implementation detail - pub fn istr(&mut self, s: &Str) { - self.write(&s.address.to_le_bytes()) - } - - /// Implementation detail - pub fn bool(&mut self, b: &bool) { - self.u8(&(*b as u8)); - } - - /// Implementation detail - pub fn debug(&mut self, val: &dyn core::fmt::Debug) { - core::write!(FmtWrite { fmt: self }, "{:?}", val).ok(); - self.write(&[0xff]); - } - - /// Implementation detail - pub fn display(&mut self, val: &dyn core::fmt::Display) { - core::write!(FmtWrite { fmt: self }, "{}", val).ok(); - self.write(&[0xff]); - } - - #[inline(never)] - pub fn header(&mut self, s: &Str) { - self.istr(s); - export::timestamp(Formatter { inner: self }); - } + pub(crate) _phantom: PhantomData<&'a ()>, } /// An interned string created via [`intern!`]. @@ -189,14 +13,3 @@ pub struct Str { /// 14-bit address pub(crate) address: u16, } - -struct FmtWrite<'a> { - fmt: &'a mut InternalFormatter, -} - -impl fmt::Write for FmtWrite<'_> { - fn write_str(&mut self, s: &str) -> fmt::Result { - self.fmt.write(s.as_bytes()); - Ok(()) - } -} diff --git a/src/impls/adapter.rs b/src/impls/adapter.rs index 70676481..f59087e4 100644 --- a/src/impls/adapter.rs +++ b/src/impls/adapter.rs @@ -1,8 +1,7 @@ use core::fmt; -#[cfg(feature = "unstable-test")] use crate as defmt; -use crate::{Format, Formatter}; +use crate::{export, Format, Formatter, Str}; /// An "adapter" type to feed `Debug` values into defmt macros, which expect `defmt::Format` values. /// @@ -28,12 +27,12 @@ pub struct Debug2Format<'a, T: fmt::Debug + ?Sized>(pub &'a T); impl Format for Debug2Format<'_, T> { default_format!(); - fn _format_tag() -> u16 { + fn _format_tag() -> Str { defmt_macros::internp!("{=__internal_Debug}") } - fn _format_data(&self, fmt: Formatter) { - fmt.inner.debug(&self.0); + fn _format_data(&self, _fmt: Formatter) { + export::debug(&self.0); } } @@ -66,11 +65,11 @@ pub struct Display2Format<'a, T: fmt::Display + ?Sized>(pub &'a T); impl Format for Display2Format<'_, T> { default_format!(); - fn _format_tag() -> u16 { + fn _format_tag() -> Str { defmt_macros::internp!("{=__internal_Display}") } - fn _format_data(&self, fmt: Formatter) { - fmt.inner.display(&self.0); + fn _format_data(&self, _fmt: Formatter) { + export::display(&self.0); } } diff --git a/src/impls/arrays.rs b/src/impls/arrays.rs index d4961fec..fb1a82e2 100644 --- a/src/impls/arrays.rs +++ b/src/impls/arrays.rs @@ -1,4 +1,5 @@ use super::*; +use crate::export; macro_rules! arrays { ( $($len:literal $fmt:literal,)+ ) => { $( @@ -9,13 +10,13 @@ macro_rules! arrays { default_format!(); #[inline] - fn _format_tag() -> u16 { + fn _format_tag() -> Str { internp!($fmt) } #[inline] - fn _format_data(&self, fmt: Formatter) { - fmt.inner.fmt_array(self); + fn _format_data(&self, _fmt: Formatter) { + export::fmt_array(self); } } )+ }; diff --git a/src/impls/core_/mod.rs b/src/impls/core_/mod.rs index 3289d3e7..6b47c3e1 100644 --- a/src/impls/core_/mod.rs +++ b/src/impls/core_/mod.rs @@ -10,6 +10,7 @@ mod ops; mod slice; use super::*; +use crate::export; impl Format for Option where @@ -18,17 +19,17 @@ where default_format!(); #[inline] - fn _format_tag() -> u16 { + fn _format_tag() -> Str { internp!("None|Some({=?})") } #[inline] fn _format_data(&self, fmt: Formatter) { match self { - None => fmt.inner.u8(&0), + None => export::u8(&0), Some(x) => { - fmt.inner.u8(&1); - fmt.inner.tag(T::_format_tag()); + export::u8(&1); + export::istr(&T::_format_tag()); x._format_data(fmt) } } @@ -43,7 +44,7 @@ where default_format!(); #[inline] - fn _format_tag() -> u16 { + fn _format_tag() -> Str { internp!("Err({=?})|Ok({=?})") } @@ -51,13 +52,13 @@ where fn _format_data(&self, fmt: Formatter) { match self { Err(e) => { - fmt.inner.u8(&0); - fmt.inner.tag(E::_format_tag()); + export::u8(&0); + export::istr(&E::_format_tag()); e._format_data(fmt) } Ok(x) => { - fmt.inner.u8(&1); - fmt.inner.tag(T::_format_tag()); + export::u8(&1); + export::istr(&T::_format_tag()); x._format_data(fmt) } } @@ -68,7 +69,7 @@ impl Format for core::marker::PhantomData { default_format!(); #[inline] - fn _format_tag() -> u16 { + fn _format_tag() -> Str { internp!("PhantomData") } @@ -80,7 +81,7 @@ impl Format for core::convert::Infallible { default_format!(); #[inline] - fn _format_tag() -> u16 { + fn _format_tag() -> Str { unreachable!(); } diff --git a/src/impls/mod.rs b/src/impls/mod.rs index d3d78a8e..dfe504be 100644 --- a/src/impls/mod.rs +++ b/src/impls/mod.rs @@ -2,7 +2,7 @@ macro_rules! default_format { () => { #[inline] fn format(&self, fmt: Formatter) { - fmt.inner.tag(Self::_format_tag()); + crate::export::istr(&Self::_format_tag()); self._format_data(fmt); } }; @@ -16,7 +16,7 @@ macro_rules! delegate_format { } #[inline] - fn _format_tag() -> u16 { + fn _format_tag() -> Str { <$ty as Format>::_format_tag() } diff --git a/src/impls/primitives.rs b/src/impls/primitives.rs index ffddc640..2da264fc 100644 --- a/src/impls/primitives.rs +++ b/src/impls/primitives.rs @@ -1,54 +1,43 @@ +use crate::export; + use super::*; macro_rules! prim { - ($ty:ty, $method:ident, $fmt: literal) => { + ($ty:ty, $fmt: literal, $self_:ident, $write:expr) => { impl Format for $ty { default_format!(); #[inline] - fn _format_tag() -> u16 { + fn _format_tag() -> Str { internp!($fmt) } #[inline] - fn _format_data(&self, fmt: Formatter) { - fmt.inner.$method(self); + fn _format_data(&$self_, _fmt: Formatter) { + $write } } }; } -prim!(i8, i8, "{=i8}"); -prim!(i16, i16, "{=i16}"); -prim!(i32, i32, "{=i32}"); -prim!(i64, i64, "{=i64}"); -prim!(i128, i128, "{=i128}"); -prim!(isize, isize, "{=isize}"); -prim!(u8, u8, "{=u8}"); -prim!(u16, u16, "{=u16}"); -prim!(u32, u32, "{=u32}"); -prim!(u64, u64, "{=u64}"); -prim!(u128, u128, "{=u128}"); -prim!(usize, usize, "{=usize}"); -prim!(f32, f32, "{=f32}"); -prim!(f64, f64, "{=f64}"); -prim!(str, str, "{=str}"); -prim!(bool, bool, "{=bool}"); -prim!(Str, istr, "{=istr}"); - -impl Format for char { - default_format!(); - - #[inline] - fn _format_tag() -> u16 { - internp!("{=char}") - } - - #[inline] - fn _format_data(&self, fmt: Formatter) { - fmt.inner.u32(&(*self as u32)); - } -} +prim!(i8, "{=i8}", self, export::i8(self)); +prim!(i16, "{=i16}", self, export::i16(self)); +prim!(i32, "{=i32}", self, export::i32(self)); +prim!(i64, "{=i64}", self, export::i64(self)); +prim!(i128, "{=i128}", self, export::i128(self)); +prim!(isize, "{=isize}", self, export::isize(self)); +prim!(u8, "{=u8}", self, export::u8(self)); +prim!(u16, "{=u16}", self, export::u16(self)); +prim!(u32, "{=u32}", self, export::u32(self)); +prim!(u64, "{=u64}", self, export::u64(self)); +prim!(u128, "{=u128}", self, export::u128(self)); +prim!(usize, "{=usize}", self, export::usize(self)); +prim!(f32, "{=f32}", self, export::f32(self)); +prim!(f64, "{=f64}", self, export::f64(self)); +prim!(str, "{=str}", self, export::str(self)); +prim!(bool, "{=bool}", self, export::bool(self)); +prim!(Str, "{=istr}", self, export::istr(self)); +prim!(char, "{=char}", self, export::char(self)); impl Format for [T] where @@ -57,16 +46,16 @@ where default_format!(); #[inline] - fn _format_tag() -> u16 { + fn _format_tag() -> Str { internp!("{=[?]}") } #[inline] - fn _format_data(&self, fmt: Formatter) { - fmt.inner.usize(&self.len()); + fn _format_data(&self, _fmt: Formatter) { + export::usize(&self.len()); for value in self { - fmt.inner.tag(T::_format_tag()); - value._format_data(Formatter { inner: fmt.inner }); + export::istr(&T::_format_tag()); + value._format_data(export::make_formatter()); } } } diff --git a/src/impls/tuples.rs b/src/impls/tuples.rs index 7b428a64..d1cdda75 100644 --- a/src/impls/tuples.rs +++ b/src/impls/tuples.rs @@ -1,10 +1,11 @@ use super::*; +use crate::export; impl Format for () { default_format!(); #[inline] - fn _format_tag() -> u16 { + fn _format_tag() -> Str { internp!("()") } @@ -18,17 +19,17 @@ macro_rules! tuple { default_format!(); #[inline] - fn _format_tag() -> u16 { + fn _format_tag() -> Str { internp!($format) } #[inline] #[allow(non_snake_case, unused_assignments)] - fn _format_data(&self, fmt: Formatter) { + fn _format_data(&self, _fmt: Formatter) { let ($(ref $name,)+) = *self; $( - fmt.inner.tag($name::_format_tag()); - let formatter = Formatter { inner: fmt.inner }; + export::istr(&$name::_format_tag()); + let formatter = export::make_formatter(); $name._format_data(formatter); )+ } diff --git a/src/lib.rs b/src/lib.rs index 8ab010df..44e18e76 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,7 +27,7 @@ mod tests; mod traits; pub use crate::{ - formatter::{Formatter, InternalFormatter, Str}, + formatter::{Formatter, Str}, impls::adapter::{Debug2Format, Display2Format}, traits::{Format, Logger}, }; diff --git a/src/traits.rs b/src/traits.rs index 2b684e16..b3317f0c 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -2,7 +2,7 @@ use defmt_macros::internp; #[allow(unused_imports)] use crate as defmt; -use crate::Formatter; +use crate::{export, Formatter, Str}; /// Trait for types that can be formatted via defmt. /// @@ -49,14 +49,14 @@ pub trait Format { fn format(&self, fmt: Formatter); #[doc(hidden)] - fn _format_tag() -> u16 { + fn _format_tag() -> Str { internp!("{=__internal_FormatSequence}") } #[doc(hidden)] - fn _format_data(&self, fmt: Formatter) { - self.format(Formatter { inner: fmt.inner }); - fmt.inner.tag(0); // terminator + fn _format_data(&self, _fmt: Formatter) { + self.format(export::make_formatter()); + export::u16(&0); // terminator } } diff --git a/tests/encode.rs b/tests/encode.rs index bcf5167c..fd29d914 100644 --- a/tests/encode.rs +++ b/tests/encode.rs @@ -18,12 +18,12 @@ // when writing the expected output of a unit test. // // ``` -// let mut f = InternalFormatter::new(); +// let mut f = Internalexport::make_formatter(); // let index = defmt::export::fetch_string_index(); // foo(&mut f); // NOTE increases the interner index // assert_eq!(fetch_bytes(), [index]); // -// let mut f = InternalFormatter::new(); +// let mut f = Internalexport::make_formatter(); // foo(&mut f); // assert_eq!(fetch_bytes(), [index.wrapping_add(1)]); // ^^^^^^^^^^^^^^^ account for the previous `foo` call @@ -36,10 +36,7 @@ // // - the mocked index is 7 bits so its LEB128 encoding is the input byte -use defmt::{ - export::fetch_string_index, write, Debug2Format, Display2Format, Format, Formatter, - InternalFormatter, -}; +use defmt::{export::fetch_string_index, write, Debug2Format, Display2Format, Format, Formatter}; // Increase the 7-bit mocked interned index fn inc(index: u16, n: u16) -> u16 { @@ -47,9 +44,8 @@ fn inc(index: u16, n: u16) -> u16 { } fn check_format_implementation(val: &T, expected_encoding: &[u8]) { - let mut f = InternalFormatter::new(); - f.tag(T::_format_tag()); - let g = Formatter { inner: &mut f }; + defmt::export::istr(&T::_format_tag()); + let g = defmt::export::make_formatter(); val._format_data(g); assert_eq!(defmt::export::fetch_bytes(), expected_encoding); } @@ -81,18 +77,15 @@ macro_rules! check_format { #[test] fn write() { let index = fetch_string_index(); - let f = &mut InternalFormatter::new(); - let g = Formatter { inner: f }; - + let g = defmt::export::make_formatter(); write!(g, "The answer is {=u8}", 42); check!([ index, // "The answer is {=u8}", 42u8, // u8 value ]); - let f2 = &mut InternalFormatter::new(); - let g2 = Formatter { inner: f2 }; - write!(g2, "The answer is {=?}", 42u8); + let g = defmt::export::make_formatter(); + write!(g, "The answer is {=?}", 42u8); check!([ inc(index, 1), // "The answer is {=?}" inc(index, 2), // "{=u8}" / impl Format for u8 @@ -103,8 +96,7 @@ fn write() { #[test] fn bitfields_mixed() { let index = fetch_string_index(); - let f = &mut InternalFormatter::new(); - let g = Formatter { inner: f }; + let g = defmt::export::make_formatter(); write!( g, @@ -122,8 +114,7 @@ fn bitfields_mixed() { #[test] fn bitfields_across_octets() { let index = fetch_string_index(); - let f = &mut InternalFormatter::new(); - let g = Formatter { inner: f }; + let g = defmt::export::make_formatter(); write!(g, "bitfields {0=0..7} {0=9..14}", 0b0110_0011_1101_0010u16); check!([ @@ -136,8 +127,7 @@ fn bitfields_across_octets() { #[test] fn bitfields_truncate_lower() { let index = fetch_string_index(); - let f = &mut InternalFormatter::new(); - let g = Formatter { inner: f }; + let g = defmt::export::make_formatter(); write!( g, @@ -153,8 +143,7 @@ fn bitfields_truncate_lower() { #[test] fn bitfields_assert_range_exclusive() { let index = fetch_string_index(); - let f = &mut InternalFormatter::new(); - let g = Formatter { inner: f }; + let g = defmt::export::make_formatter(); write!(g, "bitfields {0=6..8}", 0b1010_0101u8,); check!([ From 8e8bbe0f8ca49c00a576c97a9c9d3903596383e3 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 14 Jun 2021 19:45:01 +0200 Subject: [PATCH 4/9] Remove Formatter arg from _format_data --- macros/src/lib.rs | 2 +- src/export.rs | 5 ++--- src/impls/adapter.rs | 4 ++-- src/impls/arrays.rs | 2 +- src/impls/core_/mod.rs | 14 +++++++------- src/impls/mod.rs | 8 ++++---- src/impls/primitives.rs | 6 +++--- src/impls/tuples.rs | 7 +++---- src/traits.rs | 2 +- 9 files changed, 24 insertions(+), 26 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 45fbc3d6..2c7503fa 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -345,7 +345,7 @@ pub fn format(ts: TokenStream) -> TokenStream { fn _format_tag() -> defmt::Str { #sym } - fn _format_data(&self, f: defmt::Formatter) { + fn _format_data(&self) { #(#exprs)* } } diff --git a/src/export.rs b/src/export.rs index 56aa6f2f..ee672dd3 100644 --- a/src/export.rs +++ b/src/export.rs @@ -203,7 +203,7 @@ mod sealed { defmt_macros::internp!("Unwrap of a None option value") } - fn _format_data(&self, _fmt: Formatter) {} + fn _format_data(&self) {} } pub trait IntoResult { @@ -258,8 +258,7 @@ pub fn panic() -> ! { /// Implementation detail pub fn fmt(f: &T) { istr(&T::_format_tag()); - let formatter = make_formatter(); - f._format_data(formatter); + f._format_data(); } /// Implementation detail diff --git a/src/impls/adapter.rs b/src/impls/adapter.rs index f59087e4..4d2e0ad4 100644 --- a/src/impls/adapter.rs +++ b/src/impls/adapter.rs @@ -31,7 +31,7 @@ impl Format for Debug2Format<'_, T> { defmt_macros::internp!("{=__internal_Debug}") } - fn _format_data(&self, _fmt: Formatter) { + fn _format_data(&self) { export::debug(&self.0); } } @@ -69,7 +69,7 @@ impl Format for Display2Format<'_, T> { defmt_macros::internp!("{=__internal_Display}") } - fn _format_data(&self, _fmt: Formatter) { + fn _format_data(&self) { export::display(&self.0); } } diff --git a/src/impls/arrays.rs b/src/impls/arrays.rs index fb1a82e2..50370a7a 100644 --- a/src/impls/arrays.rs +++ b/src/impls/arrays.rs @@ -15,7 +15,7 @@ macro_rules! arrays { } #[inline] - fn _format_data(&self, _fmt: Formatter) { + fn _format_data(&self) { export::fmt_array(self); } } diff --git a/src/impls/core_/mod.rs b/src/impls/core_/mod.rs index 6b47c3e1..37c09306 100644 --- a/src/impls/core_/mod.rs +++ b/src/impls/core_/mod.rs @@ -24,13 +24,13 @@ where } #[inline] - fn _format_data(&self, fmt: Formatter) { + fn _format_data(&self) { match self { None => export::u8(&0), Some(x) => { export::u8(&1); export::istr(&T::_format_tag()); - x._format_data(fmt) + x._format_data() } } } @@ -49,17 +49,17 @@ where } #[inline] - fn _format_data(&self, fmt: Formatter) { + fn _format_data(&self) { match self { Err(e) => { export::u8(&0); export::istr(&E::_format_tag()); - e._format_data(fmt) + e._format_data() } Ok(x) => { export::u8(&1); export::istr(&T::_format_tag()); - x._format_data(fmt) + x._format_data() } } } @@ -74,7 +74,7 @@ impl Format for core::marker::PhantomData { } #[inline] - fn _format_data(&self, _fmt: Formatter) {} + fn _format_data(&self) {} } impl Format for core::convert::Infallible { @@ -86,7 +86,7 @@ impl Format for core::convert::Infallible { } #[inline] - fn _format_data(&self, _fmt: Formatter) { + fn _format_data(&self) { unreachable!(); } } diff --git a/src/impls/mod.rs b/src/impls/mod.rs index dfe504be..f0e4a205 100644 --- a/src/impls/mod.rs +++ b/src/impls/mod.rs @@ -1,9 +1,9 @@ macro_rules! default_format { () => { #[inline] - fn format(&self, fmt: Formatter) { + fn format(&self, _fmt: Formatter) { crate::export::istr(&Self::_format_tag()); - self._format_data(fmt); + self._format_data(); } }; } @@ -21,8 +21,8 @@ macro_rules! delegate_format { } #[inline] - fn _format_data(&$self_, fmt: Formatter) { - <$ty as Format>::_format_data($val, fmt) + fn _format_data(&$self_) { + <$ty as Format>::_format_data($val) } }; } diff --git a/src/impls/primitives.rs b/src/impls/primitives.rs index 2da264fc..8a643e5e 100644 --- a/src/impls/primitives.rs +++ b/src/impls/primitives.rs @@ -13,7 +13,7 @@ macro_rules! prim { } #[inline] - fn _format_data(&$self_, _fmt: Formatter) { + fn _format_data(&$self_) { $write } } @@ -51,11 +51,11 @@ where } #[inline] - fn _format_data(&self, _fmt: Formatter) { + fn _format_data(&self) { export::usize(&self.len()); for value in self { export::istr(&T::_format_tag()); - value._format_data(export::make_formatter()); + value._format_data(); } } } diff --git a/src/impls/tuples.rs b/src/impls/tuples.rs index d1cdda75..a979fc42 100644 --- a/src/impls/tuples.rs +++ b/src/impls/tuples.rs @@ -10,7 +10,7 @@ impl Format for () { } #[inline] - fn _format_data(&self, _fmt: Formatter) {} + fn _format_data(&self) {} } macro_rules! tuple { @@ -25,12 +25,11 @@ macro_rules! tuple { #[inline] #[allow(non_snake_case, unused_assignments)] - fn _format_data(&self, _fmt: Formatter) { + fn _format_data(&self) { let ($(ref $name,)+) = *self; $( export::istr(&$name::_format_tag()); - let formatter = export::make_formatter(); - $name._format_data(formatter); + $name._format_data(); )+ } } diff --git a/src/traits.rs b/src/traits.rs index b3317f0c..0cec3679 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -54,7 +54,7 @@ pub trait Format { } #[doc(hidden)] - fn _format_data(&self, _fmt: Formatter) { + fn _format_data(&self) { self.format(export::make_formatter()); export::u16(&0); // terminator } From a33a9f51bb6f210b5167afc437709af3e703d23a Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 14 Jun 2021 19:59:17 +0200 Subject: [PATCH 5/9] Implement tag omission optimization --- decoder/src/decoder.rs | 7 ++----- src/export.rs | 10 ++++++---- src/impls/primitives.rs | 6 +----- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/decoder/src/decoder.rs b/decoder/src/decoder.rs index 9f363a2d..d891d57d 100644 --- a/decoder/src/decoder.rs +++ b/decoder/src/decoder.rs @@ -73,14 +73,11 @@ impl<'t, 'b> Decoder<'t, 'b> { &mut self, num_elements: usize, ) -> Result>, DecodeError> { - if num_elements == 0 { - return Ok(vec![]); - } + let format = self.get_format()?; + let is_enum = format.contains('|'); let mut elements = Vec::with_capacity(num_elements); for i in 0..num_elements { - let format = self.get_format()?; - let is_enum = format.contains('|'); let format = if is_enum { self.get_variant(format)? } else { diff --git a/src/export.rs b/src/export.rs index ee672dd3..5aaeccfe 100644 --- a/src/export.rs +++ b/src/export.rs @@ -292,10 +292,11 @@ pub fn isize(b: &isize) { } /// Implementation detail -pub fn fmt_slice(values: &[impl Format]) { +pub fn fmt_slice(values: &[T]) { usize(&values.len()); + istr(&T::_format_tag()); for value in values { - fmt(value); + value._format_data(); } } @@ -360,9 +361,10 @@ pub fn u8_array(a: &[u8]) { } // NOTE: This is passed `&[u8; N]` – it's just coerced to a slice. -pub fn fmt_array(a: &[impl Format]) { +pub fn fmt_array(a: &[T]) { + istr(&T::_format_tag()); for value in a { - fmt(value); + value._format_data(); } } diff --git a/src/impls/primitives.rs b/src/impls/primitives.rs index 8a643e5e..09006763 100644 --- a/src/impls/primitives.rs +++ b/src/impls/primitives.rs @@ -52,11 +52,7 @@ where #[inline] fn _format_data(&self) { - export::usize(&self.len()); - for value in self { - export::istr(&T::_format_tag()); - value._format_data(); - } + export::fmt_slice(self); } } From 1cdffed68cee4beefb2ccf8fbebf00837effa389 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 30 Jun 2021 05:48:05 +0200 Subject: [PATCH 6/9] Allow multiple `write!` calls in a Format impl. --- decoder/src/decoder.rs | 6 ++- decoder/src/frame.rs | 5 ++ decoder/src/lib.rs | 29 +++++----- firmware/qemu/src/bin/log.out | 76 ++++++++++++++------------- firmware/qemu/src/bin/log.release.out | 76 ++++++++++++++------------- firmware/qemu/src/bin/log.rs | 22 ++++++++ src/formatter.rs | 1 + 7 files changed, 127 insertions(+), 88 deletions(-) diff --git a/decoder/src/decoder.rs b/decoder/src/decoder.rs index d891d57d..8021784d 100644 --- a/decoder/src/decoder.rs +++ b/decoder/src/decoder.rs @@ -236,6 +236,7 @@ impl<'t, 'b> Decoder<'t, 'b> { args.push(Arg::Preformatted(data.into())); } Type::FormatSequence => { + let mut seq_args = Vec::new(); loop { let index = self.bytes.read_u16::()? as usize; if index == 0 { @@ -251,18 +252,19 @@ impl<'t, 'b> Decoder<'t, 'b> { // enum let variant = self.get_variant(format)?; let inner_args = self.decode_format(variant)?; - args.push(Arg::Format { + seq_args.push(Arg::Format { format: variant, args: inner_args, }); } else { let inner_args = self.decode_format(format)?; - args.push(Arg::Format { + seq_args.push(Arg::Format { format, args: inner_args, }); } } + args.push(Arg::FormatSequence { args: seq_args }) } } } diff --git a/decoder/src/frame.rs b/decoder/src/frame.rs index aad1bf11..28364d78 100644 --- a/decoder/src/frame.rs +++ b/decoder/src/frame.rs @@ -294,6 +294,11 @@ fn format_args_real( 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::FormatSequence { args } => { + for arg in args { + buf.push_str(&format_args("{=?}", &[arg.clone()], hint)) + } + } Arg::FormatSlice { elements } => { match hint { // Filter Ascii Hints, which contains u8 byte slices diff --git a/decoder/src/lib.rs b/decoder/src/lib.rs index 8a3e268e..9be7a729 100644 --- a/decoder/src/lib.rs +++ b/decoder/src/lib.rs @@ -207,7 +207,7 @@ impl Table { } // NOTE follows `parser::Type` -#[derive(Debug, PartialEq)] +#[derive(Debug, Clone, PartialEq)] enum Arg<'t> { /// Bool Bool(bool), @@ -229,6 +229,9 @@ enum Arg<'t> { FormatSlice { elements: Vec>, }, + FormatSequence { + args: Vec>, + }, /// Slice or Array of bytes. Slice(Vec), /// Char @@ -238,7 +241,7 @@ enum Arg<'t> { Preformatted(String), } -#[derive(Debug, PartialEq)] +#[derive(Debug, Clone, PartialEq)] struct FormatSliceElement<'t> { // this will usually be the same format string for all elements; except when the format string // is an enum -- in that case `format` will be the variant @@ -553,16 +556,18 @@ mod tests { None, vec![], "{=__internal_FormatSequence}", - vec![ - Arg::Format { - format: "Foo", - args: vec![] - }, - Arg::Format { - format: "Bar({=u8})", - args: vec![Arg::Uxx(42)] - } - ], + vec![Arg::FormatSequence { + args: vec![ + Arg::Format { + format: "Foo", + args: vec![] + }, + Arg::Format { + format: "Bar({=u8})", + args: vec![Arg::Uxx(42)] + } + ] + }], ), bytes.len(), )) diff --git a/firmware/qemu/src/bin/log.out b/firmware/qemu/src/bin/log.out index 12e0b6af..130270da 100644 --- a/firmware/qemu/src/bin/log.out +++ b/firmware/qemu/src/bin/log.out @@ -91,42 +91,42 @@ 0.000090 INFO (A(true), B(true)), (A(false), B(true)), (A(true), B(false)) 0.000091 INFO true, [1, 2]: DhcpReprMin { broadcast: true, a: [1, 2] } 0.000092 INFO nested `Format` impls using `write!`: outer value (inner value (42)) -0.000093 INFO S { x: -1, y: 2 } -0.000094 INFO Some(S { x: -1, y: 2 }) -0.000095 INFO [S { x: -1, y: 2 }, S { x: -1, y: 2 }] -0.000096 INFO [Some(S { x: -1, y: 2 }), None] -0.000097 INFO 127.0.0.1:8888 -0.000098 INFO i128: 0 = 0, -1 = -1, MAX = 170141183460469231731687303715884105727, MIN = -170141183460469231731687303715884105728 -0.000099 INFO u128: 0 = 0, -1 = 1, MAX = 340282366920938463463374607431768211455, MIN = 0 -0.000100 INFO 340282366920938 -0.000101 INFO -170141183460469 -0.000102 INFO Hello 💜 -0.000103 INFO Hello 💜 & 🍕 -0.000104 INFO EnumLarge::A051 -0.000105 INFO EnumLarge::A269 -0.000106 INFO S { x: "hi" } -0.000107 INFO S { x: PhantomData, y: 42 } -0.000108 INFO bitfields 97 10000100 12 b"42" b"hello" -0.000109 INFO b"Hi" -0.000110 INFO b"Hi" +0.000093 INFO manual `Format` impl with multiple `write!`: MyMultiStruct@0 IS ZERO +0.000094 INFO manual `Format` impl with multiple `write!`: MyMultiStruct@20 IS NOT ZERO, division result: 5 +0.000095 INFO S { x: -1, y: 2 } +0.000096 INFO Some(S { x: -1, y: 2 }) +0.000097 INFO [S { x: -1, y: 2 }, S { x: -1, y: 2 }] +0.000098 INFO [Some(S { x: -1, y: 2 }), None] +0.000099 INFO 127.0.0.1:8888 +0.000100 INFO i128: 0 = 0, -1 = -1, MAX = 170141183460469231731687303715884105727, MIN = -170141183460469231731687303715884105728 +0.000101 INFO u128: 0 = 0, -1 = 1, MAX = 340282366920938463463374607431768211455, MIN = 0 +0.000102 INFO 340282366920938 +0.000103 INFO -170141183460469 +0.000104 INFO Hello 💜 +0.000105 INFO Hello 💜 & 🍕 +0.000106 INFO EnumLarge::A051 +0.000107 INFO EnumLarge::A269 +0.000108 INFO S { x: "hi" } +0.000109 INFO S { x: PhantomData, y: 42 } +0.000110 INFO bitfields 97 10000100 12 b"42" b"hello" 0.000111 INFO b"Hi" -0.000112 INFO [45054, 49406] -0.000113 INFO [Data { name: b"Hi", value: true }] -0.000114 INFO true true -0.000115 INFO aabbccdd -0.000116 INFO ddccbbaa -0.000117 INFO 1..2 -0.000118 INFO 1.. -0.000119 INFO ..2 -0.000120 INFO .. -0.000121 INFO 1..=2 -0.000122 INFO ..=2 -0.000123 INFO Zip(..) -0.000124 INFO ChunksExact(..) -0.000125 INFO Iter { slice: [0, 1, 2], position: ? } -0.000126 INFO Windows(..) -0.000127 INFO 1 -0.000128 INFO 1 +0.000112 INFO b"Hi" +0.000113 INFO b"Hi" +0.000114 INFO [45054, 49406] +0.000115 INFO [Data { name: b"Hi", value: true }] +0.000116 INFO true true +0.000117 INFO aabbccdd +0.000118 INFO ddccbbaa +0.000119 INFO 1..2 +0.000120 INFO 1.. +0.000121 INFO ..2 +0.000122 INFO .. +0.000123 INFO 1..=2 +0.000124 INFO ..=2 +0.000125 INFO Zip(..) +0.000126 INFO ChunksExact(..) +0.000127 INFO Iter { slice: [0, 1, 2], position: ? } +0.000128 INFO Windows(..) 0.000129 INFO 1 0.000130 INFO 1 0.000131 INFO 1 @@ -137,5 +137,7 @@ 0.000136 INFO 1 0.000137 INFO 1 0.000138 INFO 1 -0.000139 INFO ccbbaadd -0.000140 INFO QEMU test finished! +0.000139 INFO 1 +0.000140 INFO 1 +0.000141 INFO ccbbaadd +0.000142 INFO QEMU test finished! diff --git a/firmware/qemu/src/bin/log.release.out b/firmware/qemu/src/bin/log.release.out index 7b531516..7ac523fe 100644 --- a/firmware/qemu/src/bin/log.release.out +++ b/firmware/qemu/src/bin/log.release.out @@ -89,42 +89,42 @@ 0.000088 INFO (A(true), B(true)), (A(false), B(true)), (A(true), B(false)) 0.000089 INFO true, [1, 2]: DhcpReprMin { broadcast: true, a: [1, 2] } 0.000090 INFO nested `Format` impls using `write!`: outer value (inner value (42)) -0.000091 INFO S { x: -1, y: 2 } -0.000092 INFO Some(S { x: -1, y: 2 }) -0.000093 INFO [S { x: -1, y: 2 }, S { x: -1, y: 2 }] -0.000094 INFO [Some(S { x: -1, y: 2 }), None] -0.000095 INFO 127.0.0.1:8888 -0.000096 INFO i128: 0 = 0, -1 = -1, MAX = 170141183460469231731687303715884105727, MIN = -170141183460469231731687303715884105728 -0.000097 INFO u128: 0 = 0, -1 = 1, MAX = 340282366920938463463374607431768211455, MIN = 0 -0.000098 INFO 340282366920938 -0.000099 INFO -170141183460469 -0.000100 INFO Hello 💜 -0.000101 INFO Hello 💜 & 🍕 -0.000102 INFO EnumLarge::A051 -0.000103 INFO EnumLarge::A269 -0.000104 INFO S { x: "hi" } -0.000105 INFO S { x: PhantomData, y: 42 } -0.000106 INFO bitfields 97 10000100 12 b"42" b"hello" -0.000107 INFO b"Hi" -0.000108 INFO b"Hi" +0.000091 INFO manual `Format` impl with multiple `write!`: MyMultiStruct@0 IS ZERO +0.000092 INFO manual `Format` impl with multiple `write!`: MyMultiStruct@20 IS NOT ZERO, division result: 5 +0.000093 INFO S { x: -1, y: 2 } +0.000094 INFO Some(S { x: -1, y: 2 }) +0.000095 INFO [S { x: -1, y: 2 }, S { x: -1, y: 2 }] +0.000096 INFO [Some(S { x: -1, y: 2 }), None] +0.000097 INFO 127.0.0.1:8888 +0.000098 INFO i128: 0 = 0, -1 = -1, MAX = 170141183460469231731687303715884105727, MIN = -170141183460469231731687303715884105728 +0.000099 INFO u128: 0 = 0, -1 = 1, MAX = 340282366920938463463374607431768211455, MIN = 0 +0.000100 INFO 340282366920938 +0.000101 INFO -170141183460469 +0.000102 INFO Hello 💜 +0.000103 INFO Hello 💜 & 🍕 +0.000104 INFO EnumLarge::A051 +0.000105 INFO EnumLarge::A269 +0.000106 INFO S { x: "hi" } +0.000107 INFO S { x: PhantomData, y: 42 } +0.000108 INFO bitfields 97 10000100 12 b"42" b"hello" 0.000109 INFO b"Hi" -0.000110 INFO [45054, 49406] -0.000111 INFO [Data { name: b"Hi", value: true }] -0.000112 INFO true true -0.000113 INFO aabbccdd -0.000114 INFO ddccbbaa -0.000115 INFO 1..2 -0.000116 INFO 1.. -0.000117 INFO ..2 -0.000118 INFO .. -0.000119 INFO 1..=2 -0.000120 INFO ..=2 -0.000121 INFO Zip(..) -0.000122 INFO ChunksExact(..) -0.000123 INFO Iter { slice: [0, 1, 2], position: ? } -0.000124 INFO Windows(..) -0.000125 INFO 1 -0.000126 INFO 1 +0.000110 INFO b"Hi" +0.000111 INFO b"Hi" +0.000112 INFO [45054, 49406] +0.000113 INFO [Data { name: b"Hi", value: true }] +0.000114 INFO true true +0.000115 INFO aabbccdd +0.000116 INFO ddccbbaa +0.000117 INFO 1..2 +0.000118 INFO 1.. +0.000119 INFO ..2 +0.000120 INFO .. +0.000121 INFO 1..=2 +0.000122 INFO ..=2 +0.000123 INFO Zip(..) +0.000124 INFO ChunksExact(..) +0.000125 INFO Iter { slice: [0, 1, 2], position: ? } +0.000126 INFO Windows(..) 0.000127 INFO 1 0.000128 INFO 1 0.000129 INFO 1 @@ -135,5 +135,7 @@ 0.000134 INFO 1 0.000135 INFO 1 0.000136 INFO 1 -0.000137 INFO ccbbaadd -0.000138 INFO QEMU test finished! +0.000137 INFO 1 +0.000138 INFO 1 +0.000139 INFO ccbbaadd +0.000140 INFO QEMU test finished! diff --git a/firmware/qemu/src/bin/log.rs b/firmware/qemu/src/bin/log.rs index 0017592e..1ae2edea 100644 --- a/firmware/qemu/src/bin/log.rs +++ b/firmware/qemu/src/bin/log.rs @@ -449,6 +449,28 @@ fn main() -> ! { "nested `Format` impls using `write!`: {=?}", MyStruct(Inner(42)), ); + + struct MyMultiStruct(u32); + + impl Format for MyMultiStruct { + fn format(&self, f: Formatter) { + defmt::write!(f, "MyMultiStruct@{=u32} ", self.0); + if self.0 == 0 { + defmt::write!(f, "IS ZERO") + } else { + defmt::write!(f, "IS NOT ZERO, division result: {=u32}", 100 / self.0) + } + } + } + + defmt::info!( + "manual `Format` impl with multiple `write!`: {=?}", + MyMultiStruct(0) + ); + defmt::info!( + "manual `Format` impl with multiple `write!`: {=?}", + MyMultiStruct(20) + ); } // Debug adapter diff --git a/src/formatter.rs b/src/formatter.rs index a2aa66bc..5acea7fe 100644 --- a/src/formatter.rs +++ b/src/formatter.rs @@ -1,6 +1,7 @@ use core::marker::PhantomData; /// Handle to a defmt logger. +#[derive(Copy, Clone)] pub struct Formatter<'a> { pub(crate) _phantom: PhantomData<&'a ()>, } From 52c63871e80f7054829ddda4b6cde104725ee506 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 30 Jun 2021 05:49:50 +0200 Subject: [PATCH 7/9] Update encode tests --- tests/encode.rs | 115 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 94 insertions(+), 21 deletions(-) diff --git a/tests/encode.rs b/tests/encode.rs index fd29d914..49c7cc69 100644 --- a/tests/encode.rs +++ b/tests/encode.rs @@ -43,11 +43,9 @@ fn inc(index: u16, n: u16) -> u16 { index.wrapping_add(n) } -fn check_format_implementation(val: &T, expected_encoding: &[u8]) { +fn write_format(val: &T) { defmt::export::istr(&T::_format_tag()); - let g = defmt::export::make_formatter(); - val._format_data(g); - assert_eq!(defmt::export::fetch_bytes(), expected_encoding); + val._format_data(); } macro_rules! check { @@ -69,7 +67,8 @@ macro_rules! check_format { $( v.extend(&($x).to_le_bytes()); )* - check_format_implementation($format, &v); + write_format($format); + assert_eq!(defmt::export::fetch_bytes(), v); } } } @@ -201,7 +200,7 @@ fn single_struct_manual() { impl Format for X { fn format(&self, f: Formatter) { - defmt::write!(f, "X {{ x: {=u8}, y: {=u16} }}", self.y, self.z) + defmt::write!(f, "X {{ y: {=u8}, z: {=u16} }}", self.y, self.z) } } @@ -209,10 +208,78 @@ fn single_struct_manual() { check_format!( &X { y: 1, z: 2 }, [ - index, // "X {{ x: {=u8}, y: {=u16} }}" - 1u8, // x - 2u8, // y.low - 0u8, // y.high + index, // "{=__internal_FormatSequence}" + inc(index, 1), // "X {{ y: {=u8}, z: {=u16} }}" + 1u8, // y + 2u16, // z + 0u16, // terminator + ], + ) +} + +#[test] +fn single_struct_manual_multiwrite() { + // Above `#[derive]`d impl should be equivalent to this: + struct X { + y: u8, + z: u16, + } + + impl Format for X { + fn format(&self, f: Formatter) { + defmt::write!(f, "y={=u8}", self.y); + defmt::write!(f, "z={=u16}", self.z); + } + } + + let index = fetch_string_index(); + check_format!( + &X { y: 1, z: 2 }, + [ + index, // "{=__internal_FormatSequence}" + inc(index, 1), // "y={=u8}" + 1u8, // y + inc(index, 2), // "z={=u16}" + 2u16, // z + 0u16, // terminator + ], + ) +} + +#[test] +fn slice_struct_manual_multiwrite() { + // Above `#[derive]`d impl should be equivalent to this: + struct X { + y: u8, + z: u16, + } + + impl Format for X { + fn format(&self, f: Formatter) { + defmt::write!(f, "y={=u8}", self.y); + defmt::write!(f, "z={=u16}", self.z); + } + } + + let index = fetch_string_index(); + check_format!( + &[X { y: 1, z: 2 }, X { y: 3, z: 4 }][..], + [ + index, // "{=[?]}" + 2u32, // len + inc(index, 1), // "{=__internal_FormatSequence}" + // first element + inc(index, 2), // "y={=u8}" + 1u8, // y + inc(index, 3), // "z={=u16}" + 2u16, // z + 0u16, // terminator + // second element + inc(index, 4), // "y={=u8}" + 3u8, // y + inc(index, 5), // "z={=u16}" + 4u16, // z + 0u16, // terminator ], ) } @@ -580,7 +647,13 @@ fn istr() { fn format_arrays() { let index = fetch_string_index(); let array: [u16; 0] = []; - check_format!(&array, [index]); + check_format!( + &array, + [ + index, // "{=[?;0]}" + inc(index, 1), // "{=u16}" + ] + ); let index = fetch_string_index(); let array: [u16; 3] = [1, 256, 257]; @@ -636,8 +709,9 @@ fn format_slice_of_structs() { inc(index, 1), // "X {{ y: {=?} }}" inc(index, 2), // "Y {{ z: {=u8} }}" 42u8, // [0].y.z - // second element: no tags - 24u8, // [1].y.z + // second element: no outer tag + inc(index, 3), // "Y {{ z: {=u8} }}" + 24u8, // [1].y.z ], ); } @@ -651,19 +725,18 @@ fn format_slice_of_slices() { [ index, // "{=[?]}" slice.len() as u32, // + inc(index, 1), // "{=[?]}" // first slice - inc(index, 1), // "{=[?]}" slice[0].len() as u32, - // its first element inc(index, 2), // "{=u16}" 256u16, // [0][0] - // its second element: no tag - 257u16, // [0][1] - // second slice: no tags + 257u16, // [0][1] + // second slice slice[1].len() as u32, - 258u16, // [1][0] - 259u16, // [1][1] - 260u16, // [1][2] + inc(index, 3), // "{=u16}" + 258u16, // [1][0] + 259u16, // [1][1] + 260u16, // [1][2] ], ); } From 63c3a5237dc0c90a2994e5610012c0adf60996e1 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 30 Jun 2021 06:26:44 +0200 Subject: [PATCH 8/9] remove unuseds --- decoder/src/decoder.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/decoder/src/decoder.rs b/decoder/src/decoder.rs index 8021784d..59a3caf5 100644 --- a/decoder/src/decoder.rs +++ b/decoder/src/decoder.rs @@ -1,6 +1,5 @@ use std::{ convert::{TryFrom, TryInto}, - mem, ops::Range, }; @@ -77,7 +76,7 @@ impl<'t, 'b> Decoder<'t, 'b> { let is_enum = format.contains('|'); let mut elements = Vec::with_capacity(num_elements); - for i in 0..num_elements { + for _i in 0..num_elements { let format = if is_enum { self.get_variant(format)? } else { From 1ad5b2ee763a4a27d55afb3a43275d118e6fee2d Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 30 Jun 2021 06:31:43 +0200 Subject: [PATCH 9/9] Using a Formatter multiple times is now allowed. --- tests/ui/fmt-for-loop.rs | 11 ----------- tests/ui/fmt-for-loop.stderr | 8 -------- tests/ui/fmt-twice.rs | 10 ---------- tests/ui/fmt-twice.stderr | 9 --------- tests/ui/write-for-loop.rs | 11 ----------- tests/ui/write-for-loop.stderr | 8 -------- tests/ui/write-twice.rs | 10 ---------- tests/ui/write-twice.stderr | 9 --------- 8 files changed, 76 deletions(-) delete mode 100644 tests/ui/fmt-for-loop.rs delete mode 100644 tests/ui/fmt-for-loop.stderr delete mode 100644 tests/ui/fmt-twice.rs delete mode 100644 tests/ui/fmt-twice.stderr delete mode 100644 tests/ui/write-for-loop.rs delete mode 100644 tests/ui/write-for-loop.stderr delete mode 100644 tests/ui/write-twice.rs delete mode 100644 tests/ui/write-twice.stderr diff --git a/tests/ui/fmt-for-loop.rs b/tests/ui/fmt-for-loop.rs deleted file mode 100644 index b5c372c0..00000000 --- a/tests/ui/fmt-for-loop.rs +++ /dev/null @@ -1,11 +0,0 @@ -struct S; - -impl defmt::Format for S { - fn format(&self, f: defmt::Formatter) { - for _ in 0..3 { - 0u8.format(f); - } - } -} - -fn main() {} diff --git a/tests/ui/fmt-for-loop.stderr b/tests/ui/fmt-for-loop.stderr deleted file mode 100644 index a2a19430..00000000 --- a/tests/ui/fmt-for-loop.stderr +++ /dev/null @@ -1,8 +0,0 @@ -error[E0382]: use of moved value: `f` - --> $DIR/fmt-for-loop.rs:6:24 - | -4 | fn format(&self, f: defmt::Formatter) { - | - move occurs because `f` has type `defmt::Formatter<'_>`, which does not implement the `Copy` trait -5 | for _ in 0..3 { -6 | 0u8.format(f); - | ^ value moved here, in previous iteration of loop diff --git a/tests/ui/fmt-twice.rs b/tests/ui/fmt-twice.rs deleted file mode 100644 index c29579aa..00000000 --- a/tests/ui/fmt-twice.rs +++ /dev/null @@ -1,10 +0,0 @@ -struct S; - -impl defmt::Format for S { - fn format(&self, f: defmt::Formatter) { - 0u8.format(f); - 0u16.format(f); - } -} - -fn main() {} diff --git a/tests/ui/fmt-twice.stderr b/tests/ui/fmt-twice.stderr deleted file mode 100644 index 0aa72c44..00000000 --- a/tests/ui/fmt-twice.stderr +++ /dev/null @@ -1,9 +0,0 @@ -error[E0382]: use of moved value: `f` - --> $DIR/fmt-twice.rs:6:21 - | -4 | fn format(&self, f: defmt::Formatter) { - | - move occurs because `f` has type `defmt::Formatter<'_>`, which does not implement the `Copy` trait -5 | 0u8.format(f); - | - value moved here -6 | 0u16.format(f); - | ^ value used here after move diff --git a/tests/ui/write-for-loop.rs b/tests/ui/write-for-loop.rs deleted file mode 100644 index 0e6cf191..00000000 --- a/tests/ui/write-for-loop.rs +++ /dev/null @@ -1,11 +0,0 @@ -struct S; - -impl defmt::Format for S { - fn format(&self, f: defmt::Formatter) { - for _ in 0..3 { - defmt::write!(f, "hello"); - } - } -} - -fn main() {} diff --git a/tests/ui/write-for-loop.stderr b/tests/ui/write-for-loop.stderr deleted file mode 100644 index f4f6f150..00000000 --- a/tests/ui/write-for-loop.stderr +++ /dev/null @@ -1,8 +0,0 @@ -error[E0382]: use of moved value: `f` - --> $DIR/write-for-loop.rs:6:27 - | -4 | fn format(&self, f: defmt::Formatter) { - | - move occurs because `f` has type `defmt::Formatter<'_>`, which does not implement the `Copy` trait -5 | for _ in 0..3 { -6 | defmt::write!(f, "hello"); - | ^ value moved here, in previous iteration of loop diff --git a/tests/ui/write-twice.rs b/tests/ui/write-twice.rs deleted file mode 100644 index 61010c9c..00000000 --- a/tests/ui/write-twice.rs +++ /dev/null @@ -1,10 +0,0 @@ -struct S; - -impl defmt::Format for S { - fn format(&self, f: defmt::Formatter) { - defmt::write!(f, "hello"); - defmt::write!(f, "world"); - } -} - -fn main() {} diff --git a/tests/ui/write-twice.stderr b/tests/ui/write-twice.stderr deleted file mode 100644 index 4a3066ea..00000000 --- a/tests/ui/write-twice.stderr +++ /dev/null @@ -1,9 +0,0 @@ -error[E0382]: use of moved value: `f` - --> $DIR/write-twice.rs:6:23 - | -4 | fn format(&self, f: defmt::Formatter) { - | - move occurs because `f` has type `defmt::Formatter<'_>`, which does not implement the `Copy` trait -5 | defmt::write!(f, "hello"); - | - value moved here -6 | defmt::write!(f, "world"); - | ^ value used here after move