Merge #158

158: implement RFC #128: #[interrupt(binds = ..)] r=korken89 a=japaric

closes #128 

r? @korken89 or @TeXitoi 

suggestions for tests are welcome! (2 of the 3 tests I added hit bugs in my implementation)

Co-authored-by: Jorge Aparicio <jorge@japaric.io>

book/en/src/by-example/tips.md

@@ -75,3 +75,19 @@ $ cargo nm --example ramfunc --release | grep ' foo::'
 ``` console
 $ cargo nm --example ramfunc --release | grep ' bar::'
 {{#include ../../../../ci/expected/ramfunc.grep.bar}}```
+
+## `binds`
+
+**NOTE**: Requires RTFM ~0.4.2
+
+You can give hardware tasks more task-like names using the `binds` argument: you
+name the function as you wish and specify the name of the interrupt / exception
+in the `binds` argument. Types like `Spawn` will be placed in a module named
+after the function, not the interrupt / exception. Example below:
+
+``` rust
+{{#include ../../../../examples/binds.rs}}
+```
+``` console
+$ cargo run --example binds
+{{#include ../../../../ci/expected/binds.run}}```

ci/expected/binds.run

@@ -0,0 +1,4 @@
+init
+foo called 1 time
+idle
+foo called 2 times

ci/script.sh

