From aa7eec02996aca9304187f36d674d5fe898aece6 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Tue, 16 Apr 2019 23:04:24 +0200 Subject: [PATCH] check task priority at compile time before we were checking the priority at runtime. The compile time error message when the priority is too high is kind of awful though. --- book/en/src/by-example/resources.md | 7 +++++++ macros/src/codegen.rs | 6 ++---- macros/src/syntax.rs | 4 ++-- tests/cfail/priority-too-high.rs | 22 ++++++++++++++++++++++ tests/cfail/priority-too-low.rs | 22 ++++++++++++++++++++++ 5 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 tests/cfail/priority-too-high.rs create mode 100644 tests/cfail/priority-too-low.rs diff --git a/book/en/src/by-example/resources.md b/book/en/src/by-example/resources.md index 46d04a74c2..17f4d139f3 100644 --- a/book/en/src/by-example/resources.md +++ b/book/en/src/by-example/resources.md @@ -69,6 +69,13 @@ the critical section created by the lowest priority handler. $ cargo run --example lock {{#include ../../../../ci/expected/lock.run}}``` +One more note about priorities: choosing a priority higher than what the device +supports (that is `1 << NVIC_PRIO_BITS`) will result in a compile error. Due to +limitations in the language the error is currently far from helpful: it will say +something along the lines of "evaluation of constant value failed" and the span +of the error will *not* point out to the problematic interrupt value -- we are +sorry about this! + ## Late resources Unlike normal `static` variables, which need to be assigned an initial value diff --git a/macros/src/codegen.rs b/macros/src/codegen.rs index 1d201c0866..8b054ab569 100644 --- a/macros/src/codegen.rs +++ b/macros/src/codegen.rs @@ -1989,15 +1989,13 @@ fn pre_init(ctxt: &Context, app: &App, analysis: &Analysis) -> proc_macro2::Toke )) } - // TODO turn the assertions that check that the priority is not larger than what's supported by - // the device into compile errors let device = &app.args.device; let nvic_prio_bits = quote!(#device::NVIC_PRIO_BITS); 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));)); + exprs.push(quote!(let _ = [(); ((1 << #nvic_prio_bits) - #priority as usize)];)); exprs.push(quote!(p.NVIC.set_priority( #device::Interrupt::#name, ((1 << #nvic_prio_bits) - #priority) << (8 - #nvic_prio_bits), @@ -2007,7 +2005,7 @@ fn pre_init(ctxt: &Context, app: &App, analysis: &Analysis) -> proc_macro2::Toke for (priority, dispatcher) in &analysis.dispatchers { let name = &dispatcher.interrupt; exprs.push(quote!(p.NVIC.enable(#device::Interrupt::#name);)); - exprs.push(quote!(assert!(#priority <= (1 << #nvic_prio_bits));)); + exprs.push(quote!(let _ = [(); ((1 << #nvic_prio_bits) - #priority as usize)];)); exprs.push(quote!(p.NVIC.set_priority( #device::Interrupt::#name, ((1 << #nvic_prio_bits) - #priority) << (8 - #nvic_prio_bits), diff --git a/macros/src/syntax.rs b/macros/src/syntax.rs index 7f87f6339e..228d9588e0 100644 --- a/macros/src/syntax.rs +++ b/macros/src/syntax.rs @@ -1039,10 +1039,10 @@ fn parse_args( } let value = lit.value(); - if value > u64::from(u8::MAX) { + if value > u64::from(u8::MAX) || value == 0 { return Err(parse::Error::new( lit.span(), - "this literal must be in the range 0...255", + "this literal must be in the range 1...255", )); } diff --git a/tests/cfail/priority-too-high.rs b/tests/cfail/priority-too-high.rs new file mode 100644 index 0000000000..ec324014b3 --- /dev/null +++ b/tests/cfail/priority-too-high.rs @@ -0,0 +1,22 @@ +#![no_main] +#![no_std] + +extern crate lm3s6965; +extern crate panic_halt; +extern crate rtfm; + +use rtfm::app; + +#[app(device = lm3s6965)] //~ error evaluation of constant value failed +const APP: () = { + #[init] + fn init() {} + + // OK, this is the maximum priority supported by the device + #[interrupt(priority = 8)] + fn UART0() {} + + // this value is too high! + #[interrupt(priority = 9)] + fn UART1() {} +}; diff --git a/tests/cfail/priority-too-low.rs b/tests/cfail/priority-too-low.rs new file mode 100644 index 0000000000..6dcbfd6348 --- /dev/null +++ b/tests/cfail/priority-too-low.rs @@ -0,0 +1,22 @@ +#![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() {} + + // OK, this is the minimum priority that tasks can have + #[interrupt(priority = 1)] + fn UART0() {} + + // this value is too low! + #[interrupt(priority = 0)] //~ error this literal must be in the range 1...255 + fn UART1() {} +};