Add tokens for being in a thread

Cargo.toml

@@ -83,3 +83,9 @@ with_msg_v2 = []
 # This is primarily for documentation building; enabling it may also help users
 # of nightly when they see a mess of types.
 actual_never_type = []
+
+# Enable documentation enhancements that depend on nightly
+#
+# This has some effects of its own (making ValueInThread fundamental), and also
+# enables actual_never_type.
+nightly_docs = [ "actual_never_type" ]

src/interrupt.rs

@@ -1,6 +1,6 @@
 //! Interaction with interrupts
 //!
-//! The RIOT wrappers offer three ways to interact with interrupts:
+//! The RIOT wrappers offer two ways to interact with interrupts:
 //!
 //! * Utility functions can disable interrupts (creating critical sections), check whether
 //!   interrupts are enabled or to determine whether code is executed in a thread or an ISR.
@@ -10,27 +10,26 @@
 //!
 //!   These are typechecked to be Send, as they are moved from the thread to the interrupt context.
 //!
-//! * Writing interrupt handlers (using the [`interrupt!`] macro).
-//!
-//!   Writing interrupt handlers is something that obviously needs some care; when using this module,
-//!   you must understand the implications of not doing so within the CPU implementation. (Note: The
-//!   author does not).
-//!
-//!   This is intended to be used for implementing special interfaces that have no generalization in
-//!   RIOT (eg. setting actions at particular points in a PWM cycle).
+//! Not provided by riot-wrappers are methods of implementing interrupts that are directly called
+//! by the CPU's interrupt mechanism. These are `extern "C"` functions (often with a `() -> ()`
+//! signature) exported under a particular name using `#[no_mangle]`. Any platform specifics (such
+//! as the [`riot_sys::inline::cortexm_isr_end()`] function) need to be managed by the
+//! implementer, just as when implementing a C interrupt.
 //!
 //! Rust code intended for use within interrupts does not generally need special precautions -- but
 //! several functions (generally, anything that blocks) are discouraged (as they may fail or stall
-//! the system) outside of a thread context.
+//! the system) outside of a thread context, or even "forbidden" (because they reliably lock up the
+//! system, such as [crate::mutex::Mutex::lock()]). These functions often have preferred
+//! alternatives that can be statically known to be executed in a thread context by keeping a copy
+//! of [`crate::thread::InThread`].
 
 /// Trivial safe wrapper for
 /// [`irq_is_in`](https://doc.riot-os.org/group__core__irq.html#ga83decbeef665d955290f730125ef0e3f)
 ///
 /// Returns true when called from an interrupt service routine
+#[deprecated(note = "Use crate::thread::InThread::new() instead")]
 pub fn irq_is_in() -> bool {
-    // "as u32" cast: Necessary for versions before https://github.com/RIOT-OS/RIOT/pull/17359
-    // (2022.01)
-    (unsafe { riot_sys::irq_is_in() }) as u32 != 0
+    unsafe { riot_sys::irq_is_in() }
 }
 
 /// Trivial safe wrapper for
@@ -39,10 +38,22 @@ pub fn irq_is_in() -> bool {
 /// Returns true if interrupts are currently enabled
 ///
 /// Note that this only returns reliable values when called from a thread context.
+#[deprecated(note = "use crate::thread::InThread::irq_is_enabled() instead")]
 pub fn irq_is_enabled() -> bool {
-    // "as u32" cast: Necessary for versions before https://github.com/RIOT-OS/RIOT/pull/17359
-    // (2022.01)
-    (unsafe { riot_sys::irq_is_enabled() }) as u32 != 0
+    unsafe { riot_sys::irq_is_enabled() }
+}
+
+impl crate::thread::InThread {
+    /// Trivial safe wrapper for
+    /// [`irq_is_enabled`](https://doc.riot-os.org/group__core__irq.html#ga7fa965063ff2f4f4cea34f1c2a8fac25)
+    ///
+    /// Returns true if interrupts are currently enabled
+    ///
+    /// Using this on an `InThread` token is preferred over the global function, as the function
+    /// only returns reliable values when called from a thread context.
+    pub fn irq_is_enabled(self) -> bool {
+        unsafe { riot_sys::irq_is_enabled() }
+    }
 }
 
 /// Proof of running inside a critical section. Reexported from the [bare_metal] crate.
@@ -69,20 +80,22 @@ pub fn free<R, F: FnOnce(&CriticalSection) -> R>(f: F) -> R {
 
 /// Wrap a Rust interrupt handler in an extern "C" wrapper that does the post-return cleaups.
 ///
-/// This is probably Coretex-M specific.
+/// As with all code executed in interrupt contexts, the wrapped function should not panic.
+///
+/// ## Caveats
 ///
-/// The wrapped function should not panic; FIXME: Explore the use of rustig to ensure this.
+/// This is Cortex-M specific.
+#[deprecated(
+    note = "See module documentation: This needs to be done manually per platform; it is incomplete as riot-wrappers provides no method of enabling platform specific interrupts, and provides no other access to configure the peripheral through registers. If it is re-introduced, it will likely carry an `InIsr` token into the function."
+)]
 #[macro_export]
 macro_rules! interrupt {
     ($isr_name:ident, $rust_handler:expr) => {
         #[no_mangle]
         pub extern "C" fn $isr_name() -> () {
             $rust_handler();
 
-            // EXPANDED cpu/cortexm_common/include/cpu.h:189 (cortexm_isr_end)
-            if unsafe { core::ptr::read_volatile(&riot_sys::sched_context_switch_request) } != 0 {
-                unsafe { riot_sys::thread_yield_higher() };
-            }
+            unsafe { riot_sys::inline::cortexm_isr_end() };
         }
     };
 }