@@ -93,6 +93,7 @@ main() {
                 idle
                 init
                 interrupt
+                binds
 
                 resource
                 lock

examples/binds.rs

@@ -0,0 +1,48 @@
+//! examples/binds.rs
+
+#![deny(unsafe_code)]
+#![deny(warnings)]
+#![no_main]
+#![no_std]
+
+extern crate panic_semihosting;
+
+use cortex_m_semihosting::{debug, hprintln};
+use lm3s6965::Interrupt;
+use rtfm::app;
+
+// `examples/interrupt.rs` rewritten to use `binds`
+#[app(device = lm3s6965)]
+const APP: () = {
+    #[init]
+    fn init() {
+        rtfm::pend(Interrupt::UART0);
+
+        hprintln!("init").unwrap();
+    }
+
+    #[idle]
+    fn idle() -> ! {
+        hprintln!("idle").unwrap();
+
+        rtfm::pend(Interrupt::UART0);
+
+        debug::exit(debug::EXIT_SUCCESS);
+
+        loop {}
+    }
+
+    #[interrupt(binds = UART0)]
+    fn foo() {
+        static mut TIMES: u32 = 0;
+
+        *TIMES += 1;
+
+        hprintln!(
+            "foo called {} time{}",
+            *TIMES,
+            if *TIMES > 1 { "s" } else { "" }
+        )
+        .unwrap();
+    }
+};

macros/src/check.rs

@@ -106,10 +106,12 @@ pub fn app(app: &App) -> parse::Result<()> {
     }
 
     // Check that free interrupts are not being used
-    for int in app.interrupts.keys() {
-        if app.free_interrupts.contains_key(int) {
+    for (handler, interrupt) in &app.interrupts {
+        let name = interrupt.args.binds(handler);
+
+        if app.free_interrupts.contains_key(name) {
             return Err(parse::Error::new(
-                int.span(),
+                name.span(),
                 "free interrupts (`extern { .. }`) can't be used as interrupt handlers",
             ));
         }

macros/src/codegen.rs

@@ -468,7 +468,8 @@ fn post_init(ctxt: &Context, app: &App, analysis: &Analysis) -> proc_macro2::Tok
     // the device into compile errors
     let device = &app.args.device;
     let nvic_prio_bits = quote!(#device::NVIC_PRIO_BITS);
-    for (name, exception) in &app.exceptions {
+    for (handler, exception) in &app.exceptions {
+        let name = exception.args.binds(handler);
         let priority = exception.args.priority;
         exprs.push(quote!(assert!(#priority <= (1 << #nvic_prio_bits))));
         exprs.push(quote!(p.SCB.set_priority(
@@ -1082,9 +1083,10 @@ fn exceptions(ctxt: &mut Context, app: &App, analysis: &Analysis) -> Vec<proc_ma
             let attrs = &exception.attrs;
             let stmts = &exception.stmts;
 
+            let kind = Kind::Exception(ident.clone());
             let prelude = prelude(
                 ctxt,
-                Kind::Exception(ident.clone()),
+                kind.clone(),
                 &exception.args.resources,
                 &exception.args.spawn,
                 &exception.args.schedule,
@@ -1095,7 +1097,7 @@ fn exceptions(ctxt: &mut Context, app: &App, analysis: &Analysis) -> Vec<proc_ma
 
             let module = module(
                 ctxt,
-                Kind::Exception(ident.clone()),
+                kind,
                 !exception.args.schedule.is_empty(),
                 !exception.args.spawn.is_empty(),
                 app,
@@ -1122,7 +1124,7 @@ fn exceptions(ctxt: &mut Context, app: &App, analysis: &Analysis) -> Vec<proc_ma
             };
 
             let locals = mk_locals(&exception.statics, false);
-            let symbol = ident.to_string();
+            let symbol = exception.args.binds(ident).to_string();
             let alias = ctxt.ident_gen.mk_ident(None, false);
             let unsafety = &exception.unsafety;
             quote!(
@@ -1162,14 +1164,14 @@ fn interrupts(
     let mut root = vec![];
     let mut scoped = vec![];
 
-    let device = &app.args.device;
     for (ident, interrupt) in &app.interrupts {
         let attrs = &interrupt.attrs;
         let stmts = &interrupt.stmts;
 
+        let kind = Kind::Interrupt(ident.clone());
         let prelude = prelude(
             ctxt,
-            Kind::Interrupt(ident.clone()),
+            kind.clone(),
             &interrupt.args.resources,
             &interrupt.args.spawn,
             &interrupt.args.schedule,
@@ -1180,7 +1182,7 @@ fn interrupts(
 
         root.push(module(
             ctxt,
-            Kind::Interrupt(ident.clone()),
+            kind,
             !interrupt.args.schedule.is_empty(),
             !interrupt.args.spawn.is_empty(),
             app,
@@ -1208,7 +1210,7 @@ fn interrupts(
 
         let locals = mk_locals(&interrupt.statics, false);
         let alias = ctxt.ident_gen.mk_ident(None, false);
-        let symbol = ident.to_string();
+        let symbol = interrupt.args.binds(ident).to_string();
         let unsafety = &interrupt.unsafety;
         scoped.push(quote!(
             // unsafe trampoline to deter end-users from calling this non-reentrant function
@@ -1217,9 +1219,6 @@ fn interrupts(
             unsafe fn #alias() {
                 #[inline(always)]
                 #unsafety fn interrupt() {
-                    // check that this interrupt exists
-                    let _ = #device::interrupt::#ident;
-
                     #(#locals)*
 
                     #baseline_let
@@ -1994,7 +1993,8 @@ fn pre_init(ctxt: &Context, app: &App, analysis: &Analysis) -> proc_macro2::Toke
     // the device into compile errors
     let device = &app.args.device;
     let nvic_prio_bits = quote!(#device::NVIC_PRIO_BITS);
-    for (name, interrupt) in &app.interrupts {
+    for (handler, interrupt) in &app.interrupts {
+        let name = interrupt.args.binds(handler);
         let priority = interrupt.args.priority;
         exprs.push(quote!(p.NVIC.enable(#device::Interrupt::#name);));
         exprs.push(quote!(assert!(#priority <= (1 << #nvic_prio_bits));));

macros/src/syntax.rs

@@ -699,6 +699,29 @@ impl Init {
     }
 }
 
+/// Union of `TaskArgs`, `ExceptionArgs` and `InterruptArgs`
+pub struct Args {
+    pub binds: Option<Ident>,
+    pub capacity: Option<u8>,
+    pub priority: u8,
+    pub resources: Idents,
+    pub schedule: Idents,
+    pub spawn: Idents,
+}
+
+impl Default for Args {
+    fn default() -> Self {
+        Args {
+            binds: None,
+            capacity: None,
+            priority: 1,
+            resources: Idents::new(),
+            schedule: Idents::new(),
+            spawn: Idents::new(),
+        }
+    }
+}
+
 pub struct Exception {
     pub args: ExceptionArgs,
     pub attrs: Vec<Attribute>,
@@ -708,16 +731,25 @@ pub struct Exception {
 }
 
 pub struct ExceptionArgs {
+    binds: Option<Ident>,
     pub priority: u8,
     pub resources: Idents,
     pub schedule: Idents,
     pub spawn: Idents,
 }
 
+impl ExceptionArgs {
+    /// Returns the name of the exception / interrupt this handler binds to
+    pub fn binds<'a>(&'a self, handler: &'a Ident) -> &'a Ident {
+        self.binds.as_ref().unwrap_or(handler)
+    }
+}
+
 impl Parse for ExceptionArgs {
     fn parse(input: ParseStream<'_>) -> parse::Result<Self> {
-        parse_args(input, false).map(
-            |TaskArgs {
+        parse_args(input, /* binds */ true, /* capacity */ false).map(
+            |Args {
+                 binds,
                  priority,
                  resources,
                  schedule,
@@ -725,6 +757,7 @@ impl Parse for ExceptionArgs {
                  ..
              }| {
                 ExceptionArgs {
+                    binds,
                     priority,
                     resources,
                     schedule,
@@ -755,7 +788,7 @@ impl Exception {
         }
 
         let span = item.ident.span();
-        match &*item.ident.to_string() {
+        match &*args.binds.as_ref().unwrap_or(&item.ident).to_string() {
             "MemoryManagement" | "BusFault" | "UsageFault" | "SecureFault" | "SVCall"
             | "DebugMonitor" | "PendSV" => {} // OK
             "SysTick" => {
@@ -893,30 +926,40 @@ pub struct TaskArgs {
     pub schedule: Idents,
 }
 
-impl Default for TaskArgs {
-    fn default() -> Self {
-        TaskArgs {
-            capacity: None,
-            priority: 1,
-            resources: Idents::new(),
-            schedule: Idents::new(),
-            spawn: Idents::new(),
-        }
-    }
-}
-
 impl Parse for TaskArgs {
     fn parse(input: ParseStream<'_>) -> parse::Result<Self> {
-        parse_args(input, true)
+        parse_args(input, /* binds */ false, /* capacity */ true).map(
+            |Args {
+                 capacity,
+                 priority,
+                 resources,
+                 schedule,
+                 spawn,
+                 ..
+             }| {
+                TaskArgs {
+                    capacity,
+                    priority,
+                    resources,
+                    schedule,
+                    spawn,
+                }
+            },
+        )
     }
 }
 
-// Parser shared by TaskArgs and ExceptionArgs / InterruptArgs
-fn parse_args(input: ParseStream<'_>, accept_capacity: bool) -> parse::Result<TaskArgs> {
+// Parser shared by ExceptionArgs, InterruptArgs and TaskArgs
+fn parse_args(
+    input: ParseStream<'_>,
+    accepts_binds: bool,
+    accepts_capacity: bool,
+) -> parse::Result<Args> {
     if input.is_empty() {
-        return Ok(TaskArgs::default());
+        return Ok(Args::default());
     }
 
+    let mut binds = None;
     let mut capacity = None;
     let mut priority = None;
     let mut resources = None;
@@ -936,7 +979,20 @@ fn parse_args(input: ParseStream<'_>, accept_capacity: bool) -> parse::Result<Ta
 
         let ident_s = ident.to_string();
         match &*ident_s {
-            "capacity" if accept_capacity => {
+            "binds" if accepts_binds => {
+                if binds.is_some() {
+                    return Err(parse::Error::new(
+                        ident.span(),
+                        "argument appears more than once",
+                    ));
+                }
+
+                // #ident
+                let ident = content.parse()?;
+
+                binds = Some(ident);
+            }
+            "capacity" if accepts_capacity => {
                 if capacity.is_some() {
                     return Err(parse::Error::new(
                         ident.span(),
@@ -1052,7 +1108,11 @@ fn parse_args(input: ParseStream<'_>, accept_capacity: bool) -> parse::Result<Ta
             _ => {
                 return Err(parse::Error::new(
                     ident.span(),
-                    "expected one of: priority, resources, schedule or spawn",
+                    format!(
+                        "expected one of: {}{}priority, resources, schedule or spawn",
+                        if accepts_binds { "binds, " } else { "" },
+                        if accepts_capacity { "capacity, " } else { "" },
+                    ),
                 ));
             }
         }
@@ -1065,7 +1125,8 @@ fn parse_args(input: ParseStream<'_>, accept_capacity: bool) -> parse::Result<Ta
         let _: Token![,] = content.parse()?;
     }
 
-    Ok(TaskArgs {
+    Ok(Args {
+        binds,
         capacity,
         priority: priority.unwrap_or(1),
         resources: resources.unwrap_or(Idents::new()),

tests/cfail/used-free-interrupt-2.rs

@@ -0,0 +1,21 @@
+#![no_main]
+#![no_std]
+
+extern crate lm3s6965;
+extern crate panic_halt;
+extern crate rtfm;
+
+use rtfm::app;
+
+#[app(device = lm3s6965)]
+const APP: () = {
+    #[init]
+    fn init() {}
+
+    #[interrupt(binds = UART0)] //~ ERROR free interrupts (`extern { .. }`) can't be used as interrupt handlers
+    fn foo() {}
+
+    extern "C" {
+        fn UART0();
+    }
+};

tests/cpass/binds.rs

@@ -0,0 +1,27 @@
+//! Check that `binds` works as advertised
+#![no_main]
+#![no_std]
+
+extern crate lm3s6965;
+extern crate panic_halt;
+extern crate rtfm;
+
+use rtfm::app;
+
+#[app(device = lm3s6965)]
+const APP: () = {
+    #[init]
+    fn init() {}
+
+    #[exception(binds = SVCall)]
+    fn foo() {}
+
+    #[interrupt(binds = UART0)]
+    fn bar() {}
+};
+
+#[allow(dead_code)]
+fn foo_trampoline(_: foo::Context) {}
+
+#[allow(dead_code)]
+fn bar_trampoline(_: bar::Context) {}