Update to 0.2.0: Add HandlerArray type.v0.2.0

  * Create HandlerArray as a safe wrapper around Handler.

  * Add Cargo features for HandlerArray size.

  * Move Handler into sub-module.

  * Add CriticalSection sub-module for architecture dependent support
    of interrupt-free contexts.

  * Add build rules to pull in cortex-m support for CriticalSection
    automatically.

7 files changed, 468 insertions, 201 deletions

diff --git a/src/array.rs b/src/array.rs
new file mode 100644
index 0000000..e01fe74
--- /dev/null
+++ b/src/array.rs
@@ -0,0 +1,171 @@
+//! Safe wrapper for closure-based interrupt handlers.
+//!
+//! # Notes
+//!
+//! The number of entries allowed is defined by Cargo features. The
+//! default is 32 as this seems a reasonable comprimise between the
+//! size of the array and utility. Each array entry costs two words of
+//! space for the closure reference. Thus a full array of 256 entries
+//! on a 32-bit architecture costs 2048 bytes of memory, which can be
+//! quite a lot on resource constrained devices.
+//!
+//! One day, when const-generics are stabilized, this will be more
+//! elegant.
+//!
+//! # Examples
+//!
+//! ``` no_run
+//! use clint::HandlerArray;
+//! use cortex_m_rt::exception;
+//!
+//! static HANDLERS: HandlerArray = HandlerArray::new();
+//!
+//! fn main() {
+//!     // NB: This closure has to be created outside of `with_overrides` to
+//!     // ensure it lives as long as `with_overrides` scope lasts.
+//!     let mut cl = || {
+//!         // Your interrupt handling code.
+//!     };
+//!     HANDLERS.with_overrides(|arr| {
+//!         arr.register(0, &mut cl);
+//!
+//!         loop {
+//!             // Your main loop.
+//!         }
+//!     })
+//! }
+//!
+//! #[exception]
+//! fn SysTick() {
+//!     HANDLERS.call(0);
+//! }
+//! ```
+
+use crate::cs::{CriticalSection, Locker};
+use crate::Handler;
+
+use core::cell::UnsafeCell;
+
+// Define features for the underlying array size so that we can
+// statically allocate it.
+// TODO: Use const generics when available.
+#[cfg(feature = "isr-8")]
+const NR_ISR: usize = 8;
+#[cfg(feature = "isr-16")]
+const NR_ISR: usize = 16;
+#[cfg(feature = "isr-32")]
+const NR_ISR: usize = 32;
+#[cfg(feature = "isr-64")]
+const NR_ISR: usize = 64;
+#[cfg(feature = "isr-128")]
+const NR_ISR: usize = 128;
+#[cfg(feature = "isr-256")]
+const NR_ISR: usize = 256;
+
+/// Safely use `Handler`s by enclosing them in an array.
+///
+/// This type provides a safe wrapper around `Handler` by ensuring
+/// that closures are swapped safely using critical sections, and that
+/// the lifetime of those handlers is sufficient by using the inner
+/// scope of `with_overrides`/`lock_overrides`.
+#[derive(Debug)]
+pub struct HandlerArray<'a> {
+    h: UnsafeCell<[Handler<'a>; NR_ISR]>,
+}
+
+impl<'a> HandlerArray<'a> {
+    /// Create a new `HandlerArray` filled with no-op handlers.
+    pub const fn new() -> Self {
+        Self {
+            h: UnsafeCell::new([Handler::new(); NR_ISR]),
+        }
+    }
+
+    /// Register `f` for entry `nr` in this array using the default
+    /// critical section locker.
+    pub fn register<F>(&self, nr: usize, f: &'a mut F)
+    where
+        F: FnMut() + Send + 'a,
+    {
+        self.lock_register(&Locker::new(), nr, f)
+    }
+
+    /// Register `f` for entry `nr` in this array using `cs` to create
+    /// a critical section for updating the array.
+    pub fn lock_register<F, CS>(&self, cs: &CS, nr: usize, f: &'a mut F)
+    where
+        F: FnMut() + Send + 'a,
+        CS: CriticalSection,
+    {
+        cs.with_lock(|| unsafe { (*self.h.get())[nr].replace(f) });
+    }
+
+    /// Call the handler for entry `nr`.
+    pub fn call(&self, nr: usize) {
+        // Unsafe: there's always a valid handler to call except for
+        // when it's being actively replaced. As long as that happens
+        // while in a critical section, there's no risk of data races.
+        unsafe { (*self.h.get())[nr].call() }
+    }
+
+    /// Create a new array for use in `f`'s scope. The existing
+    /// handlers can be overridden using `register` or
+    /// `lock_register`. When `f` exits, all previous handlers are
+    /// restored.
+    pub fn with_overrides<'b>(&self, f: impl FnOnce(&HandlerArray<'b>)) {
+        self.lock_overrides(&Locker::new(), f)
+    }
+
+    /// Same as `with_overrides` but allows you to specify your own
+    /// implementation of `CriticalSection` instead of using the
+    /// default.
+    pub fn lock_overrides<'b, CS>(&self, cs: &CS, f: impl FnOnce(&HandlerArray<'b>))
+    where
+        CS: CriticalSection,
+    {
+        // Create a shorter-lived array from `self` that matches the
+        // lifetime of `f` so we can make sure `register` is only
+        // called with closures that will live as long as `f` does.
+        //
+        // Unsafe: This requires that we back up and restore the handlers
+        // in the array to make sure there's always something alive in
+        // whatever the real scope of `array' is.
+        let tmp: &HandlerArray<'b> = unsafe { core::mem::transmute(self) };
+
+        // Back up old handlers before entering inner scope so we can
+        // restore them on exit.
+        let bk = HandlerArray::new();
+        unsafe { core::ptr::copy_nonoverlapping(tmp.h.get(), bk.h.get(), 1) }
+        f(tmp);
+
+        // Put the old handlers back inside a critical section to avoid
+        // data races.
+        cs.with_lock(|| unsafe { core::ptr::copy_nonoverlapping(bk.h.get(), tmp.h.get(), 1) });
+    }
+}
+
+// Unsafe: as long as `register` and `with_overrides` use critical
+// sections appropriately, it should be safe to share this between
+// threads.
+unsafe impl<'a> Sync for HandlerArray<'a> {}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn overrides_unwind() {
+        static mut CALLS: usize = 0;
+        let mut cl = || unsafe { CALLS += 1 };
+        let cl_ref = &mut cl;
+
+        let ht = HandlerArray::new();
+        ht.with_overrides(|t| {
+            t.register(0, cl_ref);
+            ht.call(0);
+        });
+        unsafe { assert_eq!(CALLS, 1) };
+        ht.call(0);
+        unsafe { assert_eq!(CALLS, 1) };
+    }
+}
diff --git a/src/cs/cortex.rs b/src/cs/cortex.rs
new file mode 100644
index 0000000..cc97a12
--- /dev/null
+++ b/src/cs/cortex.rs
@@ -0,0 +1,20 @@
+use super::CriticalSection;
+
+use cortex_m::interrupt;
+
+pub struct Locker {}
+
+impl Locker {
+    pub const fn new() -> Self {
+        Self {}
+    }
+}
+
+impl CriticalSection for Locker {
+    fn with_lock<F, R>(&self, f: F) -> R
+    where
+        F: FnOnce() -> R,
+    {
+        interrupt::free(|_cs| f())
+    }
+}
diff --git a/src/cs/dummy.rs b/src/cs/dummy.rs
new file mode 100644
index 0000000..86f4e90
--- /dev/null
+++ b/src/cs/dummy.rs
@@ -0,0 +1,18 @@
+use super::CriticalSection;
+
+pub struct Locker {}
+
+impl Locker {
+    pub const fn new() -> Self {
+        Self {}
+    }
+}
+
+impl CriticalSection for Locker {
+    fn with_lock<F, R>(&self, f: F) -> R
+    where
+        F: FnOnce() -> R,
+    {
+        f()
+    }
+}
diff --git a/src/cs/mod.rs b/src/cs/mod.rs
new file mode 100644
index 0000000..8f97b6e
--- /dev/null
+++ b/src/cs/mod.rs
@@ -0,0 +1,38 @@
+//! Critical Section support.
+//!
+//! Types that implement the `CriticalSection` trait can be used to
+//! create critical sections to prevent data races between interrupt
+//! and non-interrupt contexts.
+//!
+//! # Note
+//!
+//! Critical sections are only acquired for updating a
+//! `HandlerTable`'s entries. They are *not* used when calling into a
+//! closure. This is because the expected implementation of critical
+//! sections turns off interrupts entirely. Given that interrupts are
+//! off, it is impossible to call an ISR, and thus no data race can
+//! occur. Additionally, because critcal sections are only enforced on
+//! updates, deadlock is impossible between updating a `HandlerTable`
+//! entry and calling into it.
+//!
+//! However, if you are going to implement your own `CriticalSection`,
+//! you need to be aware of this limitation and its rationale to avoid
+//! getting into trouble.
+
+/// Generic trait which supplies the ability to create a critical
+/// section.
+pub trait CriticalSection {
+    /// Execute `f` within a critical section.
+    fn with_lock<F, R>(&self, f: F) -> R
+    where
+        F: FnOnce() -> R;
+}
+
+#[cfg_attr(any(all(target_arch = "arm", target_os = "none")), path = "cortex.rs")]
+#[cfg_attr(
+    not(any(all(target_arch = "arm", target_os = "none"))),
+    path = "dummy.rs"
+)]
+mod csimpl;
+
+pub use csimpl::Locker;
diff --git a/src/fnnop.rs b/src/fnnop.rs
new file mode 100644
index 0000000..871a6ab
--- /dev/null
+++ b/src/fnnop.rs
@@ -0,0 +1,19 @@
+// This module needs the following features.
+//
+//#![feature(unboxed_closures)]
+//#![feature(fn_traits)]
+
+pub struct FnNOP();
+
+impl Fn<()> for FnNOP {
+    extern "rust-call" fn call(&self, _args: ()) {}
+}
+impl FnMut<()> for FnNOP {
+    extern "rust-call" fn call_mut(&mut self, _args: ()) {}
+}
+impl FnOnce<()> for FnNOP {
+    type Output = ();
+    extern "rust-call" fn call_once(self, _args: ()) {}
+}
+
+static mut NOP: FnNOP = FnNOP();
diff --git a/src/handler.rs b/src/handler.rs
new file mode 100644
index 0000000..ec1f22e
--- /dev/null
+++ b/src/handler.rs
@@ -0,0 +1,185 @@
+//! Call closures from interrupt handlers.
+//!
+//! # Motivation
+//!
+//! Existing solutions for interrupt handlers typically revolve around
+//! wrapping resources needed by the handler in an `Option`, wrapped
+//! in a `RefCell` wrapped in an `Mutex`, incurring some run-time
+//! overhead every time the resource is required in the interrupt
+//! handler, in addition to a fair amount of boilerplate. This module
+//! attempts to leverage Rust's borrow checker and move semantics to
+//! allow interrupt handlers to directly use their resources with a
+//! minimum of overhead.
+//!
+//! To accomplish this, we use a closure which is called by an
+//! interrupt handler. Because the closure has access to its
+//! environment, we can use `move`, references, and mutable references
+//! to ensure that variables are available as necessary to the
+//! interrupt handler, while leveraging the borrow checker to ensure
+//! safety at compile time. The only overhead is what it takes to call
+//! the closure itself.
+//!
+//! # Safety
+//!
+//! While this module endeavors to use Rust's safety guarantees to
+//! allow for use of resources inside interrupt handlers, due to
+//! expected use-cases, closure semantics, and how interrupt handlers
+//! are invoked from hardware, certain operations cannot be done
+//! safely at this level.
+//!
+//! Notably, for the handler to be useful when called from interrupt
+//! context, it needs to be stored in a `static mut` variable. This
+//! means that the closure you supply it must also be effectively
+//! `static` or replaced with a longer-lived closure before it goes
+//! out of scope. `Handler::default_handler()` is provided for this
+//! purpose.
+//!
+//! Additionally, replacement of an interrupt handler's closure may
+//! race with the calling of the interrupt handler's closure (i.e.,
+//! `Handler.replace()` may happen concurrently with
+//! `Handler.call()`). You need to avoid this situation however is
+//! appropriate for your code. The expected usage would be replacing
+//! the handler`s closure once, while interrupts are disabled, thus
+//! preventing the simultaneous replace/call problem. As this module
+//! makes no assumptions about the environment in which it will be
+//! used, this cannot be done for you.
+//!
+//! # Examples
+//!
+//! This example for an ARM Cortex-M system demonstrates safe usage by
+//! only replacing the closure for `SYSTICK_HANDLER` inside a critical
+//! section obtained by `cortex_m::interrupt::free()`, and shows how
+//! it is called via the `SysTick()` function, which is called
+//! directly from hardware.
+//!
+//! ``` no_run
+//! use clint::Handler;
+//! use cortex_m_rt::exception;
+//!
+//! static mut SYSTICK_HANDLER: Handler = Handler::new();
+//!
+//! fn main() {
+//!     // NB: `closure` is in the lexical scope of `main`, and thus
+//!     // cannot go out of scope.
+//!     let closure = || {
+//!         // Your interrupt handling code.
+//!     };
+//!     // Replace the handler for SysTick with closure while interrupts are
+//!     // disabled.
+//!     cortex_m::interrupt::free(|_| {
+//!         unsafe { SYSTICK_HANDLER.replace(&closure) };
+//!     });
+//!
+//!     loop {
+//!         // Your main loop.
+//!     }
+//! }
+//!
+//! #[exception]
+//! fn SysTick() {
+//!     unsafe { SYSTICK_HANDLER.call() };
+//! }
+//! ```
+
+pub struct Handler<'a> {
+    // Handler that will be executed on `call`.
+    h: *const dyn FnMut(),
+    lifetime: core::marker::PhantomData<&'a dyn FnMut()>,
+}
+
+impl<'a> Handler<'a> {
+    /// Returns a new Handler that initially does nothing when
+    /// called. Override its behavior by using `replace`.
+    pub const fn new() -> Self {
+        Self {
+            h: &Self::default_handler,
+            lifetime: core::marker::PhantomData,
+        }
+    }
+
+    /// Replace the behavior of this handler with `f`.
+    ///
+    /// # Safety
+    ///
+    /// There is no exclusion on replacing the handler's behavior
+    /// while it is being executed. It is your responsibility to make
+    /// sure that it's not being executed when you call `replace`.
+    pub unsafe fn replace(&mut self, f: &(dyn FnMut() + Send + 'a)) {
+        self.h = core::mem::transmute::<_, &'a _>(f);
+    }
+
+    /// Execute this handler.
+    ///
+    /// # Safety
+    ///
+    /// This function assumes that a replace is not occurring when the
+    /// closure is being looked up. You need to ensure that `replace`
+    /// and `call` can not occur at the same time.
+    pub unsafe fn call(&self) {
+        let f: &mut dyn FnMut() = &mut *(self.h as *mut dyn FnMut());
+        f();
+    }
+
+    /// Do nothing handler. Needed by `call` until `replace` is used
+    /// to set specific behavior. Can also be used to replace a
+    /// closure that is about to go out of scope.
+    pub fn default_handler() {}
+}
+
+impl<'a> core::fmt::Debug for Handler<'a> {
+    fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
+        let (f0, f1) = unsafe { core::mem::transmute::<_, (usize, usize)>(self.h) };
+        write!(f, "Handler{{ h: (0x{:x}, 0x{:x}) }}", f0, f1)
+    }
+}
+
+// FIXME: This probably shouldn't be Copy/Clone, but it needs to be in
+// order for array initialization to work with [Handler::new(); 32].
+impl<'a> core::marker::Copy for Handler<'a> {}
+impl<'a> core::clone::Clone for Handler<'a> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn replace() {
+        static mut X: usize = 0;
+
+        let mut handler = Handler::new();
+        unsafe {
+            handler.replace(&|| X += 1);
+            assert_eq!(X, 0);
+            handler.call();
+            handler.call();
+            assert_eq!(X, 2);
+        }
+    }
+
+    #[test]
+    fn replace_static() {
+        static mut HANDLER: Handler = Handler::new();
+        static mut X: usize = 0;
+
+        unsafe {
+            HANDLER.replace(&|| X += 1);
+            assert_eq!(X, 0);
+            HANDLER.call();
+            HANDLER.call();
+            assert_eq!(X, 2);
+        }
+    }
+
+    #[test]
+    fn replace_with_default() {
+        let mut handler = Handler::new();
+        unsafe {
+            handler.replace(&Handler::default_handler);
+            handler.call()
+        }
+    }
+}
diff --git a/src/lib.rs b/src/lib.rs
index 39cbfb0..8e16b26 100755..100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,209 +1,25 @@
-#![no_std]
-#![feature(const_fn)]
-#![feature(test)]
-
-//! Call closures from interrupt handlers.
-//!
-//! # Motivation
-//!
-//! Existing solutions for interrupt handlers typically revolve around
-//! wrapping resources needed by the handler in an `Option`, wrapped
-//! in a `RefCell` wrapped in an `Mutex`, incurring some run-time
-//! overhead every time the resource is required in the interrupt
-//! handler, in addition to a fair amount of boilerplate. This module
-//! attempts to leverage Rust's borrow checker and move semantics to
-//! allow interrupt handlers to directly use their resources with a
-//! minimum of overhead.
-//!
-//! To accomplish this, we use a closure which is called by an
-//! interrupt handler. Because the closure has access to its
-//! environment, we can use `move`, references, and mutable references
-//! to ensure that variables are available as necessary to the
-//! interrupt handler, while leveraging the borrow checker to ensure
-//! safety at compile time. The only overhead is what it takes to call
-//! the closure itself.
-//!
-//! # Safety
+//! CLosure INTerrupt handlers
 //!
