Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Format::format to take Formatter argument by value #305

Merged
merged 9 commits into from
Dec 16, 2020
Merged
4 changes: 2 additions & 2 deletions book/src/format.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ struct CRCCNF {
}

impl defmt::Format for CRCCNF {
fn format(&self, f: &mut defmt::Formatter) {
fn format(&self, f: defmt::Formatter) {
// format the bitfields of the register as struct fields
defmt::write!(
f,
Expand All @@ -68,7 +68,7 @@ Example below:
struct MyU8 { inner: u8 }

impl defmt::Format for MyU8 {
fn format(&self, f: &mut defmt::Formatter) {
fn format(&self, f: defmt::Formatter) {
self.inner.format(f)
}
}
Expand Down
4 changes: 2 additions & 2 deletions book/src/re-entrancy.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ The re-entrancy issue arises if the `Format` implementation calls a logging macr
# extern crate defmt;
# struct X;
impl defmt::Format for X {
fn format(&self, f: &mut defmt::Formatter) {
fn format(&self, f: defmt::Formatter) {
// ^ this is a handle to the global logger
defmt::info!("Hello!");
// ..
Expand All @@ -27,5 +27,5 @@ impl defmt::Format for X {

`f` is a handle to the global logger.
The `info!` call inside the `format` method is trying to access the global logger again.
If `info!` succeeds then you have two exclusive handles (`&mut Formatter`) to the logger and that's UB.
If `info!` succeeds then you have two exclusive handles (`Formatter`) to the logger and that's UB.
If `info!` uses a spinlock to access the logger then this will deadlock.
6 changes: 3 additions & 3 deletions book/src/ser-format.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ First let's see how a primitive implements the `Format` trait:
# trait Format { fn format(&self, fmt: &mut defmt::Formatter); }
impl Format for u8 {
fn format(&self, fmt: &mut defmt::Formatter) {
if fmt.needs_tag() {
if fmt.inner.needs_tag() {
let t = internp!("{:u8}");
fmt.u8(&t);
fmt.inner.u8(&t);
}
fmt.u8(self)
fmt.inner.u8(self)
// on the wire: [1, 42]
// string index ^ ^^ `self`
}
Expand Down
4 changes: 0 additions & 4 deletions firmware/qemu/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ test = false
name = "log"
test = false

[[bin]]
name = "double-write"
test = false

[[bin]]
name = "unwrap"
test = false
Expand Down
35 changes: 0 additions & 35 deletions firmware/qemu/src/bin/double-write.rs

This file was deleted.

4 changes: 2 additions & 2 deletions firmware/qemu/src/bin/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ fn main() -> ! {
struct Inner(u8);

impl Format for Inner {
fn format(&self, f: &mut Formatter) {
fn format(&self, f: Formatter) {
defmt::write!(f, "inner value ({:u8})", self.0);
}
}
Expand All @@ -432,7 +432,7 @@ fn main() -> ! {
struct MyStruct(Inner);

impl Format for MyStruct {
fn format(&self, f: &mut Formatter) {
fn format(&self, f: Formatter) {
defmt::write!(f, "outer value ({:?})", self.0);
}
}
Expand Down
1 change: 0 additions & 1 deletion firmware/qemu/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ test "panic"
test "assert"
test "assert-eq"
test "assert-ne"
test "double-write"
test "unwrap"
test "defmt-test"
if rustc -V | grep nightly; then
Expand Down
30 changes: 15 additions & 15 deletions macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ pub fn global_logger(args: TokenStream, input: TokenStream) -> TokenStream {
#vis struct #ident;

#[no_mangle]
unsafe fn _defmt_acquire() -> Option<defmt::Formatter> {
<#ident as defmt::Logger>::acquire().map(|nn| defmt::Formatter::from_raw(nn))
unsafe fn _defmt_acquire() -> Option<defmt::InternalFormatter> {
<#ident as defmt::Logger>::acquire().map(|nn| defmt::InternalFormatter::from_raw(nn))
}

#[no_mangle]
unsafe fn _defmt_release(f: defmt::Formatter) {
unsafe fn _defmt_release(f: defmt::InternalFormatter) {
<#ident as defmt::Logger>::release(f.into_raw())
}
)
Expand Down Expand Up @@ -244,7 +244,7 @@ pub fn format(ts: TokenStream) -> TokenStream {
quote!()
} else {
quote!(
f.u8(&#i);
f.inner.u8(&#i);
)
};

Expand All @@ -256,7 +256,7 @@ pub fn format(ts: TokenStream) -> TokenStream {
// 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.with_tag(|f| {
f.inner.with_tag(|f| {
#(#exprs;)*
});
}
Expand All @@ -265,8 +265,8 @@ pub fn format(ts: TokenStream) -> TokenStream {

let sym = mksym(&fs, "fmt", false);
exprs.push(quote!(
if f.needs_tag() {
f.istr(&defmt::export::istr(#sym));
if f.inner.needs_tag() {
f.inner.istr(&defmt::export::istr(#sym));
}
));
exprs.push(quote!(match self {
Expand All @@ -282,8 +282,8 @@ pub fn format(ts: TokenStream) -> TokenStream {

let sym = mksym(&fs, "fmt", false);
exprs.push(quote!(
if f.needs_tag() {
f.istr(&defmt::export::istr(#sym));
if f.inner.needs_tag() {
f.inner.istr(&defmt::export::istr(#sym));
}
));
exprs.push(quote!(match self {
Expand Down Expand Up @@ -316,7 +316,7 @@ pub fn format(ts: TokenStream) -> TokenStream {

quote!(
impl #impl_generics defmt::Format for #ident #type_generics #where_clause {
fn format(&self, f: &mut defmt::Formatter) {
fn format(&self, f: defmt::Formatter) {
#(#exprs)*
}
}
Expand Down Expand Up @@ -362,10 +362,10 @@ fn fields(
core::write!(format, "{}: {{:{}}}", ident, ty).ok();

if ty == "?" {
list.push(quote!(f.fmt(#ident, false)));
list.push(quote!(f.inner.fmt(#ident, false)));
} else {
let method = format_ident!("{}", ty);
list.push(quote!(f.#method(#ident)));
list.push(quote!(f.inner.#method(#ident)));
}
pats.push(quote!( #ident ));
} else {
Expand All @@ -375,10 +375,10 @@ fn fields(

let ident = format_ident!("arg{}", i);
if ty == "?" {
list.push(quote!(f.fmt(#ident, false)));
list.push(quote!(f.inner.fmt(#ident, false)));
} else {
let method = format_ident!("{}", ty);
list.push(quote!(f.#method(#ident)));
list.push(quote!(f.inner.#method(#ident)));
}

let i = syn::Index::from(i);
Expand Down Expand Up @@ -959,7 +959,7 @@ pub fn write(ts: TokenStream) -> TokenStream {
let fmt = &write.fmt;
// FIXME: Introduce a new `"write"` tag and decode it in a loop (breaking change).
let sym = mksym(&ls, "fmt", false);
quote!(match (&mut *#fmt, #(&(#args)),*) {
quote!(match (#fmt.inner, #(&(#args)),*) {
(_fmt_, #(#pats),*) => {
_fmt_.write_macro_start();
// HACK conditional should not be here; see FIXME in `format`
Expand Down
20 changes: 10 additions & 10 deletions src/export.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{Formatter, Str};
use crate::{InternalFormatter, Str};

#[cfg(feature = "unstable-test")]
thread_local! {
Expand All @@ -23,27 +23,27 @@ pub fn fetch_add_string_index() -> usize {
}

#[cfg(feature = "unstable-test")]
pub fn acquire() -> Option<Formatter> {
pub fn acquire() -> Option<InternalFormatter> {
None
}

#[cfg(not(feature = "unstable-test"))]
#[inline(never)]
pub fn acquire() -> Option<Formatter> {
pub fn acquire() -> Option<InternalFormatter> {
extern "Rust" {
fn _defmt_acquire() -> Option<Formatter>;
fn _defmt_acquire() -> Option<InternalFormatter>;
}
unsafe { _defmt_acquire() }
}

#[cfg(feature = "unstable-test")]
pub fn release(_: Formatter) {}
pub fn release(_: InternalFormatter) {}

#[cfg(not(feature = "unstable-test"))]
#[inline(never)]
pub fn release(fmt: Formatter) {
pub fn release(fmt: InternalFormatter) {
extern "Rust" {
fn _defmt_release(fmt: Formatter);
fn _defmt_release(fmt: InternalFormatter);
}
unsafe { _defmt_release(fmt) }
}
Expand Down Expand Up @@ -147,10 +147,10 @@ mod sealed {
pub struct NoneError;

impl Format for NoneError {
fn format(&self, fmt: &mut Formatter) {
if fmt.needs_tag() {
fn format(&self, fmt: Formatter) {
if fmt.inner.needs_tag() {
let t = internp!("Unwrap of a None option value");
fmt.u8(&t);
fmt.inner.u8(&t);
}
}
}
Expand Down
Loading