src/lib.rs

@@ -20,6 +20,7 @@
 )]
 // Primarily for documentation, see feature docs
 #![cfg_attr(feature = "actual_never_type", feature(never_type))]
+#![cfg_attr(feature = "nightly_docs", feature(fundamental))]
 
 /// riot-sys is re-exported here as it is necessary in some places when using it to get values (eg.
 /// in [error::NumericError::from_constant]). It is also used in macros such as [static_command!].

src/mutex.rs

@@ -40,16 +40,20 @@ impl<T> Mutex<T> {
     }
 
     /// Get an accessor to the mutex when the mutex is available
+    ///
+    /// ## Panics
+    ///
+    /// This function checks at runtime whether it is called in a thread context, and panics
+    /// otherwise. Consider promoting a reference with an [`InThread`](crate::thread::InThread)
+    /// token's [`.promote(&my_mutex)`](crate::thread::InThread::promote) to gain access to a
+    /// better [`.lock()`](crate::thread::ValueInThread<&Mutex<T>>::lock) method, which suffers
+    /// neither the panic condition nor the runtime overhead.
     #[doc(alias = "mutex_lock")]
     pub fn lock(&self) -> MutexGuard<T> {
-        assert!(
-            crate::interrupt::irq_is_in() == false,
-            "Mutex::lock may only be called outside of interrupt contexts.",
-        );
-        // unsafe: All preconditions of the C function are met (not-NULL through taking a &self,
-        // being initialized through RAII guarantees, thread context was checked before).
-        unsafe { riot_sys::mutex_lock(crate::inline_cast_mut(self.mutex.get())) };
-        MutexGuard { mutex: &self }
+        crate::thread::InThread::new()
+            .expect("Mutex::lock may only be called outside of interrupt contexts")
+            .promote(&self)
+            .lock()
     }
 
     /// Get an accessor to the mutex if the mutex is available
