Merge #293

293: make defmt attributes forward input attributes r=Lotterleben a=japaric

work towards fixing #252 

- [x] reject {panic_handler,timestamp} + ( `#[export_name]` OR `#[no_mangle]`)

(I was expecting `#[timestamp] #[inline(always)] fn foo() -> u64 { 0 }` to fail at link time but it works ... perhaps export_name undoes inlining?)



Co-authored-by: Jorge Aparicio <jorge.aparicio@ferrous-systems.com>

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 = [
   ".",

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

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

macros/src/lib.rs

@@ -12,11 +12,37 @@ 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,
 };
 
+/// 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],
+) -> parse::Result<()> {
+    for attr in attrs_to_check {
+        if let Some(ident) = attr.path.get_ident() {
+            let ident = ident.to_string();
+            if reject_list.contains(&&*ident) {
+                return Err(parse::Error::new(
+                    attr.span(),
+                    format!(
+                        "`#[{}]` attribute cannot be used together with `#[{}]`",
+                        attr_name, ident
+                    ),
+                ));
+            }
+        }
+    }
+
+    Ok(())
+}
+
 #[proc_macro_attribute]
 pub fn global_logger(args: TokenStream, input: TokenStream) -> TokenStream {
     if !args.is_empty() {
@@ -39,8 +65,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]
@@ -92,8 +120,14 @@ pub fn panic_handler(args: TokenStream, input: TokenStream) -> TokenStream {
             .into();
     }
 
+    let attrs = &f.attrs;
+    if let Err(e) = check_attribute_conflicts("panic_handler", attrs, &["export_name", "no_mangle"])
+    {
+        return e.to_compile_error().into();
+    }
     let block = &f.block;
     quote!(
+        #(#attrs)*
         #[export_name = "_defmt_panic"]
         fn #ident() -> ! {
             #block
@@ -138,9 +172,14 @@ pub fn timestamp(args: TokenStream, input: TokenStream) -> TokenStream {
             .into();
     }
 
+    let attrs = &f.attrs;
+    if let Err(e) = check_attribute_conflicts("timestamp", attrs, &["export_name", "no_mangle"]) {
+        return e.to_compile_error().into();
+    }
     let block = &f.block;
     quote!(
         #[export_name = "_defmt_timestamp"]
+        #(#attrs)*
         fn #ident() -> u64 {
             #block
         }

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

tests/ui.rs

@@ -0,0 +1,5 @@
+#[test]
+fn ui() {
+    let t = trybuild::TestCases::new();
+    t.compile_fail("tests/ui/*.rs");
+}

tests/ui/panic-handler-export-name.rs

@@ -0,0 +1,8 @@
+#![no_main]
+#![no_std]
+
+#[defmt::panic_handler]
+#[export_name = "hello"]
+fn foo() -> ! {
+    loop {}
+}

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"]
+  | ^^^^^^^^^^^^^^^^^^^^^^^^

tests/ui/panic-handler-no-mangle.rs

@@ -0,0 +1,8 @@
+#![no_main]
+#![no_std]
+
+#[defmt::panic_handler]
+#[no_mangle]
+fn panic() -> ! {
+    loop {}
+}

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]
+  | ^^^^^^^^^^^^

tests/ui/timestamp-export-name.rs

@@ -0,0 +1,8 @@
+#![no_main]
+#![no_std]
+
+#[defmt::timestamp]
+#[export_name = "hello"]
+fn foo() -> u64 {
+    0
+}

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"]
+  | ^^^^^^^^^^^^^^^^^^^^^^^^

tests/ui/timestamp-no-mangle.rs

@@ -0,0 +1,8 @@
+#![no_main]
+#![no_std]
+
+#[defmt::timestamp]
+#[no_mangle]
+fn foo() -> u64 {
+    0
+}

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]
+  | ^^^^^^^^^^^^