m: Fix review comments

- Untangle imports. - Inline the entirely of ListLock into with_inner(). - Add documentation for feature. Signed-off-by: John Nunley <[email protected]>
smol-rs · Nov 23, 2024 · 8aa6a50 · 8aa6a50
1 parent be58b66
commit 8aa6a50
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 50 deletions.
diff --git a/src/intrusive.rs b/src/intrusive.rs
@@ -18,14 +18,12 @@ use core::mem;
 use core::pin::Pin;
 use core::ptr::NonNull;
 
-#[cfg(all(feature = "std", not(feature = "critical-section")))]
-use crate::sync::{Mutex, MutexGuard};
-#[cfg(feature = "critical-section")]
-use critical_section::Mutex;
-
 pub(super) struct List<T>(
-    #[cfg(all(feature = "std", not(feature = "critical-section")))] Mutex<Inner<T>>,
-    #[cfg(feature = "critical-section")] Mutex<RefCell<Inner<T>>>,
+    /// libstd-based implementation uses a normal Muetx to secure the data.
+    #[cfg(all(feature = "std", not(feature = "critical-section")))] crate::sync::Mutex<Inner<T>>,
+
+    /// CS-based implementation uses a CS cell that wraps a RefCell.
+    #[cfg(feature = "critical-section")] critical_section::Mutex<RefCell<Inner<T>>>,
 );
 
 struct Inner<T> {
@@ -57,9 +55,13 @@ impl<T> List<T> {
         };
 
         #[cfg(feature = "critical-section")]
-        let inner = RefCell::new(inner);
+        {
+            let inner = RefCell::new(inner);
+            Self(critical_section::Mutex::new(inner))
+        }
 
-        Self(Mutex::new(inner))
+        #[cfg(not(feature = "critical-section"))]
+        Self(std::sync::Mutex::new(inner))
     }
 
     /// Get the total number of listeners without blocking.
@@ -71,7 +73,7 @@ impl<T> List<T> {
     /// Get the total number of listeners without blocking.
     #[cfg(feature = "critical-section")]
     pub(crate) fn try_total_listeners(&self) -> Option<usize> {
-        Some(critical_section::with(|cs| self.0.borrow(cs).borrow().len))
+        Some(self.total_listeners())
     }
 
     /// Get the total number of listeners with blocking.
@@ -82,15 +84,44 @@ impl<T> List<T> {
 
     /// Get the total number of listeners with blocking.
     #[cfg(all(feature = "std", feature = "critical-section"))]
+    #[allow(unused)]
     pub(crate) fn total_listeners(&self) -> usize {
-        self.try_total_listeners().unwrap()
+        critical_section::with(|cs| self.0.borrow(cs).borrow().len)
     }
 }
 
 impl<T> crate::Inner<T> {
     #[cfg(all(feature = "std", not(feature = "critical-section")))]
     fn with_inner<R>(&self, f: impl FnOnce(&mut Inner<T>) -> R) -> R {
-        let mut list = self.lock();
+        struct ListLock<'a, 'b, T> {
+            lock: std::sync::MutexGuard<'a, Inner<T>>,
+            inner: &'b crate::Inner<T>,
+        }
+
+        impl<T> Deref for ListLock<'_, '_, T> {
+            type Target = Inner<T>;
+
+            fn deref(&self) -> &Self::Target {
+                &self.lock
+            }
+        }
+
+        impl<T> DerefMut for ListLock<'_, '_, T> {
+            fn deref_mut(&mut self) -> &mut Self::Target {
+                &mut self.lock
+            }
+        }
+
+        impl<T> Drop for ListLock<'_, '_, T> {
+            fn drop(&mut self) {
+                update_notified(&self.inner.notified, &self.lock);
+            }
+        }
+
+        let mut list = ListLock {
+            inner: self,
+            lock: self.list.0.lock().unwrap_or_else(|e| e.into_inner()),
+        };
         f(&mut list)
     }
 
@@ -118,14 +149,6 @@ impl<T> crate::Inner<T> {
         })
     }
 
-    #[cfg(all(feature = "std", not(feature = "critical-section")))]
-    fn lock(&self) -> ListLock<'_, '_, T> {
-        ListLock {
-            inner: self,
-            lock: self.list.0.lock().unwrap_or_else(|e| e.into_inner()),
-        }
-    }
-
     /// Add a new listener to the list.
     pub(crate) fn insert(&self, mut listener: Pin<&mut Option<Listener<T>>>) {
         self.with_inner(|inner| {
@@ -337,35 +360,6 @@ impl<T> Inner<T> {
     }
 }
 
-#[cfg(all(feature = "std", not(feature = "critical-section")))]
-struct ListLock<'a, 'b, T> {
-    lock: MutexGuard<'a, Inner<T>>,
-    inner: &'b crate::Inner<T>,
-}
-
-#[cfg(all(feature = "std", not(feature = "critical-section")))]
-impl<T> Deref for ListLock<'_, '_, T> {
-    type Target = Inner<T>;
-
-    fn deref(&self) -> &Self::Target {
-        &self.lock
-    }
-}
-
-#[cfg(all(feature = "std", not(feature = "critical-section")))]
-impl<T> DerefMut for ListLock<'_, '_, T> {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.lock
-    }
-}
-
-#[cfg(all(feature = "std", not(feature = "critical-section")))]
-impl<T> Drop for ListLock<'_, '_, T> {
-    fn drop(&mut self) {
-        update_notified(&self.inner.notified, &self.lock);
-    }
-}
-
 fn update_notified<T>(slot: &crate::sync::atomic::AtomicUsize, list: &Inner<T>) {
     // Update the notified count.
     let notified = if list.notified < list.len {

diff --git a/src/lib.rs b/src/lib.rs
@@ -63,11 +63,15 @@
 //! # Features
 //!
 //! - The `std` feature (enabled by default) enables the use of the Rust standard library. Disable it for `no_std`
-//!   support
+//!   support.
+//! 
+//! - The `critical-section` feature enables usage of the [`critical-section`] crate to enable a
+//!   more efficient implementation of `event-listener` for `no_std` platforms.
 //!
 //! - The `portable-atomic` feature enables the use of the [`portable-atomic`] crate to provide
 //!   atomic operations on platforms that don't support them.
 //!
+//! [`critical-section`]: https://crates.io/crates/critical-section
 //! [`portable-atomic`]: https://crates.io/crates/portable-atomic
 
 #![cfg_attr(not(feature = "std"), no_std)]
@@ -1365,6 +1369,7 @@ mod sync {
     #[cfg(feature = "portable-atomic")]
     pub(super) use portable_atomic_util::Arc;
 
+    #[allow(unused)]
     #[cfg(all(feature = "std", not(feature = "critical-section"), not(loom)))]
     pub(super) use std::sync::{Mutex, MutexGuard};
     #[cfg(all(feature = "std", not(target_family = "wasm"), not(loom)))]