-//! While this module endeavors to use Rust's safety guarantees to
-//! allow for use of resources inside interrupt handlers, due to
-//! expected use-cases, closure semantics, and how interrupt handlers
-//! are invoked from hardware, certain operations cannot be done
-//! safely at this level.
+//! Use closures as interrupt service routines to leverage Rust's
+//! borrow checker for safe, exclusive usage of device peripherals and
+//! other data without locking.
 //!
-//! Notably, for the handler to be useful when called from interrupt
-//! context, it needs to be stored in a `static mut` variable. This
-//! means that the closure you supply it must also be effectively
-//! `static` or replaced with a longer-lived closure before it goes
-//! out of scope. `Handler::default_handler()` is provided for this
-//! purpose.
+//! # Layout
 //!
-//! Additionally, replacement of an interrupt handler's closure may
-//! race with the calling of the interrupt handler's closure (i.e.,
-//! `Handler.replace()` may happen concurrently with
-//! `Handler.call()`). You need to avoid this situation however is
-//! appropriate for your code. The expected usage would be replacing
-//! the handler`s closure once, while interrupts are disabled, thus
-//! preventing the simultaneous replace/call problem. As this module
-//! makes no assumptions about the environment in which it will be
-//! used, this cannot be done for you.
+//! See [`array`'s module documentation](table/index.html#examples) for basic,
+//! safe usage.
 //!
