From 26d2fb0e316e0617032c9619c481f5eeb9423fc6 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Tue, 17 Nov 2020 13:58:47 +0100 Subject: [PATCH 01/20] add defmt::panic! --- defmt.x.in | 1 + elf2table/src/lib.rs | 3 +- macros/src/lib.rs | 98 ++++++++++++++++++++++++++++++++++++++++---- src/export.rs | 7 ++++ src/lib.rs | 12 +++++- 5 files changed, 110 insertions(+), 11 deletions(-) diff --git a/defmt.x.in b/defmt.x.in index 8fdcc3d3..d34c9f22 100644 --- a/defmt.x.in +++ b/defmt.x.in @@ -3,6 +3,7 @@ EXTERN(_defmt_acquire); EXTERN(_defmt_release); EXTERN(__defmt_default_timestamp); PROVIDE(_defmt_timestamp = __defmt_default_timestamp); +PROVIDE(_defmt_panic = __defmt_default_panic); SECTIONS { diff --git a/elf2table/src/lib.rs b/elf2table/src/lib.rs index 91a490cb..2e2d0348 100644 --- a/elf2table/src/lib.rs +++ b/elf2table/src/lib.rs @@ -75,7 +75,8 @@ pub fn parse(elf: &[u8]) -> Result, anyhow::Error> { } }; - defmt_decoder::check_version(version).map_err(anyhow::Error::msg)?; + // TODO(japaric) REMOVE! + // defmt_decoder::check_version(version).map_err(anyhow::Error::msg)?; // second pass to demangle symbols let mut map = BTreeMap::new(); diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 8b0b9a47..bb0b9a36 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -60,6 +60,52 @@ pub fn global_logger(args: TokenStream, input: TokenStream) -> TokenStream { .into() } +#[proc_macro_attribute] +pub fn panic_handler(args: TokenStream, input: TokenStream) -> TokenStream { + if !args.is_empty() { + return parse::Error::new( + Span2::call_site(), + "`#[defmt::panic_handler]` attribute takes no arguments", + ) + .to_compile_error() + .into(); + } + let f = parse_macro_input!(input as ItemFn); + + let rety_is_ok = match &f.sig.output { + ReturnType::Default => false, + ReturnType::Type(_, ty) => match &**ty { + Type::Never(_) => true, + _ => false, + }, + }; + + let ident = &f.sig.ident; + if f.sig.constness.is_some() + || f.sig.asyncness.is_some() + || f.sig.unsafety.is_some() + || f.sig.abi.is_some() + || !f.sig.generics.params.is_empty() + || f.sig.generics.where_clause.is_some() + || f.sig.variadic.is_some() + || !f.sig.inputs.is_empty() + || !rety_is_ok + { + return parse::Error::new(ident.span(), "function must have signature `fn() -> !`") + .to_compile_error() + .into(); + } + + let block = &f.block; + quote!( + #[export_name = "_defmt_panic"] + fn #ident() -> ! { + #block + } + ) + .into() +} + #[proc_macro_attribute] pub fn timestamp(args: TokenStream, input: TokenStream) -> TokenStream { if !args.is_empty() { @@ -393,8 +439,11 @@ fn is_logging_enabled(level: Level) -> TokenStream2 { // note that we are not using a `Level` type shared with `decoder` due to Cargo bugs in crate sharing // TODO -> move Level to parser? -fn log(level: Level, ts: TokenStream) -> TokenStream { - let log = parse_macro_input!(ts as Log); +fn log_ts(level: Level, ts: TokenStream) -> TokenStream { + log(level, parse_macro_input!(ts as Log)).into() +} + +fn log(level: Level, log: Log) -> TokenStream2 { let ls = log.litstr.value(); let fragments = match defmt_parser::parse(&ls) { Ok(args) => args, @@ -433,32 +482,65 @@ fn log(level: Level, ts: TokenStream) -> TokenStream { } } }) - .into() } #[proc_macro] pub fn trace(ts: TokenStream) -> TokenStream { - log(Level::Trace, ts) + log_ts(Level::Trace, ts) } #[proc_macro] pub fn debug(ts: TokenStream) -> TokenStream { - log(Level::Debug, ts) + log_ts(Level::Debug, ts) } #[proc_macro] pub fn info(ts: TokenStream) -> TokenStream { - log(Level::Info, ts) + log_ts(Level::Info, ts) } #[proc_macro] pub fn warn(ts: TokenStream) -> TokenStream { - log(Level::Warn, ts) + log_ts(Level::Warn, ts) } #[proc_macro] pub fn error(ts: TokenStream) -> TokenStream { - log(Level::Error, ts) + log_ts(Level::Error, ts) +} + +// not naming this `panic` to avoid shadowing `core::panic` in this scope +#[proc_macro] +pub fn panic_(ts: TokenStream) -> TokenStream { + let log_stmt = if ts.is_empty() { + // panic!() -> error!("panicked at 'explicit panic'") + log( + Level::Error, + Log { + litstr: LitStr::new("panicked at 'explicit panic'", Span2::call_site()), + rest: None, + }, + ) + } else { + // panic!("a", b, c) -> error!("panicked at 'a'", b, c) + let args = parse_macro_input!(ts as Log); + log( + Level::Error, + Log { + litstr: LitStr::new( + &format!("panicked at '{}'", args.litstr.value()), + Span2::call_site(), + ), + rest: args.rest, + }, + ) + }; + + quote!( + #log_stmt; + defmt::export::panic() + ) + .into() } // TODO share more code with `log` diff --git a/src/export.rs b/src/export.rs index b8492984..9f177049 100644 --- a/src/export.rs +++ b/src/export.rs @@ -146,3 +146,10 @@ mod sealed { pub fn truncate(x: impl sealed::Truncate) -> T { x.truncate() } + +pub fn panic() -> ! { + extern "Rust" { + fn _defmt_panic() -> !; + } + unsafe { _defmt_panic() } +} diff --git a/src/lib.rs b/src/lib.rs index 205aec4a..123de16e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,8 +16,7 @@ #[cfg(feature = "alloc")] extern crate alloc; -use core::mem::MaybeUninit; -use core::ptr::NonNull; +use core::{mem::MaybeUninit, ptr::NonNull}; #[doc(hidden)] pub mod export; @@ -26,6 +25,10 @@ mod leb; #[cfg(test)] mod tests; +pub use defmt_macros::panic_ as panic; + +pub use defmt_macros::panic_handler; + /// Creates an interned string ([`Str`]) from a string literal. /// /// This must be called on a string literal, and will allocate the literal in the object file. At @@ -461,3 +464,8 @@ pub trait Format { fn default_timestamp() -> u64 { 0 } + +#[export_name = "__defmt_default_panic"] +fn default_panic() -> ! { + core::panic!() +} From de4e7c76bc6d6f282420468b9020573364186142 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Wed, 18 Nov 2020 11:21:39 +0100 Subject: [PATCH 02/20] add todo! and unimplemented! --- macros/src/lib.rs | 34 ++++++++++++++++++++++++++++++++++ src/lib.rs | 3 +++ 2 files changed, 37 insertions(+) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index bb0b9a36..02a4811b 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -543,6 +543,40 @@ pub fn panic_(ts: TokenStream) -> TokenStream { .into() } +// not naming this `todo` to avoid shadowing `core::todo` in this scope +#[proc_macro] +pub fn todo_(ts: TokenStream) -> TokenStream { + let log_stmt = if ts.is_empty() { + // todo!() -> error!("panicked at 'not yet implemented'") + log( + Level::Error, + Log { + litstr: LitStr::new("panicked at 'not yet implemented'", Span2::call_site()), + rest: None, + }, + ) + } else { + // todo!("a", b, c) -> error!("panicked at 'not yet implemented: a'", b, c) + let args = parse_macro_input!(ts as Log); + log( + Level::Error, + Log { + litstr: LitStr::new( + &format!("panicked at 'not yet implemented: {}'", args.litstr.value()), + Span2::call_site(), + ), + rest: args.rest, + }, + ) + }; + + quote!( + #log_stmt; + defmt::export::panic() + ) + .into() +} + // TODO share more code with `log` #[proc_macro] pub fn winfo(ts: TokenStream) -> TokenStream { diff --git a/src/lib.rs b/src/lib.rs index 123de16e..61a79361 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,6 +25,9 @@ mod leb; #[cfg(test)] mod tests; +pub use defmt_macros::todo_ as todo; +pub use defmt_macros::todo_ as unimplemented; + pub use defmt_macros::panic_ as panic; pub use defmt_macros::panic_handler; From 266de535a174c5de72ba741ea874bdeb03a18b3a Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Wed, 18 Nov 2020 11:34:44 +0100 Subject: [PATCH 03/20] fix x86 build --- src/lib.rs | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 61a79361..c151a13c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -215,12 +215,6 @@ impl Formatter { unsafe { self.writer.as_mut().write(bytes) } } - /// Implementation detail - #[cfg(target_arch = "x86_64")] - pub unsafe fn from_raw(_: NonNull) -> Self { - unreachable!() - } - /// Implementation detail #[cfg(not(target_arch = "x86_64"))] pub unsafe fn from_raw(writer: NonNull) -> Self { @@ -232,12 +226,6 @@ impl Formatter { } } - /// Implementation detail - #[cfg(target_arch = "x86_64")] - pub unsafe fn into_raw(self) -> NonNull { - unreachable!() - } - /// Implementation detail #[cfg(not(target_arch = "x86_64"))] pub unsafe fn into_raw(self) -> NonNull { @@ -426,6 +414,29 @@ impl Formatter { } } +// these need to be in a separate module or `unreachable!` will end up calling `defmt::panic` and +// this will not compile +// (using `core::unreachable!` instead of `unreachable!` doesn't help) +#[cfg(target_arch = "x86_64")] +mod x86_64 { + use core::ptr::NonNull; + + use super::Write; + + #[doc(hidden)] + impl super::Formatter { + /// Implementation detail + pub unsafe fn from_raw(_: NonNull) -> Self { + unreachable!() + } + + /// Implementation detail + pub unsafe fn into_raw(self) -> NonNull { + unreachable!() + } + } +} + /// Trait for defmt logging targets. pub trait Write { /// Writes `bytes` to the destination. From 556244f8da6edcc2d1348a90d24c18c032e4e96f Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Wed, 18 Nov 2020 11:53:50 +0100 Subject: [PATCH 04/20] add unreachable! --- macros/src/lib.rs | 66 ++++++++++++++++++++++------------------------- src/lib.rs | 2 ++ 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 02a4811b..2d5f2e0e 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -509,15 +509,17 @@ pub fn error(ts: TokenStream) -> TokenStream { log_ts(Level::Error, ts) } -// not naming this `panic` to avoid shadowing `core::panic` in this scope -#[proc_macro] -pub fn panic_(ts: TokenStream) -> TokenStream { +fn panic( + ts: TokenStream, + zero_args_string: &str, + string_transform: impl FnOnce(&str) -> String, +) -> TokenStream { let log_stmt = if ts.is_empty() { // panic!() -> error!("panicked at 'explicit panic'") log( Level::Error, Log { - litstr: LitStr::new("panicked at 'explicit panic'", Span2::call_site()), + litstr: LitStr::new(zero_args_string, Span2::call_site()), rest: None, }, ) @@ -527,10 +529,7 @@ pub fn panic_(ts: TokenStream) -> TokenStream { log( Level::Error, Log { - litstr: LitStr::new( - &format!("panicked at '{}'", args.litstr.value()), - Span2::call_site(), - ), + litstr: LitStr::new(&string_transform(&args.litstr.value()), Span2::call_site()), rest: args.rest, }, ) @@ -543,38 +542,35 @@ pub fn panic_(ts: TokenStream) -> TokenStream { .into() } +// not naming this `panic` to avoid shadowing `core::panic` in this scope +#[proc_macro] +pub fn panic_(ts: TokenStream) -> TokenStream { + panic(ts, "panicked at 'explicit panic'", |format_string| { + format!("panicked at '{}'", format_string) + }) +} + // not naming this `todo` to avoid shadowing `core::todo` in this scope #[proc_macro] pub fn todo_(ts: TokenStream) -> TokenStream { - let log_stmt = if ts.is_empty() { - // todo!() -> error!("panicked at 'not yet implemented'") - log( - Level::Error, - Log { - litstr: LitStr::new("panicked at 'not yet implemented'", Span2::call_site()), - rest: None, - }, - ) - } else { - // todo!("a", b, c) -> error!("panicked at 'not yet implemented: a'", b, c) - let args = parse_macro_input!(ts as Log); - log( - Level::Error, - Log { - litstr: LitStr::new( - &format!("panicked at 'not yet implemented: {}'", args.litstr.value()), - Span2::call_site(), - ), - rest: args.rest, - }, - ) - }; + panic(ts, "panicked at 'not yet implemented'", |format_string| { + format!("panicked at 'not yet implemented: {}'", format_string) + }) +} - quote!( - #log_stmt; - defmt::export::panic() +// not naming this `unreachable` to avoid shadowing `core::unreachable` in this scope +#[proc_macro] +pub fn unreachable_(ts: TokenStream) -> TokenStream { + panic( + ts, + "panicked at 'internal error: entered unreachable code'", + |format_string| { + format!( + "panicked at 'internal error: entered unreachable code: {}'", + format_string + ) + }, ) - .into() } // TODO share more code with `log` diff --git a/src/lib.rs b/src/lib.rs index c151a13c..fa0328fc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,6 +25,8 @@ mod leb; #[cfg(test)] mod tests; +pub use defmt_macros::unreachable_ as unreachable; + pub use defmt_macros::todo_ as todo; pub use defmt_macros::todo_ as unimplemented; From 23ff934893e2c5d0f6d091e324e5c9c05eeef858 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Wed, 18 Nov 2020 12:13:18 +0100 Subject: [PATCH 05/20] add assert! --- macros/src/lib.rs | 78 ++++++++++++++++++++++++++++++++++++++++++----- src/lib.rs | 2 ++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 2d5f2e0e..060dc7e1 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -440,10 +440,10 @@ fn is_logging_enabled(level: Level) -> TokenStream2 { // note that we are not using a `Level` type shared with `decoder` due to Cargo bugs in crate sharing // TODO -> move Level to parser? fn log_ts(level: Level, ts: TokenStream) -> TokenStream { - log(level, parse_macro_input!(ts as Log)).into() + log(level, parse_macro_input!(ts as FormatArgs)).into() } -fn log(level: Level, log: Log) -> TokenStream2 { +fn log(level: Level, log: FormatArgs) -> TokenStream2 { let ls = log.litstr.value(); let fragments = match defmt_parser::parse(&ls) { Ok(args) => args, @@ -518,17 +518,17 @@ fn panic( // panic!() -> error!("panicked at 'explicit panic'") log( Level::Error, - Log { + FormatArgs { litstr: LitStr::new(zero_args_string, Span2::call_site()), rest: None, }, ) } else { // panic!("a", b, c) -> error!("panicked at 'a'", b, c) - let args = parse_macro_input!(ts as Log); + let args = parse_macro_input!(ts as FormatArgs); log( Level::Error, - Log { + FormatArgs { litstr: LitStr::new(&string_transform(&args.litstr.value()), Span2::call_site()), rest: args.rest, }, @@ -573,6 +573,36 @@ pub fn unreachable_(ts: TokenStream) -> TokenStream { ) } +// not naming this `assert` to avoid shadowing `core::assert` in this scope +#[proc_macro] +pub fn assert_(ts: TokenStream) -> TokenStream { + let assert = parse_macro_input!(ts as Assert); + + let condition = assert.condition; + let log_stmt = if let Some(args) = assert.args { + log(Level::Error, args) + } else { + log( + Level::Error, + FormatArgs { + litstr: LitStr::new( + &format!("assertion failed: {}", quote!(#condition)), + Span2::call_site(), + ), + rest: None, + }, + ) + }; + + quote!( + if !(#condition) { + #log_stmt; + defmt::export::panic() + } + ) + .into() +} + // TODO share more code with `log` #[proc_macro] pub fn winfo(ts: TokenStream) -> TokenStream { @@ -612,12 +642,46 @@ pub fn winfo(ts: TokenStream) -> TokenStream { .into() } -struct Log { +struct Assert { + condition: Expr, + args: Option, +} + +impl Parse for Assert { + fn parse(input: ParseStream) -> parse::Result { + let condition = input.parse()?; + if input.is_empty() { + // assert!(a) + return Ok(Assert { + condition, + args: None, + }); + } + + let _comma: Token![,] = input.parse()?; + + if input.is_empty() { + // assert!(a,) + Ok(Assert { + condition, + args: None, + }) + } else { + // assert!(a, "b", c) + Ok(Assert { + condition, + args: Some(input.parse()?), + }) + } + } +} + +struct FormatArgs { litstr: LitStr, rest: Option<(Token![,], Punctuated)>, } -impl Parse for Log { +impl Parse for FormatArgs { fn parse(input: ParseStream) -> parse::Result { Ok(Self { litstr: input.parse()?, diff --git a/src/lib.rs b/src/lib.rs index fa0328fc..eeedf7a9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,6 +25,8 @@ mod leb; #[cfg(test)] mod tests; +pub use defmt_macros::assert_ as assert; + pub use defmt_macros::unreachable_ as unreachable; pub use defmt_macros::todo_ as todo; From 31a9e499f35547b82350fb8ad7794c15dd867dc3 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Wed, 18 Nov 2020 12:33:28 +0100 Subject: [PATCH 06/20] assert: adjust panic message to resemble core::assert's --- macros/src/lib.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 060dc7e1..83a72c83 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -580,13 +580,22 @@ pub fn assert_(ts: TokenStream) -> TokenStream { let condition = assert.condition; let log_stmt = if let Some(args) = assert.args { - log(Level::Error, args) + log( + Level::Error, + FormatArgs { + litstr: LitStr::new( + &format!("panicked at '{}'", args.litstr.value()), + Span2::call_site(), + ), + rest: args.rest, + }, + ) } else { log( Level::Error, FormatArgs { litstr: LitStr::new( - &format!("assertion failed: {}", quote!(#condition)), + &format!("panicked at 'assertion failed: {}'", quote!(#condition)), Span2::call_site(), ), rest: None, From 29fa9fc5b63a7dbb133fdb7e3335ca292124030b Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Wed, 18 Nov 2020 13:59:01 +0100 Subject: [PATCH 07/20] add assert_eq! --- macros/src/lib.rs | 115 ++++++++++++++++++++++++++++++++++++++++++++-- src/lib.rs | 1 + 2 files changed, 112 insertions(+), 4 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 83a72c83..ee8064b6 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -7,15 +7,14 @@ use proc_macro::TokenStream; use defmt_parser::{Fragment, Level}; use proc_macro2::{Ident as Ident2, Span as Span2, TokenStream as TokenStream2}; use quote::{format_ident, quote}; -use syn::GenericParam; -use syn::WhereClause; use syn::{ parse::{self, Parse, ParseStream}, parse_macro_input, punctuated::Punctuated, spanned::Spanned as _, - Data, DeriveInput, Expr, Fields, FieldsNamed, FieldsUnnamed, ItemFn, ItemStruct, LitStr, - ReturnType, Token, Type, WherePredicate, + Data, DeriveInput, Expr, ExprPath, Fields, FieldsNamed, FieldsUnnamed, GenericParam, ItemFn, + ItemStruct, LitStr, Path, PathArguments, PathSegment, ReturnType, Token, Type, WhereClause, + WherePredicate, }; #[proc_macro_attribute] @@ -612,6 +611,73 @@ pub fn assert_(ts: TokenStream) -> TokenStream { .into() } +// not naming this `assert_eq` to avoid shadowing `core::assert_eq` in this scope +#[proc_macro] +pub fn assert_eq_(ts: TokenStream) -> TokenStream { + let assert = parse_macro_input!(ts as AssertEq); + + let left = assert.left; + let right = assert.right; + + let mut log_args = Punctuated::new(); + + let extra_string = if let Some(args) = assert.args { + if let Some(rest) = args.rest { + log_args.extend(rest.1); + } + format!(": {}", args.litstr.value()) + } else { + String::new() + }; + + for val in &["left_val", "right_val"] { + let mut segments = Punctuated::new(); + segments.push(PathSegment { + ident: Ident2::new(*val, Span2::call_site()), + arguments: PathArguments::None, + }); + + log_args.push(Expr::Path(ExprPath { + attrs: vec![], + qself: None, + path: Path { + leading_colon: None, + segments, + }, + })); + } + + let log_stmt = log( + Level::Error, + FormatArgs { + litstr: LitStr::new( + &format!( + "panicked at 'assertion failed: `(left == right)`'{} + left: `{{:?}}` +right: `{{:?}}`", + extra_string + ), + Span2::call_site(), + ), + rest: Some((syn::token::Comma::default(), log_args)), + }, + ); + + quote!( + // evaluate arguments first + match (&(#left), &(#right)) { + (left_val, right_val) => { + // following `core::assert_eq!` + if !(*left_val == *right_val) { + #log_stmt; + defmt::export::panic() + } + } + } + ) + .into() +} + // TODO share more code with `log` #[proc_macro] pub fn winfo(ts: TokenStream) -> TokenStream { @@ -685,6 +751,47 @@ impl Parse for Assert { } } +struct AssertEq { + left: Expr, + right: Expr, + args: Option, +} + +impl Parse for AssertEq { + fn parse(input: ParseStream) -> parse::Result { + let left = input.parse()?; + let _comma: Token![,] = input.parse()?; + let right = input.parse()?; + + if input.is_empty() { + // assert_eq!(a, b) + return Ok(AssertEq { + left, + right, + args: None, + }); + } + + let _comma: Token![,] = input.parse()?; + + if input.is_empty() { + // assert_eq!(a, b,) + Ok(AssertEq { + left, + right, + args: None, + }) + } else { + // assert_eq!(a, b, "c", d) + Ok(AssertEq { + left, + right, + args: Some(input.parse()?), + }) + } + } +} + struct FormatArgs { litstr: LitStr, rest: Option<(Token![,], Punctuated)>, diff --git a/src/lib.rs b/src/lib.rs index eeedf7a9..e4a71e2c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,6 +26,7 @@ mod leb; mod tests; pub use defmt_macros::assert_ as assert; +pub use defmt_macros::assert_eq_ as assert_eq; pub use defmt_macros::unreachable_ as unreachable; From 698d28349aea9328e06f836005b78dfa4af80bea Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 11:28:58 +0100 Subject: [PATCH 08/20] add assert_ne --- macros/src/lib.rs | 38 +++++++++++++++++++++++++++++++++++--- src/lib.rs | 1 + 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index ee8064b6..d31f1a5a 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -1,6 +1,7 @@ //! INTERNAL; DO NOT USE. Please use the `defmt` crate to access the functionality implemented here mod symbol; +use core::fmt; use core::fmt::Write as _; use proc_macro::TokenStream; @@ -611,9 +612,35 @@ pub fn assert_(ts: TokenStream) -> TokenStream { .into() } +#[derive(PartialEq)] +enum BinOp { + Eq, + Ne, +} + +impl fmt::Display for BinOp { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str(match self { + BinOp::Eq => "==", + BinOp::Ne => "!=", + }) + } +} + // not naming this `assert_eq` to avoid shadowing `core::assert_eq` in this scope #[proc_macro] pub fn assert_eq_(ts: TokenStream) -> TokenStream { + assert_binop(ts, BinOp::Eq) +} + +// not naming this `assert_ne` to avoid shadowing `core::assert_ne` in this scope +#[proc_macro] +pub fn assert_ne_(ts: TokenStream) -> TokenStream { + assert_binop(ts, BinOp::Ne) +} + +// not naming this `assert_eq` to avoid shadowing `core::assert_eq` in this scope +fn assert_binop(ts: TokenStream, binop: BinOp) -> TokenStream { let assert = parse_macro_input!(ts as AssertEq); let left = assert.left; @@ -652,10 +679,10 @@ pub fn assert_eq_(ts: TokenStream) -> TokenStream { FormatArgs { litstr: LitStr::new( &format!( - "panicked at 'assertion failed: `(left == right)`'{} + "panicked at 'assertion failed: `(left {} right)`'{} left: `{{:?}}` right: `{{:?}}`", - extra_string + binop, extra_string ), Span2::call_site(), ), @@ -663,12 +690,17 @@ right: `{{:?}}`", }, ); + let mut cond = quote!(*left_val == *right_val); + if binop == BinOp::Eq { + cond = quote!(!(#cond)); + } + quote!( // evaluate arguments first match (&(#left), &(#right)) { (left_val, right_val) => { // following `core::assert_eq!` - if !(*left_val == *right_val) { + if #cond { #log_stmt; defmt::export::panic() } diff --git a/src/lib.rs b/src/lib.rs index e4a71e2c..8e5a987b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,6 +27,7 @@ mod tests; pub use defmt_macros::assert_ as assert; pub use defmt_macros::assert_eq_ as assert_eq; +pub use defmt_macros::assert_ne_ as assert_ne; pub use defmt_macros::unreachable_ as unreachable; From db68648a7fa045a5fa41d61391cc8bb44a6023cc Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 11:43:12 +0100 Subject: [PATCH 09/20] add debug_assert, debug_assert_eq and debug_assert_ne --- macros/src/lib.rs | 31 +++++++++++++++++++++++++++++++ src/lib.rs | 4 ++++ 2 files changed, 35 insertions(+) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index d31f1a5a..83a474b9 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -710,6 +710,37 @@ right: `{{:?}}`", .into() } +// NOTE these `debug_*` macros can be written using `macro_rules!` (that'd be simpler) but that +// results in an incorrect source code location being reported: the location of the `macro_rules!` +// statement is reported. Using a proc-macro results in the call site being reported, which is what +// we want +#[proc_macro] +pub fn debug_assert_(ts: TokenStream) -> TokenStream { + let assert = TokenStream2::from(assert_(ts)); + quote!(if cfg!(debug_assertions) { + #assert + }) + .into() +} + +#[proc_macro] +pub fn debug_assert_eq_(ts: TokenStream) -> TokenStream { + let assert = TokenStream2::from(assert_eq_(ts)); + quote!(if cfg!(debug_assertions) { + #assert + }) + .into() +} + +#[proc_macro] +pub fn debug_assert_ne_(ts: TokenStream) -> TokenStream { + let assert = TokenStream2::from(assert_ne_(ts)); + quote!(if cfg!(debug_assertions) { + #assert + }) + .into() +} + // TODO share more code with `log` #[proc_macro] pub fn winfo(ts: TokenStream) -> TokenStream { diff --git a/src/lib.rs b/src/lib.rs index 8e5a987b..b1137c42 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -29,6 +29,10 @@ pub use defmt_macros::assert_ as assert; pub use defmt_macros::assert_eq_ as assert_eq; pub use defmt_macros::assert_ne_ as assert_ne; +pub use defmt_macros::debug_assert_ as debug_assert; +pub use defmt_macros::debug_assert_eq_ as debug_assert_eq; +pub use defmt_macros::debug_assert_ne_ as debug_assert_ne; + pub use defmt_macros::unreachable_ as unreachable; pub use defmt_macros::todo_ as todo; From 031acfe18ea52567ee743aa45e1ca4881a84227f Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 12:44:16 +0100 Subject: [PATCH 10/20] add API docs --- src/lib.rs | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index b1137c42..fdec021a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,21 +25,103 @@ mod leb; #[cfg(test)] mod tests; +/// Just like the [`core::assert!`] macro but `defmt` is used to log the panic message +/// +/// [`core::assert!`]: https://doc.rust-lang.org/core/macro.assert.html +/// +/// If used, the format string must follow the defmt syntax (documented in [the manual]) +/// +/// [the manual]: https://defmt.ferrous-systems.com/macros.html pub use defmt_macros::assert_ as assert; + +/// Just like the [`core::assert_eq!`] macro but `defmt` is used to log the panic message +/// +/// [`core::assert_eq!`]: https://doc.rust-lang.org/core/macro.assert_eq.html +/// +/// If used, the format string must follow the defmt syntax (documented in [the manual]) +/// +/// [the manual]: https://defmt.ferrous-systems.com/macros.html pub use defmt_macros::assert_eq_ as assert_eq; + +/// Just like the [`core::assert_ne!`] macro but `defmt` is used to log the panic message +/// +/// [`core::assert_ne!`]: https://doc.rust-lang.org/core/macro.assert_ne.html +/// +/// If used, the format string must follow the defmt syntax (documented in [the manual]) +/// +/// [the manual]: https://defmt.ferrous-systems.com/macros.html pub use defmt_macros::assert_ne_ as assert_ne; +/// Just like the [`core::debug_assert!`] macro but `defmt` is used to log the panic message +/// +/// [`core::debug_assert!`]: https://doc.rust-lang.org/core/macro.debug_assert.html +/// +/// If used, the format string must follow the defmt syntax (documented in [the manual]) +/// +/// [the manual]: https://defmt.ferrous-systems.com/macros.html pub use defmt_macros::debug_assert_ as debug_assert; + +/// Just like the [`core::debug_assert_eq!`] macro but `defmt` is used to log the panic message +/// +/// [`core::debug_assert_eq!`]: https://doc.rust-lang.org/core/macro.debug_assert_eq.html +/// +/// If used, the format string must follow the defmt syntax (documented in [the manual]) +/// +/// [the manual]: https://defmt.ferrous-systems.com/macros.html pub use defmt_macros::debug_assert_eq_ as debug_assert_eq; + +/// Just like the [`core::debug_assert_ne!`] macro but `defmt` is used to log the panic message +/// +/// [`core::debug_assert_ne!`]: https://doc.rust-lang.org/core/macro.debug_assert_ne.html +/// +/// If used, the format string must follow the defmt syntax (documented in [the manual]) +/// +/// [the manual]: https://defmt.ferrous-systems.com/macros.html pub use defmt_macros::debug_assert_ne_ as debug_assert_ne; +/// Just like the [`core::unreachable!`] macro but `defmt` is used to log the panic message +/// +/// [`core::unreachable!`]: https://doc.rust-lang.org/core/macro.unreachable.html +/// +/// If used, the format string must follow the defmt syntax (documented in [the manual]) +/// +/// [the manual]: https://defmt.ferrous-systems.com/macros.html pub use defmt_macros::unreachable_ as unreachable; +/// Just like the [`core::todo!`] macro but `defmt` is used to log the panic message +/// +/// [`core::todo!`]: https://doc.rust-lang.org/core/macro.todo.html +/// +/// If used, the format string must follow the defmt syntax (documented in [the manual]) +/// +/// [the manual]: https://defmt.ferrous-systems.com/macros.html pub use defmt_macros::todo_ as todo; + +/// Just like the [`core::unimplemented!`] macro but `defmt` is used to log the panic message +/// +/// [`core::unimplemented!`]: https://doc.rust-lang.org/core/macro.unimplemented.html +/// +/// If used, the format string must follow the defmt syntax (documented in [the manual]) +/// +/// [the manual]: https://defmt.ferrous-systems.com/macros.html pub use defmt_macros::todo_ as unimplemented; +/// Just like the [`core::panic!`] macro but `defmt` is used to log the panic message +/// +/// [`core::panic!`]: https://doc.rust-lang.org/core/macro.panic.html +/// +/// If used, the format string must follow the defmt syntax (documented in [the manual]) +/// +/// [the manual]: https://defmt.ferrous-systems.com/macros.html pub use defmt_macros::panic_ as panic; +/// Overrides the panicking behavior of `defmt::panic!` +/// +/// By default, `defmt::panic!` calls `core::panic!` after logging the panic message using `defmt`. +/// This can result in the panic message being printed twice in some cases. To avoid that issue use +/// this macro. See [the manual] for details. +/// +/// [the manual]: https://defmt.ferrous-systems.com/panic.html pub use defmt_macros::panic_handler; /// Creates an interned string ([`Str`]) from a string literal. From ed4daa4b761c75b0b224417935cb12a69fb6c68a Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 12:45:03 +0100 Subject: [PATCH 11/20] re-add version check --- elf2table/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/elf2table/src/lib.rs b/elf2table/src/lib.rs index 2e2d0348..91a490cb 100644 --- a/elf2table/src/lib.rs +++ b/elf2table/src/lib.rs @@ -75,8 +75,7 @@ pub fn parse(elf: &[u8]) -> Result, anyhow::Error> { } }; - // TODO(japaric) REMOVE! - // defmt_decoder::check_version(version).map_err(anyhow::Error::msg)?; + defmt_decoder::check_version(version).map_err(anyhow::Error::msg)?; // second pass to demangle symbols let mut map = BTreeMap::new(); From d9f3694e71cbc0800baa5337d73177d621c8d98c Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 14:11:30 +0100 Subject: [PATCH 12/20] book: document `panic!` and `assert!` macros --- book/src/SUMMARY.md | 1 + book/src/panic.md | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 book/src/panic.md diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index bde00a48..b27917a0 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -14,6 +14,7 @@ - [Filtering](./filtering.md) - [#[timestamp]](./timestamp.md) - [#[global_logger]](./global-logger.md) + - [panic! and assert!](./panic.md) - [Printers](./printers.md) - [Migrating from git defmt to stable defmt](./migration.md) - [Design & impl details](./design.md) diff --git a/book/src/panic.md b/book/src/panic.md new file mode 100644 index 00000000..662baa18 --- /dev/null +++ b/book/src/panic.md @@ -0,0 +1,38 @@ +# `panic!` and `assert!` + +The `defmt` crate provides its own version of `panic!`-like and `assert!`-like macros. +The `defmt` version of these macros will log the panic message using `defmt` and then call `core::panic!` (by default). +Because the panic message is formatted using `defmt!` the format string must use the same syntax as the logging macros (e.g. `info!`). + +## `#[defmt::panic_handler]` + +Because `defmt::panic!` invokes `core::panic!` this can result in the panic message being printed twice if your `#[core::panic_handler]` also prints the panic message. +This is the case if you use [`panic-probe`] with the `print-defmt` feature enabled but not an issue if you are using the [`panic-abort`] crate, for example. + +[`panic-probe`]: https://crates.io/crates/panic-probe +[`panic-abort`]: https://crates.io/crates/panic-abort + +To avoid this issue you can use the `#[defmt::panic_handler]` to *override* the panicking behavior of `defmt::panic`-like and `defmt::assert`-like macros. +This attribute must be placed on a function with signature `fn() -> !`. +In this function you'll want to replicate the panicking behavior of `#[core::panic_handler]` but leave out the part that prints the panic message. +For example: + + + +``` rust, ignore +#[panic_handler] // built-in ("core") attribute +fn core_panic(info: &core::panic::PanicInfo) -> ! { + print(info); // e.g. using RTT + reset() +} + +#[defmt::panic_handler] // defmt's attribute +fn defmt_panic() -> ! { + // leave out the printing part here + reset() +} +``` + +If you are using the `panic-probe` crate then you should "abort" (call `cortex_m::asm::udf`) from `#[defmt::panic_handler]` to match its behavior. + +NOTE: even if you don't run into the "double panic message printed" issue you may still want to use `#[defmt::panic_handler]` because this way `defmt::panic` and `defmt::assert` will *not* go through the `core::panic` machinery and that *may* reduce code size (we recommend you measure the effect of the change). From 7bdcde128c9204b18bd7a0ced3f12f005d3c681f Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 14:31:37 +0100 Subject: [PATCH 13/20] assert: move closing single quote --- macros/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 83a474b9..90906da5 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -679,7 +679,7 @@ fn assert_binop(ts: TokenStream, binop: BinOp) -> TokenStream { FormatArgs { litstr: LitStr::new( &format!( - "panicked at 'assertion failed: `(left {} right)`'{} + "panicked at 'assertion failed: `(left {} right)`{}' left: `{{:?}}` right: `{{:?}}`", binop, extra_string From c2c335a51956c6b74d47684cb5055eddd41a30ff Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 14:49:24 +0100 Subject: [PATCH 14/20] add snapshot tests --- firmware/qemu/src/bin/assert-eq.out | 3 +++ firmware/qemu/src/bin/assert-eq.release.out | 3 +++ firmware/qemu/src/bin/assert-eq.rs | 24 ++++++++++++++++++++ firmware/qemu/src/bin/assert-ne.out | 3 +++ firmware/qemu/src/bin/assert-ne.release.out | 3 +++ firmware/qemu/src/bin/assert-ne.rs | 24 ++++++++++++++++++++ firmware/qemu/src/bin/assert.out | 1 + firmware/qemu/src/bin/assert.release.out | 1 + firmware/qemu/src/bin/assert.rs | 25 +++++++++++++++++++++ firmware/qemu/src/bin/panic.out | 1 + firmware/qemu/src/bin/panic.release.out | 1 + firmware/qemu/src/bin/panic.rs | 21 +++++++++++++++++ firmware/qemu/test.sh | 4 +++- 13 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 firmware/qemu/src/bin/assert-eq.out create mode 100644 firmware/qemu/src/bin/assert-eq.release.out create mode 100644 firmware/qemu/src/bin/assert-eq.rs create mode 100644 firmware/qemu/src/bin/assert-ne.out create mode 100644 firmware/qemu/src/bin/assert-ne.release.out create mode 100644 firmware/qemu/src/bin/assert-ne.rs create mode 100644 firmware/qemu/src/bin/assert.out create mode 100644 firmware/qemu/src/bin/assert.release.out create mode 100644 firmware/qemu/src/bin/assert.rs create mode 100644 firmware/qemu/src/bin/panic.out create mode 100644 firmware/qemu/src/bin/panic.release.out create mode 100644 firmware/qemu/src/bin/panic.rs diff --git a/firmware/qemu/src/bin/assert-eq.out b/firmware/qemu/src/bin/assert-eq.out new file mode 100644 index 00000000..fe17c3f3 --- /dev/null +++ b/firmware/qemu/src/bin/assert-eq.out @@ -0,0 +1,3 @@ +0.000000 ERROR panicked at 'assertion failed: `(left == right)`: dev' + left: `41` +right: `43` diff --git a/firmware/qemu/src/bin/assert-eq.release.out b/firmware/qemu/src/bin/assert-eq.release.out new file mode 100644 index 00000000..06e706db --- /dev/null +++ b/firmware/qemu/src/bin/assert-eq.release.out @@ -0,0 +1,3 @@ +0.000000 ERROR panicked at 'assertion failed: `(left == right)`: release' + left: `41` +right: `43` diff --git a/firmware/qemu/src/bin/assert-eq.rs b/firmware/qemu/src/bin/assert-eq.rs new file mode 100644 index 00000000..c2fcd749 --- /dev/null +++ b/firmware/qemu/src/bin/assert-eq.rs @@ -0,0 +1,24 @@ +#![no_std] +#![no_main] + +use cortex_m_rt::entry; +use cortex_m_semihosting::debug; + +use defmt_semihosting as _; // global logger + +#[entry] +fn main() -> ! { + let x = 42; + defmt::debug_assert_eq!(x - 1, x + 1, "dev"); + defmt::assert_eq!(x - 1, x + 1, "release"); + defmt::unreachable!(); +} + +// like `panic-semihosting` but doesn't print to stdout (that would corrupt the defmt stream) +#[cfg(target_os = "none")] +#[panic_handler] +fn panic(_: &core::panic::PanicInfo) -> ! { + loop { + debug::exit(debug::EXIT_SUCCESS) + } +} diff --git a/firmware/qemu/src/bin/assert-ne.out b/firmware/qemu/src/bin/assert-ne.out new file mode 100644 index 00000000..fd779a00 --- /dev/null +++ b/firmware/qemu/src/bin/assert-ne.out @@ -0,0 +1,3 @@ +0.000000 ERROR panicked at 'assertion failed: `(left != right)`: dev' + left: `42` +right: `42` diff --git a/firmware/qemu/src/bin/assert-ne.release.out b/firmware/qemu/src/bin/assert-ne.release.out new file mode 100644 index 00000000..d706306c --- /dev/null +++ b/firmware/qemu/src/bin/assert-ne.release.out @@ -0,0 +1,3 @@ +0.000000 ERROR panicked at 'assertion failed: `(left != right)`: release' + left: `42` +right: `42` diff --git a/firmware/qemu/src/bin/assert-ne.rs b/firmware/qemu/src/bin/assert-ne.rs new file mode 100644 index 00000000..5686c327 --- /dev/null +++ b/firmware/qemu/src/bin/assert-ne.rs @@ -0,0 +1,24 @@ +#![no_std] +#![no_main] + +use cortex_m_rt::entry; +use cortex_m_semihosting::debug; + +use defmt_semihosting as _; // global logger + +#[entry] +fn main() -> ! { + let x = 42; + defmt::debug_assert_ne!(x, x, "dev"); + defmt::assert_ne!(x, x, "release"); + defmt::unreachable!(); +} + +// like `panic-semihosting` but doesn't print to stdout (that would corrupt the defmt stream) +#[cfg(target_os = "none")] +#[panic_handler] +fn panic(_: &core::panic::PanicInfo) -> ! { + loop { + debug::exit(debug::EXIT_SUCCESS) + } +} diff --git a/firmware/qemu/src/bin/assert.out b/firmware/qemu/src/bin/assert.out new file mode 100644 index 00000000..c725b0cd --- /dev/null +++ b/firmware/qemu/src/bin/assert.out @@ -0,0 +1 @@ +0.000000 ERROR panicked at 'assertion failed: dev' diff --git a/firmware/qemu/src/bin/assert.release.out b/firmware/qemu/src/bin/assert.release.out new file mode 100644 index 00000000..9cee8e99 --- /dev/null +++ b/firmware/qemu/src/bin/assert.release.out @@ -0,0 +1 @@ +0.000000 ERROR panicked at 'assertion failed: release' diff --git a/firmware/qemu/src/bin/assert.rs b/firmware/qemu/src/bin/assert.rs new file mode 100644 index 00000000..ee9af8d8 --- /dev/null +++ b/firmware/qemu/src/bin/assert.rs @@ -0,0 +1,25 @@ +#![no_std] +#![no_main] + +use cortex_m_rt::entry; +use cortex_m_semihosting::debug; + +use defmt_semihosting as _; // global logger + +#[entry] +fn main() -> ! { + let dev = false; + let release = false; + defmt::debug_assert!(dev); + defmt::assert!(release); + defmt::unreachable!(); +} + +// like `panic-semihosting` but doesn't print to stdout (that would corrupt the defmt stream) +#[cfg(target_os = "none")] +#[panic_handler] +fn panic(_: &core::panic::PanicInfo) -> ! { + loop { + debug::exit(debug::EXIT_SUCCESS) + } +} diff --git a/firmware/qemu/src/bin/panic.out b/firmware/qemu/src/bin/panic.out new file mode 100644 index 00000000..c67a5e08 --- /dev/null +++ b/firmware/qemu/src/bin/panic.out @@ -0,0 +1 @@ +0.000000 ERROR panicked at 'The answer is 42' diff --git a/firmware/qemu/src/bin/panic.release.out b/firmware/qemu/src/bin/panic.release.out new file mode 100644 index 00000000..c67a5e08 --- /dev/null +++ b/firmware/qemu/src/bin/panic.release.out @@ -0,0 +1 @@ +0.000000 ERROR panicked at 'The answer is 42' diff --git a/firmware/qemu/src/bin/panic.rs b/firmware/qemu/src/bin/panic.rs new file mode 100644 index 00000000..0bac55bd --- /dev/null +++ b/firmware/qemu/src/bin/panic.rs @@ -0,0 +1,21 @@ +#![no_std] +#![no_main] + +use cortex_m_rt::entry; +use cortex_m_semihosting::debug; + +use defmt_semihosting as _; // global logger + +#[entry] +fn main() -> ! { + defmt::panic!("The answer is {:?}", 42); +} + +// like `panic-semihosting` but doesn't print to stdout (that would corrupt the defmt stream) +#[cfg(target_os = "none")] +#[panic_handler] +fn panic(_: &core::panic::PanicInfo) -> ! { + loop { + debug::exit(debug::EXIT_SUCCESS) + } +} diff --git a/firmware/qemu/test.sh b/firmware/qemu/test.sh index 3638e962..5c675241 100755 --- a/firmware/qemu/test.sh +++ b/firmware/qemu/test.sh @@ -5,12 +5,14 @@ set -o errexit function test() { local bin=$1 local features=${2-,} - + cargo rb "$bin" --features "$features" | tee "src/bin/$bin.out.new" | diff "src/bin/$bin.out" - cargo rrb "$bin" --features "$features" | tee "src/bin/$bin.release.out.new" | diff "src/bin/$bin.release.out" - } test "log" +test "panic" +test "assert" if rustc -V | grep nightly; then test "alloc" "alloc" fi From df1424ced6177bfcf22f293095d6d68e7a170869 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 14:56:56 +0100 Subject: [PATCH 15/20] hardwire export::panic to panic! on x86 for testing --- src/export.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/export.rs b/src/export.rs index 9f177049..b04dafaa 100644 --- a/src/export.rs +++ b/src/export.rs @@ -52,12 +52,12 @@ pub fn release(fmt: Formatter) { unsafe { _defmt_release(fmt) } } +/// For testing purposes #[cfg(target_arch = "x86_64")] pub fn timestamp() -> u64 { (T.with(|i| i.fetch_add(1, core::sync::atomic::Ordering::Relaxed)) & 0x7f) as u64 } -/// For testing purposes #[cfg(not(target_arch = "x86_64"))] pub fn timestamp() -> u64 { extern "Rust" { @@ -147,6 +147,13 @@ pub fn truncate(x: impl sealed::Truncate) -> T { x.truncate() } +/// For testing purposes +#[cfg(target_arch = "x86_64")] +pub fn panic() -> ! { + panic!() +} + +#[cfg(not(target_arch = "x86_64"))] pub fn panic() -> ! { extern "Rust" { fn _defmt_panic() -> !; From 9d066055593707cac6bf270a27e22062e3c92c04 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 15:03:45 +0100 Subject: [PATCH 16/20] fix x86 build warnings --- firmware/qemu/src/bin/assert-eq.rs | 3 +-- firmware/qemu/src/bin/assert-ne.rs | 3 +-- firmware/qemu/src/bin/assert.rs | 3 +-- firmware/qemu/src/bin/panic.rs | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/firmware/qemu/src/bin/assert-eq.rs b/firmware/qemu/src/bin/assert-eq.rs index c2fcd749..d5963d6c 100644 --- a/firmware/qemu/src/bin/assert-eq.rs +++ b/firmware/qemu/src/bin/assert-eq.rs @@ -2,7 +2,6 @@ #![no_main] use cortex_m_rt::entry; -use cortex_m_semihosting::debug; use defmt_semihosting as _; // global logger @@ -19,6 +18,6 @@ fn main() -> ! { #[panic_handler] fn panic(_: &core::panic::PanicInfo) -> ! { loop { - debug::exit(debug::EXIT_SUCCESS) + cortex_m_semihosting::debug::exit(debug::EXIT_SUCCESS) } } diff --git a/firmware/qemu/src/bin/assert-ne.rs b/firmware/qemu/src/bin/assert-ne.rs index 5686c327..b101cc43 100644 --- a/firmware/qemu/src/bin/assert-ne.rs +++ b/firmware/qemu/src/bin/assert-ne.rs @@ -2,7 +2,6 @@ #![no_main] use cortex_m_rt::entry; -use cortex_m_semihosting::debug; use defmt_semihosting as _; // global logger @@ -19,6 +18,6 @@ fn main() -> ! { #[panic_handler] fn panic(_: &core::panic::PanicInfo) -> ! { loop { - debug::exit(debug::EXIT_SUCCESS) + cortex_m_semihosting::debug::exit(debug::EXIT_SUCCESS) } } diff --git a/firmware/qemu/src/bin/assert.rs b/firmware/qemu/src/bin/assert.rs index ee9af8d8..8995cd80 100644 --- a/firmware/qemu/src/bin/assert.rs +++ b/firmware/qemu/src/bin/assert.rs @@ -2,7 +2,6 @@ #![no_main] use cortex_m_rt::entry; -use cortex_m_semihosting::debug; use defmt_semihosting as _; // global logger @@ -20,6 +19,6 @@ fn main() -> ! { #[panic_handler] fn panic(_: &core::panic::PanicInfo) -> ! { loop { - debug::exit(debug::EXIT_SUCCESS) + cortex_m_semihosting::debug::exit(debug::EXIT_SUCCESS) } } diff --git a/firmware/qemu/src/bin/panic.rs b/firmware/qemu/src/bin/panic.rs index 0bac55bd..890b7706 100644 --- a/firmware/qemu/src/bin/panic.rs +++ b/firmware/qemu/src/bin/panic.rs @@ -2,7 +2,6 @@ #![no_main] use cortex_m_rt::entry; -use cortex_m_semihosting::debug; use defmt_semihosting as _; // global logger @@ -16,6 +15,6 @@ fn main() -> ! { #[panic_handler] fn panic(_: &core::panic::PanicInfo) -> ! { loop { - debug::exit(debug::EXIT_SUCCESS) + cortex_m_semihosting::debug::exit(debug::EXIT_SUCCESS) } } From 0945a360e705c076638321176195900d50d9d51c Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 15:27:19 +0100 Subject: [PATCH 17/20] single workspace work arounds --- firmware/qemu/Cargo.toml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/firmware/qemu/Cargo.toml b/firmware/qemu/Cargo.toml index db3c8c6e..bab2bb25 100644 --- a/firmware/qemu/Cargo.toml +++ b/firmware/qemu/Cargo.toml @@ -24,6 +24,22 @@ defmt-error = [] # log at ERROR level default = ["defmt-default"] alloc = ["defmt/alloc", "alloc-cortex-m"] +[[bin]] +name = "assert" +test = false + +[[bin]] +name = "assert-eq" +test = false + +[[bin]] +name = "assert-ne" +test = false + +[[bin]] +name = "panic" +test = false + [[bin]] name = "log" test = false From 1ffc47a05f44bda8222abbeb2e0d6214ddf233b4 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 15:34:34 +0100 Subject: [PATCH 18/20] fix imports --- firmware/qemu/src/bin/assert-eq.rs | 4 +++- firmware/qemu/src/bin/assert-ne.rs | 4 +++- firmware/qemu/src/bin/assert.rs | 4 +++- firmware/qemu/src/bin/panic.rs | 4 +++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/firmware/qemu/src/bin/assert-eq.rs b/firmware/qemu/src/bin/assert-eq.rs index d5963d6c..0aa88bae 100644 --- a/firmware/qemu/src/bin/assert-eq.rs +++ b/firmware/qemu/src/bin/assert-eq.rs @@ -17,7 +17,9 @@ fn main() -> ! { #[cfg(target_os = "none")] #[panic_handler] fn panic(_: &core::panic::PanicInfo) -> ! { + use cortex_m_semihosting::debug; + loop { - cortex_m_semihosting::debug::exit(debug::EXIT_SUCCESS) + debug::exit(debug::EXIT_SUCCESS) } } diff --git a/firmware/qemu/src/bin/assert-ne.rs b/firmware/qemu/src/bin/assert-ne.rs index b101cc43..82cb4423 100644 --- a/firmware/qemu/src/bin/assert-ne.rs +++ b/firmware/qemu/src/bin/assert-ne.rs @@ -17,7 +17,9 @@ fn main() -> ! { #[cfg(target_os = "none")] #[panic_handler] fn panic(_: &core::panic::PanicInfo) -> ! { + use cortex_m_semihosting::debug; + loop { - cortex_m_semihosting::debug::exit(debug::EXIT_SUCCESS) + debug::exit(debug::EXIT_SUCCESS) } } diff --git a/firmware/qemu/src/bin/assert.rs b/firmware/qemu/src/bin/assert.rs index 8995cd80..8664f1ee 100644 --- a/firmware/qemu/src/bin/assert.rs +++ b/firmware/qemu/src/bin/assert.rs @@ -18,7 +18,9 @@ fn main() -> ! { #[cfg(target_os = "none")] #[panic_handler] fn panic(_: &core::panic::PanicInfo) -> ! { + use cortex_m_semihosting::debug; + loop { - cortex_m_semihosting::debug::exit(debug::EXIT_SUCCESS) + debug::exit(debug::EXIT_SUCCESS) } } diff --git a/firmware/qemu/src/bin/panic.rs b/firmware/qemu/src/bin/panic.rs index 890b7706..9e83a01d 100644 --- a/firmware/qemu/src/bin/panic.rs +++ b/firmware/qemu/src/bin/panic.rs @@ -14,7 +14,9 @@ fn main() -> ! { #[cfg(target_os = "none")] #[panic_handler] fn panic(_: &core::panic::PanicInfo) -> ! { + use cortex_m_semihosting::debug; + loop { - cortex_m_semihosting::debug::exit(debug::EXIT_SUCCESS) + debug::exit(debug::EXIT_SUCCESS) } } From 7cb0ebaaf9e8f3f152fe20f340b6041f4ee46752 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 19 Nov 2020 15:42:39 +0100 Subject: [PATCH 19/20] simplify assert_ne output instead of printing 'left' and 'right' lines (which will have the same content) print a single line --- firmware/qemu/src/bin/assert-ne.out | 3 +- firmware/qemu/src/bin/assert-ne.release.out | 3 +- firmware/qemu/test.sh | 2 + macros/src/lib.rs | 57 ++++++++++++--------- 4 files changed, 38 insertions(+), 27 deletions(-) diff --git a/firmware/qemu/src/bin/assert-ne.out b/firmware/qemu/src/bin/assert-ne.out index fd779a00..34238191 100644 --- a/firmware/qemu/src/bin/assert-ne.out +++ b/firmware/qemu/src/bin/assert-ne.out @@ -1,3 +1,2 @@ 0.000000 ERROR panicked at 'assertion failed: `(left != right)`: dev' - left: `42` -right: `42` +left/right: `42` diff --git a/firmware/qemu/src/bin/assert-ne.release.out b/firmware/qemu/src/bin/assert-ne.release.out index d706306c..7afb9a46 100644 --- a/firmware/qemu/src/bin/assert-ne.release.out +++ b/firmware/qemu/src/bin/assert-ne.release.out @@ -1,3 +1,2 @@ 0.000000 ERROR panicked at 'assertion failed: `(left != right)`: release' - left: `42` -right: `42` +left/right: `42` diff --git a/firmware/qemu/test.sh b/firmware/qemu/test.sh index 5c675241..aed3c47b 100755 --- a/firmware/qemu/test.sh +++ b/firmware/qemu/test.sh @@ -13,6 +13,8 @@ function test() { test "log" test "panic" test "assert" +test "assert-eq" +test "assert-ne" if rustc -V | grep nightly; then test "alloc" "alloc" fi diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 90906da5..a8aa4d01 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -1,7 +1,6 @@ //! INTERNAL; DO NOT USE. Please use the `defmt` crate to access the functionality implemented here mod symbol; -use core::fmt; use core::fmt::Write as _; use proc_macro::TokenStream; @@ -618,15 +617,6 @@ enum BinOp { Ne, } -impl fmt::Display for BinOp { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str(match self { - BinOp::Eq => "==", - BinOp::Ne => "!=", - }) - } -} - // not naming this `assert_eq` to avoid shadowing `core::assert_eq` in this scope #[proc_macro] pub fn assert_eq_(ts: TokenStream) -> TokenStream { @@ -657,7 +647,12 @@ fn assert_binop(ts: TokenStream, binop: BinOp) -> TokenStream { String::new() }; - for val in &["left_val", "right_val"] { + let vals = match binop { + BinOp::Eq => &["left_val", "right_val"][..], + BinOp::Ne => &["left_val"][..], + }; + + for val in vals { let mut segments = Punctuated::new(); segments.push(PathSegment { ident: Ident2::new(*val, Span2::call_site()), @@ -674,21 +669,37 @@ fn assert_binop(ts: TokenStream, binop: BinOp) -> TokenStream { })); } - let log_stmt = log( - Level::Error, - FormatArgs { - litstr: LitStr::new( - &format!( - "panicked at 'assertion failed: `(left {} right)`{}' + let log_stmt = match binop { + BinOp::Eq => log( + Level::Error, + FormatArgs { + litstr: LitStr::new( + &format!( + "panicked at 'assertion failed: `(left == right)`{}' left: `{{:?}}` right: `{{:?}}`", - binop, extra_string + extra_string + ), + Span2::call_site(), ), - Span2::call_site(), - ), - rest: Some((syn::token::Comma::default(), log_args)), - }, - ); + rest: Some((syn::token::Comma::default(), log_args)), + }, + ), + BinOp::Ne => log( + Level::Error, + FormatArgs { + litstr: LitStr::new( + &format!( + "panicked at 'assertion failed: `(left != right)`{}' +left/right: `{{:?}}`", + extra_string + ), + Span2::call_site(), + ), + rest: Some((syn::token::Comma::default(), log_args)), + }, + ), + }; let mut cond = quote!(*left_val == *right_val); if binop == BinOp::Eq { From 7957f9d9c1f3153c21eb03dfe83c37a973aa7b26 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Mon, 23 Nov 2020 13:48:01 +0000 Subject: [PATCH 20/20] Update book/src/panic.md Co-authored-by: Jonas Schievink --- book/src/panic.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/book/src/panic.md b/book/src/panic.md index 662baa18..b7870fe0 100644 --- a/book/src/panic.md +++ b/book/src/panic.md @@ -14,7 +14,7 @@ This is the case if you use [`panic-probe`] with the `print-defmt` feature enabl To avoid this issue you can use the `#[defmt::panic_handler]` to *override* the panicking behavior of `defmt::panic`-like and `defmt::assert`-like macros. This attribute must be placed on a function with signature `fn() -> !`. -In this function you'll want to replicate the panicking behavior of `#[core::panic_handler]` but leave out the part that prints the panic message. +In this function you'll want to replicate the panicking behavior of the Rust `#[panic_handler]` but leave out the part that prints the panic message. For example: