From 30b019230475087ed1f8aea691496405f33bf5d2 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Mon, 30 Nov 2020 18:50:51 +0100 Subject: [PATCH 1/6] make defmt attributes forward input attributes --- macros/src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index fc0b2dd5..901e9a9e 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -42,8 +42,10 @@ pub fn global_logger(args: TokenStream, input: TokenStream) -> TokenStream { .into(); } + let attrs = &s.attrs; let vis = &s.vis; quote!( + #(#attrs)* #vis struct #ident; #[no_mangle] @@ -95,8 +97,10 @@ pub fn panic_handler(args: TokenStream, input: TokenStream) -> TokenStream { .into(); } + let attrs = &f.attrs; let block = &f.block; quote!( + #(#attrs)* #[export_name = "_defmt_panic"] fn #ident() -> ! { #block @@ -141,9 +145,11 @@ pub fn timestamp(args: TokenStream, input: TokenStream) -> TokenStream { .into(); } + let attrs = &f.attrs; let block = &f.block; quote!( #[export_name = "_defmt_timestamp"] + #(#attrs)* fn #ident() -> u64 { #block } From 57d3ed1401fcbaa8e4872f360b9cc30ecfa78c8b Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Tue, 1 Dec 2020 18:27:22 +0100 Subject: [PATCH 2/6] reject incompatible attributes --- macros/src/lib.rs | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 901e9a9e..892580f7 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -12,11 +12,34 @@ use syn::{ parse_macro_input, punctuated::Punctuated, spanned::Spanned as _, - Data, DeriveInput, Expr, ExprPath, Fields, FieldsNamed, FieldsUnnamed, GenericParam, ItemFn, - ItemStruct, LitStr, Path, PathArguments, PathSegment, ReturnType, Token, Type, WhereClause, - WherePredicate, + Attribute, Data, DeriveInput, Expr, ExprPath, Fields, FieldsNamed, FieldsUnnamed, GenericParam, + ItemFn, ItemStruct, LitStr, Path, PathArguments, PathSegment, ReturnType, Token, Type, + WhereClause, WherePredicate, }; +fn reject_attributes( + which_attr: &str, + attrs: &[Attribute], + block_list: &[&str], +) -> parse::Result<()> { + for attr in attrs { + if let Some(ident) = attr.path.get_ident() { + let ident = ident.to_string(); + if block_list.contains(&&*ident) { + return Err(parse::Error::new( + attr.span(), + format!( + "`#[{}]` attribute cannot be used together with `#[{}]`", + which_attr, ident + ), + )); + } + } + } + + Ok(()) +} + #[proc_macro_attribute] pub fn global_logger(args: TokenStream, input: TokenStream) -> TokenStream { if !args.is_empty() { @@ -98,6 +121,9 @@ pub fn panic_handler(args: TokenStream, input: TokenStream) -> TokenStream { } let attrs = &f.attrs; + if let Err(e) = reject_attributes("panic_handler", attrs, &["export_name"]) { + return e.to_compile_error().into(); + } let block = &f.block; quote!( #(#attrs)* @@ -146,6 +172,9 @@ pub fn timestamp(args: TokenStream, input: TokenStream) -> TokenStream { } let attrs = &f.attrs; + if let Err(e) = reject_attributes("timestamp", attrs, &["export_name"]) { + return e.to_compile_error().into(); + } let block = &f.block; quote!( #[export_name = "_defmt_timestamp"] From 9dfda028c5ef3396edea24794445506cc4e7269d Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Tue, 15 Dec 2020 12:36:51 +0100 Subject: [PATCH 3/6] also reject `#[no_mangle]` which is sugar for `#[export_name = "$item_name"] --- macros/src/lib.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 892580f7..62bd63d1 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -18,19 +18,20 @@ use syn::{ }; fn reject_attributes( - which_attr: &str, - attrs: &[Attribute], - block_list: &[&str], + // appears in the error message + attr_name: &str, + attrs_to_check: &[Attribute], + reject_list: &[&str], ) -> parse::Result<()> { - for attr in attrs { + for attr in attrs_to_check { if let Some(ident) = attr.path.get_ident() { let ident = ident.to_string(); - if block_list.contains(&&*ident) { + if reject_list.contains(&&*ident) { return Err(parse::Error::new( attr.span(), format!( "`#[{}]` attribute cannot be used together with `#[{}]`", - which_attr, ident + attr_name, ident ), )); } @@ -121,7 +122,7 @@ pub fn panic_handler(args: TokenStream, input: TokenStream) -> TokenStream { } let attrs = &f.attrs; - if let Err(e) = reject_attributes("panic_handler", attrs, &["export_name"]) { + if let Err(e) = reject_attributes("panic_handler", attrs, &["export_name", "no_mangle"]) { return e.to_compile_error().into(); } let block = &f.block; @@ -172,7 +173,7 @@ pub fn timestamp(args: TokenStream, input: TokenStream) -> TokenStream { } let attrs = &f.attrs; - if let Err(e) = reject_attributes("timestamp", attrs, &["export_name"]) { + if let Err(e) = reject_attributes("timestamp", attrs, &["export_name", "no_mangle"]) { return e.to_compile_error().into(); } let block = &f.block; From 8da13fe8c26574ed377e7a53856b21f05aa3c5fe Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Tue, 15 Dec 2020 12:42:28 +0100 Subject: [PATCH 4/6] add compile fail tests --- Cargo.toml | 3 +++ tests/ui.rs | 5 +++++ tests/ui/panic-handler-export-name.rs | 8 ++++++++ tests/ui/panic-handler-export-name.stderr | 5 +++++ tests/ui/panic-handler-no-mangle.rs | 8 ++++++++ tests/ui/panic-handler-no-mangle.stderr | 5 +++++ tests/ui/timestamp-export-name.rs | 8 ++++++++ tests/ui/timestamp-export-name.stderr | 5 +++++ tests/ui/timestamp-no-mangle.rs | 8 ++++++++ tests/ui/timestamp-no-mangle.stderr | 5 +++++ 10 files changed, 60 insertions(+) create mode 100644 tests/ui.rs create mode 100644 tests/ui/panic-handler-export-name.rs create mode 100644 tests/ui/panic-handler-export-name.stderr create mode 100644 tests/ui/panic-handler-no-mangle.rs create mode 100644 tests/ui/panic-handler-no-mangle.stderr create mode 100644 tests/ui/timestamp-export-name.rs create mode 100644 tests/ui/timestamp-export-name.stderr create mode 100644 tests/ui/timestamp-no-mangle.rs create mode 100644 tests/ui/timestamp-no-mangle.stderr diff --git a/Cargo.toml b/Cargo.toml index 62df82ec..522d774e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,9 @@ unstable-test = ["defmt-macros/unstable-test"] defmt-macros = { path = "macros", version = "0.1.1" } heapless = "0.5.6" +[dev-dependencies] +trybuild = "1.0.37" + [workspace] members = [ ".", diff --git a/tests/ui.rs b/tests/ui.rs new file mode 100644 index 00000000..870c2f95 --- /dev/null +++ b/tests/ui.rs @@ -0,0 +1,5 @@ +#[test] +fn ui() { + let t = trybuild::TestCases::new(); + t.compile_fail("tests/ui/*.rs"); +} diff --git a/tests/ui/panic-handler-export-name.rs b/tests/ui/panic-handler-export-name.rs new file mode 100644 index 00000000..49c1031c --- /dev/null +++ b/tests/ui/panic-handler-export-name.rs @@ -0,0 +1,8 @@ +#![no_main] +#![no_std] + +#[defmt::panic_handler] +#[export_name = "hello"] +fn foo() -> ! { + loop {} +} diff --git a/tests/ui/panic-handler-export-name.stderr b/tests/ui/panic-handler-export-name.stderr new file mode 100644 index 00000000..b276f9b0 --- /dev/null +++ b/tests/ui/panic-handler-export-name.stderr @@ -0,0 +1,5 @@ +error: `#[panic_handler]` attribute cannot be used together with `#[export_name]` + --> $DIR/panic-handler-export-name.rs:5:1 + | +5 | #[export_name = "hello"] + | ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/panic-handler-no-mangle.rs b/tests/ui/panic-handler-no-mangle.rs new file mode 100644 index 00000000..8eb8410e --- /dev/null +++ b/tests/ui/panic-handler-no-mangle.rs @@ -0,0 +1,8 @@ +#![no_main] +#![no_std] + +#[defmt::panic_handler] +#[no_mangle] +fn panic() -> ! { + loop {} +} diff --git a/tests/ui/panic-handler-no-mangle.stderr b/tests/ui/panic-handler-no-mangle.stderr new file mode 100644 index 00000000..2563973c --- /dev/null +++ b/tests/ui/panic-handler-no-mangle.stderr @@ -0,0 +1,5 @@ +error: `#[panic_handler]` attribute cannot be used together with `#[no_mangle]` + --> $DIR/panic-handler-no-mangle.rs:5:1 + | +5 | #[no_mangle] + | ^^^^^^^^^^^^ diff --git a/tests/ui/timestamp-export-name.rs b/tests/ui/timestamp-export-name.rs new file mode 100644 index 00000000..b68fa1ab --- /dev/null +++ b/tests/ui/timestamp-export-name.rs @@ -0,0 +1,8 @@ +#![no_main] +#![no_std] + +#[defmt::timestamp] +#[export_name = "hello"] +fn foo() -> u64 { + 0 +} diff --git a/tests/ui/timestamp-export-name.stderr b/tests/ui/timestamp-export-name.stderr new file mode 100644 index 00000000..f72abff5 --- /dev/null +++ b/tests/ui/timestamp-export-name.stderr @@ -0,0 +1,5 @@ +error: `#[timestamp]` attribute cannot be used together with `#[export_name]` + --> $DIR/timestamp-export-name.rs:5:1 + | +5 | #[export_name = "hello"] + | ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/timestamp-no-mangle.rs b/tests/ui/timestamp-no-mangle.rs new file mode 100644 index 00000000..ee5b091b --- /dev/null +++ b/tests/ui/timestamp-no-mangle.rs @@ -0,0 +1,8 @@ +#![no_main] +#![no_std] + +#[defmt::timestamp] +#[no_mangle] +fn foo() -> u64 { + 0 +} diff --git a/tests/ui/timestamp-no-mangle.stderr b/tests/ui/timestamp-no-mangle.stderr new file mode 100644 index 00000000..5519e390 --- /dev/null +++ b/tests/ui/timestamp-no-mangle.stderr @@ -0,0 +1,5 @@ +error: `#[timestamp]` attribute cannot be used together with `#[no_mangle]` + --> $DIR/timestamp-no-mangle.rs:5:1 + | +5 | #[no_mangle] + | ^^^^^^^^^^^^ From 95e091421023e627ccb483971d1e418f11d79459 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Tue, 15 Dec 2020 17:02:30 +0100 Subject: [PATCH 5/6] document conflicts --- book/src/panic.md | 4 ++++ book/src/timestamp.md | 4 ++++ src/lib.rs | 8 ++++++++ 3 files changed, 16 insertions(+) diff --git a/book/src/panic.md b/book/src/panic.md index 2f2c4db1..a0463961 100644 --- a/book/src/panic.md +++ b/book/src/panic.md @@ -38,3 +38,7 @@ fn defmt_panic() -> ! { 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). + +## Inter-operation with built-in attributes + +The `#[panic_handler]` attribute cannot be used together with the `export_name` or `no_mangle` attributes diff --git a/book/src/timestamp.md b/book/src/timestamp.md index dda6d8e8..feebf199 100644 --- a/book/src/timestamp.md +++ b/book/src/timestamp.md @@ -96,3 +96,7 @@ count: u64 <- (high1 << 32) | low ``` The loop should be kept as tight as possible and the read operations must be single-instruction operations. + +## Inter-operation with built-in attributes + +The `#[timestamp]` attribute cannot be used together with the `export_name` or `no_mangle` attributes diff --git a/src/lib.rs b/src/lib.rs index 981225be..31ea56ab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -160,6 +160,10 @@ pub use defmt_macros::unwrap; /// this macro. See [the manual] for details. /// /// [the manual]: https://defmt.ferrous-systems.com/panic.html +/// +/// # Inter-operation with built-in attributes +/// +/// This attribute cannot be used together with the `export_name` or `no_mangle` attributes pub use defmt_macros::panic_handler; /// Creates an interned string ([`Str`]) from a string literal. @@ -260,6 +264,10 @@ pub use defmt_macros::global_logger; /// 0 /// } /// ``` +/// +/// # Inter-operation with built-in attributes +/// +/// This attribute cannot be used together with the `export_name` or `no_mangle` attributes pub use defmt_macros::timestamp; #[doc(hidden)] // documented as the `Format` trait instead From 659442e645dcb499a1c7bd2915a3ba5c20de260e Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Tue, 15 Dec 2020 17:06:17 +0100 Subject: [PATCH 6/6] document new helper function --- macros/src/lib.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 62bd63d1..86986275 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -17,8 +17,10 @@ use syn::{ WhereClause, WherePredicate, }; -fn reject_attributes( - // appears in the error message +/// Checks if any attribute in `attrs_to_check` is in `reject_list` and returns a compiler error if there's a match +/// +/// The compiler error will indicate that the attribute conflicts with `attr_name` +fn check_attribute_conflicts( attr_name: &str, attrs_to_check: &[Attribute], reject_list: &[&str], @@ -122,7 +124,8 @@ pub fn panic_handler(args: TokenStream, input: TokenStream) -> TokenStream { } let attrs = &f.attrs; - if let Err(e) = reject_attributes("panic_handler", attrs, &["export_name", "no_mangle"]) { + if let Err(e) = check_attribute_conflicts("panic_handler", attrs, &["export_name", "no_mangle"]) + { return e.to_compile_error().into(); } let block = &f.block; @@ -173,7 +176,7 @@ pub fn timestamp(args: TokenStream, input: TokenStream) -> TokenStream { } let attrs = &f.attrs; - if let Err(e) = reject_attributes("timestamp", attrs, &["export_name", "no_mangle"]) { + if let Err(e) = check_attribute_conflicts("timestamp", attrs, &["export_name", "no_mangle"]) { return e.to_compile_error().into(); } let block = &f.block;