Skip to content

Commit

Permalink
Merge #508
Browse files Browse the repository at this point in the history 
508: [5/n] Format trait v2 r=japaric a=Dirbaio

Part of #492 . Depends on ~~#505 #507 #521~~ #524

- Implemented new Format trait according to #492.
- Adjusted everything else accordingly.

Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>
  • Loading branch information
@bors @Dirbaio
bors[bot] and Dirbaio authored Jun 30, 2021
2 parents c0edd77 + 1ad5b2e commit 5abfc33
Show file tree
Hide file tree
Showing 30 changed files with 844 additions and 1,028 deletions.
14 changes: 6 additions & 8 deletions book/src/ser-format.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +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) {
if fmt.inner.needs_tag() {
let t = internp!("{=u8}");
fmt.inner.tag(&t);
}
fmt.inner.u8(self)
let t = internp!("{=u8}");
defmt::export::istr(&t);
defmt::export::u8(self)
// on the wire: [1, 42]
// string index ^ ^^ `self`
}
Expand All @@ -27,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]
Expand Down
137 changes: 35 additions & 102 deletions decoder/src/decoder.rs
View file
Original file line number Diff line number Diff line change
@@ -1,41 +1,20 @@
use std::{
convert::{TryFrom, TryInto},
mem,
ops::Range,
};

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<FormatList<'t>>,
// 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
Expand All @@ -48,28 +27,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::<LE>()? 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)
}

Expand Down Expand Up @@ -107,80 +72,20 @@ impl<'t, 'b> Decoder<'t, 'b> {
&mut self,
num_elements: usize,
) -> Result<Vec<FormatSliceElement<'t>>, DecodeError> {
if num_elements == 0 {
return Ok(vec![]);
}

let format = self.get_format()?;

// let variant_format = if
let is_enum = format.contains('|');
let below_enum = self.below_enum;

if is_enum {
self.below_enum = true;
}

let mut elements = Vec::with_capacity(num_elements);
let mut formats = vec![];
let mut cursor = 0;
for i in 0..num_elements {
let is_first = i == 0;

for _i in 0..num_elements {
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)
}

Expand Down Expand Up @@ -230,10 +135,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,
Expand Down Expand Up @@ -332,6 +234,37 @@ 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::<LE>()? 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)?;
seq_args.push(Arg::Format {
format: variant,
args: inner_args,
});
} else {
let inner_args = self.decode_format(format)?;
seq_args.push(Arg::Format {
format,
args: inner_args,
});
}
}
args.push(Arg::FormatSequence { args: seq_args })
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions decoder/src/frame.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
63 changes: 61 additions & 2 deletions decoder/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ impl Table {
}

// NOTE follows `parser::Type`
#[derive(Debug, PartialEq)]
#[derive(Debug, Clone, PartialEq)]
enum Arg<'t> {
/// Bool
Bool(bool),
Expand All @@ -229,6 +229,9 @@ enum Arg<'t> {
FormatSlice {
elements: Vec<FormatSliceElement<'t>>,
},
FormatSequence {
args: Vec<Arg<'t>>,
},
/// Slice or Array of bytes.
Slice(Vec<u8>),
/// Char
Expand All @@ -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
Expand Down Expand Up @@ -515,6 +518,62 @@ 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::FormatSequence {
args: 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();
Expand Down
11 changes: 8 additions & 3 deletions defmt.x.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.*);

Expand All @@ -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");
Loading

0 comments on commit 5abfc33

Please sign in to comment.