@@ -84,7 +88,7 @@ impl<T> Mutex<T> {
     /// be dropped (or even leaked-and-pushed-off-the-stack) even in a locked state. (A possibility
     /// that is fine -- we sure don't want to limit mutex usage to require a Pin reference.)
     ///
-    /// The function could be generalized to some generic lifetime, but there doesn't seem to b a
+    /// The function could be generalized to some generic lifetime, but there doesn't seem to be a
     /// point to it.
     pub fn try_leak(&'static self) -> Option<&'static mut T> {
         let guard = self.try_lock()?;
@@ -93,6 +97,20 @@ impl<T> Mutex<T> {
     }
 }
 
+impl<T> crate::thread::ValueInThread<&Mutex<T>> {
+    /// Get an accessor to the mutex when the mutex is available
+    ///
+    /// Through the [crate::thread::ValueInThread], this is already guaranteed to run in a thread
+    /// context, so no additional check is performed.
+    #[doc(alias = "mutex_lock")]
+    pub fn lock(&self) -> MutexGuard<T> {
+        // unsafe: All preconditions of the C function are met (not-NULL through taking a &self,
+        // being initialized through RAII guarantees, thread context is in the InThread).
+        unsafe { riot_sys::mutex_lock(crate::inline_cast_mut(self.mutex.get())) };
+        MutexGuard { mutex: &self }
+    }
+}
+
 unsafe impl<T: Send> Send for Mutex<T> {}
 unsafe impl<T: Send> Sync for Mutex<T> {}
 

src/panic.rs

@@ -2,7 +2,14 @@
 fn panic(info: &::core::panic::PanicInfo) -> ! {
     use crate::thread;
 
-    if crate::interrupt::irq_is_in() || !crate::interrupt::irq_is_enabled() {
+    let os_can_continue = crate::thread::InThread::new()
+        // Panics with IRQs off are fatal because we can't safely re-enable them
+        .map(|i| i.irq_is_enabled())
+        // Panics in ISRs are always fatal because continuing in threads would signal to the
+        // remaining system that the ISR terminated
+        .unwrap_or(false);
+
+    if !os_can_continue {
         // We can't abort on stable -- but even if we could: Set a breakpoint and wait for the
         // fault handler to reboot us of no debugger is attached? Spin endlessly? core_panic should
         // already answer all these questions.

src/thread.rs

@@ -23,7 +23,7 @@ pub use riot_c::*;
 mod tokenparts;
 #[cfg(doc)]
 pub use tokenparts::TokenParts;
-pub use tokenparts::{EndToken, StartToken, TerminationToken};
+pub use tokenparts::{EndToken, InIsr, InThread, StartToken, TerminationToken, ValueInThread};
 
 mod stack_stats;
 pub use stack_stats::{StackStats, StackStatsError};

src/thread/tokenparts.rs

@@ -1,4 +1,5 @@
-//! Zero-sized types with which code in threads can safely document doing things the first time.
+//! Zero-sized types for threads to document that something is done (often, done the first time)
+//! inside the thread.
 
 use core::marker::PhantomData;
 use core::mem::MaybeUninit;
@@ -35,14 +36,8 @@ pub type TerminationToken = EndToken;
 /// * `FLAG_SEMANTICS`: If this is true, the thread has not assigned semantics to flags yet.
 ///
 /// (FLAG_SEMANTICS are not used yet, but are already prepared for a wrapper similar to `msg::v2`).
-pub struct TokenParts<
-    const MSG_SEMANTICS: bool,
-    const MSG_QUEUE: bool,
-    const FLAG_SEMANTICS: bool,
-    // Do we need something for "we're in a thread" factory? (Probably also doesn't need tracking
-    // b/c it can be Clone -- and everything in RIOT alerady does a cheap irq_is_in check rather
-    // than taking a ZST)
-> {
+pub struct TokenParts<const MSG_SEMANTICS: bool, const MSG_QUEUE: bool, const FLAG_SEMANTICS: bool>
+{
     pub(super) _not_send: PhantomData<*const ()>,
 }
 
@@ -58,6 +53,15 @@ impl TokenParts<true, true, true> {
     }
 }
 
+impl<const MS: bool, const MQ: bool, const FS: bool> TokenParts<MS, MQ, FS> {
+    /// Extract a token that states that code that has access to it is running in a thread (and not
+    /// in an interrupt).
+    pub fn in_thread(&self) -> InThread {
+        // unsafe: TokenParts is not Send, so we can be sure to be in a thread
+        unsafe { InThread::new_unchecked() }
+    }
+}
+
 impl<const MQ: bool, const FS: bool> TokenParts<true, MQ, FS> {
     /// Extract the claim that the thread was not previously configured with any messages that
     /// would be sent to it.
@@ -187,3 +191,95 @@ impl<const MQ: bool> TokenParts<true, MQ, true> {
         self.can_end()
     }
 }
+
+/// Zero-size statement that the current code is not running in an interrupt
+#[derive(Copy, Clone, Debug)]
+pub struct InThread {
+    _not_send: PhantomData<*const ()>,
+}
+
+/// Zero-size statement that the current code is running in an interrupt
+#[derive(Copy, Clone, Debug)]
+pub struct InIsr {
+    _not_send: PhantomData<*const ()>,
+}
+
+impl InThread {
+    unsafe fn new_unchecked() -> Self {
+        InThread {
+            _not_send: PhantomData,
+        }
+    }
+
+    /// Check that the code is running in thread mode
+    ///
+    /// Note that this is actually running code; to avoid that, call [`TokenParts::in_thread()`],
+    /// which is a purely type-level procedure.
+    pub fn new() -> Result<Self, InIsr> {
+        #[allow(deprecated)] // It's deprecatedly pub
+        match crate::interrupt::irq_is_in() {
+            true => Err(unsafe { InIsr::new_unchecked() }),
+            false => Ok(unsafe { InThread::new_unchecked() }),
+        }
+    }
+
+    /// Wrap a `value` in a [`ValueInThread`]. This makes it non-Send, but may make additional
+    /// (safe) methods on it, using the knowledge that it is still being used inside a thread.
+    pub fn promote<T>(self, value: T) -> ValueInThread<T> {
+        ValueInThread {
+            value,
+            in_thread: self,
+        }
+    }
+}
+
+impl InIsr {
+    unsafe fn new_unchecked() -> Self {
+        InIsr {
+            _not_send: PhantomData,
+        }
+    }
+
+    /// Check that the code is running in IRQ mode
+    pub fn new() -> Result<Self, InThread> {
+        match InThread::new() {
+            Ok(i) => Err(i),
+            Err(i) => Ok(i),
+        }
+    }
+}
+
+/// A value combined with an [InThread](crate::thread::InThread) marker
+///
+/// This does barely implement anything on its own, but the module implementing `T` might provide
+/// extra methods.
+// Making the type fundamental results in ValueInThread<&Mutex<T>> being shown at Mutex's page.
+#[cfg_attr(feature = "nightly_docs", fundamental)]
+pub struct ValueInThread<T> {
+    value: T,
+    in_thread: InThread,
+}
+
+impl<T> ValueInThread<T> {
+    /// Extract the wrapped value
+    ///
+    /// This does not produce the original `in_thread` value; these are easy enough to re-obtain or
+    /// to keep a copy of around.
+    pub fn into_inner(self) -> T {
+        self.value
+    }
+}
+
+impl<T> core::ops::Deref for ValueInThread<T> {
+    type Target = T;
+
+    fn deref(&self) -> &T {
+        &self.value
+    }
+}
+
+impl<T> core::ops::DerefMut for ValueInThread<T> {
+    fn deref_mut(&mut self) -> &mut T {
+        &mut self.value
+    }
+}