-//! # Examples
+//! The [`handler`](handler) module contains the underyling, unsafe
+//! implementation.
 //!
-//! This example for an ARM Cortex-M system demonstrates safe usage by
-//! only replacing the closure for `SYSTICK_HANDLER` inside a critical
-//! section obtained by `cortex_m::interrupt::free()`, and shows how
-//! it is called via the `SysTick()` function, which is called
-//! directly from hardware.
-//!
-//! ``` no_run
-//! use clint::Handler;
-//! use cortex_m_rt::exception;
-//!
-//! static mut SYSTICK_HANDLER: Handler = Handler::new();
-//!
-//! fn main() {
-//!     // NB: `closure` is in the lexical scope of `main`, and thus
-//!     // cannot go out of scope.
-//!     let closure = || {
-//!         // Your interrupt handling code.
-//!     };
-//!     // Replace the handler for SysTick with closure while interrupts are
-//!     // disabled.
-//!     cortex_m::interrupt::free(|_| {
-//!         unsafe { SYSTICK_HANDLER.replace(&closure) };
-//!     });
-//!
-//!     loop {
-//!         // Your main loop.
-//!     }
-//! }
-//!
-//! #[exception]
-//! fn SysTick() {
-//!     unsafe { SYSTICK_HANDLER.call() };
-//! }
-//! ```
-
-pub struct Handler {
-    // Handler that will be executed on `call`.
-    h: *const dyn FnMut(),
-}
-
-impl Handler {
-    /// Returns a new Handler that initially does nothing when
-    /// called. Override it's behavior by using `replace`.
-    pub const fn new() -> Self {
-        Self {
-            h: &Self::default_handler,
-        }
-    }
+//! Critical section support is supplied by the [`cs` module](cs).
 
-    /// Replace the behavior of this handler with `f`.
-    ///
-    /// # Safety
-    ///
-    /// There is no exclusion on replacing the handler's behavior
-    /// while it is being executed. It is your responsibility to make
-    /// sure that it's not being executed when you call `replace`.
-    pub unsafe fn replace<F>(&mut self, f: &F)
-    where
-        F: FnMut() + 'static,
-    {
-        self.h = f;
-    }
-
-    /// Execute this handler.
-    ///
-    /// # Safety
-    ///
-    /// This function assumes that a replace is not occurring when the
-    /// closure is being looked up. You need to ensure that `replace`
-    /// and `call` can not occur at the same time.
-    pub unsafe fn call(&self) {
-        let f: &mut dyn FnMut() = &mut *(self.h as *mut dyn FnMut());
-        f();
-    }
-
-    /// Do nothing handler. Needed by `call` until `replace` is used
-    /// to set specific behavior. Can also be used to replace a
-    /// closure that is about to go out of scope.
-    pub fn default_handler() {}
-}
-
-impl core::fmt::Debug for Handler {
-    fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
-        let (f0, f1) = unsafe { core::mem::transmute::<_, (usize, usize)>(self.h) };
-        write!(f, "Handler{{ h: (0x{:x}, 0x{:x}) }}", f0, f1)
-    }
-}
-
-#[cfg(test)]
-mod test {
-    extern crate test;
-    use super::*;
-    use test::Bencher;
-
-    #[test]
-    fn replace() {
-        static mut X: usize = 0;
-
-        let mut handler = Handler::new();
-        unsafe {
-            handler.replace(&|| X += 1);
-            assert_eq!(X, 0);
-            handler.call();
-            handler.call();
-            assert_eq!(X, 2);
-        }
-    }
-
-    #[test]
-    fn replace_static() {
-        static mut HANDLER: Handler = Handler::new();
-        static mut X: usize = 0;
-
-        unsafe {
-            HANDLER.replace(&|| X += 1);
-            assert_eq!(X, 0);
-            HANDLER.call();
-            HANDLER.call();
-            assert_eq!(X, 2);
-        }
-    }
-
-    #[test]
-    fn replace_with_default() {
-        let mut handler = Handler::new();
-        unsafe {
-            handler.replace(&Handler::default_handler);
-            handler.call()
-        }
-    }
-
-    const ITER_COUNT: usize = 10_000;
-
-    #[bench]
-    fn bench_bare_fn(b: &mut Bencher) {
-        static mut X: usize = 0;
-        #[inline(never)]
-        fn inc() {
-            unsafe { X += 1 };
-        }
-
-        let n = test::black_box(ITER_COUNT);
-        b.iter(|| (0..n).for_each(|_| inc()));
-        assert!(unsafe { X } > 0);
-    }
+#![no_std]
+#![feature(const_fn)]
 
-    #[bench]
-    fn bench_handler(b: &mut Bencher) {
-        static mut X: usize = 0;
-        let mut handler = Handler::new();
-        unsafe { handler.replace(&move || X += 1) };
+pub mod array;
+pub mod cs;
+pub mod handler;
 
-        let n = test::black_box(ITER_COUNT);
-        b.iter(|| (0..n).for_each(|_| unsafe { handler.call() }));
-        assert!(unsafe { X } > 0);
-    }
-}
+pub use array::HandlerArray;
+pub use handler::Handler;