Refactor Loom integration (#6)

* Add generic loom testing

* Add cfg module

* Run cargo-hack with loom cfg

.github/workflows/ci.yml

@@ -79,8 +79,6 @@ jobs:
   linter:
     name: Linter
     runs-on: ubuntu-latest
-    env:
-      RUSTFLAGS: -D warnings -D clippy::pedantic -D clippy::nursery
     steps:
       - name: Checkout repository
         uses: actions/checkout@v3
@@ -89,11 +87,13 @@ jobs:
       - name: Install cargo-hack
         uses: taiki-e/install-action@cargo-hack
       - name: Lint feature powerset
+        env:
+          RUSTFLAGS: -D warnings -D clippy::pedantic -D clippy::nursery
         run: cargo hack clippy --feature-powerset --no-dev-deps
       - name: Lint loom
         env:
-          RUSTFLAGS: --cfg loom
-        run: cargo clippy --profile test --all-features
+          RUSTFLAGS: --cfg loom -D warnings -D clippy::pedantic -D clippy::nursery
+        run: cargo hack clippy --profile test --feature-powerset
 
   miri:
     name: Miri

Makefile.toml

@@ -10,8 +10,9 @@ default_to_workspace = false
 # Build package for no_std environment.
 [tasks.no-std]
 command = "cargo"
-args = ["hack", "build", "--target", "thumbv7m-none-eabi", "--no-dev-deps",
-        "--feature-powerset", "--skip", "yield,thread_local"]
+args = ["hack", "build", "--target", "thumbv7m-none-eabi",
+        "--feature-powerset", "--no-dev-deps", "--skip",
+        "yield,thread_local"]
 
 # Build docs for docs.rs.
 [tasks.docsrs]
@@ -26,25 +27,32 @@ command = "cargo"
 env = { "RUSTFLAGS" = "${CLIPPY_FLAGS}" }
 args = ["hack", "clippy", "--feature-powerset", "--no-dev-deps"]
 
+# Lint all feature combinations with carg-hack on test profile.
+[tasks.lint-test]
+command = "cargo"
+env = { "RUSTFLAGS" = "${CLIPPY_FLAGS}" }
+args = ["hack", "clippy", "--profile", "test", "--feature-powerset",
+        "--no-dev-deps"]
+
 # Run tests under miri.
 # NOTE: must run as: cargo +nightly make miri
 [tasks.miri-test]
 toolchain = "nightly"
 install_crate = { rustup_component_name = "miri" }
 command = "cargo"
-args = ["miri", "test", "--all-features"]
+args = ["miri", "test", "--all-features", "${@}"]
 
 # Run Loom tests.
 [tasks.loom-test]
 command = "cargo"
 env = { "RUSTFLAGS" = "${CFG_LOOM}" }
-args = ["test", "--lib", "--release", "--all-features"]
+args = ["test", "--lib", "--release", "--all-features", "${@}"]
 
-# Lint Loom cfg.
+# Lint all feature combinations with cargo-hack on test profile and Loom cfg.
 [tasks.loom-lint]
 command = "cargo"
 env = { "RUSTFLAGS" = "${CLIPPY_FLAGS_LOOM}" }
-args = ["clippy", "--profile", "test", "--all-features"]
+args = ["hack", "clippy", "--profile", "test", "--feature-powerset"]
 
 # Run busy loop bench.
 [tasks.bench-busy]

src/barging/mutex.rs

@@ -2,19 +2,8 @@ use core::fmt;
 use core::marker::PhantomData;
 use core::sync::atomic::Ordering::{Acquire, Relaxed, Release};
 
-#[cfg(not(all(loom, test)))]
-use core::ops::{Deref, DerefMut};
-#[cfg(not(all(loom, test)))]
-use core::sync::atomic::AtomicBool;
-
-#[cfg(all(loom, test))]
-use loom::cell::{ConstPtr, MutPtr};
-#[cfg(all(loom, test))]
-use loom::sync::atomic::AtomicBool;
-
-#[cfg(all(loom, test))]
-use crate::loom::{Guard, GuardDeref, GuardDerefMut};
-
+use crate::cfg::atomic::AtomicBool;
+use crate::cfg::cell::WithUnchecked;
 use crate::raw::{Mutex as RawMutex, MutexNode};
 use crate::relax::Relax;
 
@@ -249,9 +238,9 @@ impl<T: ?Sized, R> Mutex<T, R> {
     /// let c_mutex = Arc::clone(&mutex);
     ///
     /// thread::spawn(move || {
-    ///     let mut lock = c_mutex.try_lock();
-    ///     if let Some(ref mut mutex) = lock {
-    ///         **mutex = 10;
+    ///     let mut guard = c_mutex.try_lock();
+    ///     if let Some(mut guard) = guard {
+    ///         *guard = 10;
     ///     } else {
     ///         println!("try_lock failed");
     ///     }
@@ -291,8 +280,12 @@ impl<T: ?Sized, R> Mutex<T, R> {
     /// let c_mutex = Arc::clone(&mutex);
     ///
     /// thread::spawn(move || {
-    ///     c_mutex.try_lock_with(|mut guard| {
-    ///         *guard.unwrap() = 10;
+    ///     c_mutex.try_lock_with(|guard| {
+    ///         if let Some(mut guard) = guard {
+    ///             *guard = 10;
+    ///         } else {
+    ///             println!("try_lock failed");
+    ///         }
     ///     });
     /// })
     /// .join().expect("thread::spawn failed");
@@ -373,24 +366,6 @@ impl<T: ?Sized, R> Mutex<T, R> {
     fn unlock(&self) {
         self.locked.store(false, Release);
     }
-
-    #[cfg(not(all(loom, test)))]
-    /// Returns a raw mutable pointer to the underlying data.
-    const fn data_ptr(&self) -> *mut T {
-        self.inner.data_ptr()
-    }
-
-    /// Get a Loom immutable raw pointer to the underlying data.
-    #[cfg(all(loom, test))]
-    fn data_get(&self) -> ConstPtr<T> {
-        self.inner.data_get()
-    }
-
-    /// Get a Loom mutable raw pointer to the underlying data.
-    #[cfg(all(loom, test))]
-    fn data_get_mut(&self) -> MutPtr<T> {
-        self.inner.data_get_mut()
-    }
 }
 
 impl<T: ?Sized + Default, R> Default for Mutex<T, R> {
@@ -413,6 +388,30 @@ impl<T: ?Sized + fmt::Debug, R: Relax> fmt::Debug for Mutex<T, R> {
     }
 }
 
+#[cfg(all(loom, test))]
+#[rustfmt::skip]
+impl<T: ?Sized, R: Relax> crate::loom::LockWith<T> for Mutex<T, R> {
+    type Guard<'a> = MutexGuard<'a, T, R> where T: 'a, Self: 'a;
+
+    fn new(value: T) -> Self where T: Sized {
+        Self::new(value)
+    }
+
+    fn try_lock_with<F, Ret>(&self, f: F) -> Ret
+    where
+        F: FnOnce(Option<MutexGuard<'_, T, R>>) -> Ret,
+    {
+        self.try_lock_with(f)
+    }
+
+    fn lock_with<F, Ret>(&self, f: F) -> Ret
+    where
+        F: FnOnce(MutexGuard<'_, T, R>) -> Ret,
+    {
+        self.lock_with(f)
+    }
+}
+
 #[cfg(all(feature = "lock_api", not(loom)))]
 unsafe impl<R: Relax> lock_api::RawMutex for Mutex<(), R> {
     type GuardMarker = lock_api::GuardSend;
@@ -467,24 +466,13 @@ impl<'a, T: ?Sized, R> MutexGuard<'a, T, R> {
         Self { lock }
     }
 
-    /// Runs `f` with an immutable reference to the wrapped value.
-    #[cfg(not(all(loom, test)))]
-    fn data_with<F, Ret>(&self, f: F) -> Ret
+    /// Runs `f` against an shared reference pointing to the underlying data.
+    fn with<F, Ret>(&self, f: F) -> Ret
     where
         F: FnOnce(&T) -> Ret,
     {
         // SAFETY: A guard instance holds the lock locked.
-        f(unsafe { &*self.lock.data_ptr() })
-    }
-
-    /// Runs `f` with an immutable reference to the wrapped value.
-    #[cfg(all(loom, test))]
-    pub(crate) fn data_with<F, Ret>(&self, f: F) -> Ret
-    where
-        F: FnOnce(&T) -> Ret,
-    {
-        // SAFETY: A guard instance holds the lock locked.
-        f(unsafe { self.lock.data_get().deref() })
+        unsafe { self.lock.inner.data.with_unchecked(f) }
     }
 }
 
@@ -495,49 +483,43 @@ impl<T: ?Sized, R> Drop for MutexGuard<'_, T, R> {
 }
 
 #[cfg(not(all(loom, test)))]
-impl<'a, T: ?Sized, R> Deref for MutexGuard<'a, T, R> {
+impl<'a, T: ?Sized, R> core::ops::Deref for MutexGuard<'a, T, R> {
     type Target = T;
 
     /// Dereferences the guard to access the underlying data.
     fn deref(&self) -> &T {
         // SAFETY: A guard instance holds the lock locked.
-        unsafe { &*self.lock.data_ptr() }
+        unsafe { &*self.lock.inner.data.get() }
     }
 }
 
 #[cfg(not(all(loom, test)))]
-impl<'a, T: ?Sized, R> DerefMut for MutexGuard<'a, T, R> {
+impl<'a, T: ?Sized, R> core::ops::DerefMut for MutexGuard<'a, T, R> {
     /// Mutably dereferences the guard to access the underlying data.
     fn deref_mut(&mut self) -> &mut T {
         // SAFETY: A guard instance holds the lock locked.
-        unsafe { &mut *self.lock.data_ptr() }
+        unsafe { &mut *self.lock.inner.data.get() }
     }
 }
 
 impl<'a, T: ?Sized + fmt::Debug, R> fmt::Debug for MutexGuard<'a, T, R> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.data_with(|data| fmt::Debug::fmt(data, f))
+        self.with(|data| fmt::Debug::fmt(data, f))
     }
 }
 
 impl<'a, T: ?Sized + fmt::Display, R> fmt::Display for MutexGuard<'a, T, R> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.data_with(|data| fmt::Display::fmt(data, f))
+        self.with(|data| fmt::Display::fmt(data, f))
     }
 }
 
 /// SAFETY: A guard instance hold the lock locked, with exclusive access to the
 /// underlying data.
 #[cfg(all(loom, test))]
-unsafe impl<'a, T: ?Sized, R> Guard<'a, T> for MutexGuard<'a, T, R> {
-    type Guard = Self;
-
-    fn deref(&'a self) -> GuardDeref<'a, T, Self::Guard> {
-        GuardDeref::new(self.lock.data_get())
-    }
-
-    fn deref_mut(&'a self) -> GuardDerefMut<'a, T, Self::Guard> {
-        GuardDerefMut::new(self.lock.data_get_mut())
+unsafe impl<T: ?Sized, R: Relax> crate::loom::Guard<T> for MutexGuard<'_, T, R> {
+    fn get(&self) -> &loom::cell::UnsafeCell<T> {
+        &self.lock.inner.data
     }
 }
 
@@ -715,57 +697,21 @@ mod test {
 
 #[cfg(all(loom, test))]
 mod test {
-    use loom::{model, thread};
-
     use crate::barging::yields::Mutex;
-    use crate::loom::Guard;
+    use crate::loom::model;
 
     #[test]
-    fn threads_join() {
-        use core::ops::Range;
-        use loom::sync::Arc;
-
-        fn inc(lock: Arc<Mutex<i32>>) {
-            let guard = lock.lock();
-            *guard.deref_mut() += 1;
-        }
-
-        model(|| {
-            let data = Arc::new(Mutex::new(0));
-            // 3 or more threads make this model run for too long.
-            let runs @ Range { end, .. } = 0..2;
-
-            let handles = runs
-                .into_iter()
-                .map(|_| Arc::clone(&data))
-                .map(|data| thread::spawn(move || inc(data)))
-                .collect::<Vec<_>>();
-
-            for handle in handles {
-                handle.join().unwrap();
-            }
-
-            assert_eq!(end, *data.lock().deref());
-        });
+    fn try_lock_join() {
+        model::try_lock_join::<Mutex<_>>();
     }
 
     #[test]
-    fn threads_fork() {
-        // Using std's Arc or else this model runs for loo long.
-        use std::sync::Arc;
-
-        fn inc(lock: Arc<Mutex<i32>>) {
-            let guard = lock.lock();
-            *guard.deref_mut() += 1;
-        }
+    fn lock_join() {
+        model::lock_join::<Mutex<_>>();
+    }
 
-        model(|| {
-            let data = Arc::new(Mutex::new(0));
-            // 4 or more threads make this model run for too long.
-            for _ in 0..3 {
-                let data = Arc::clone(&data);
-                thread::spawn(move || inc(data));
-            }
-        });
+    #[test]
+    fn mixed_lock_join() {
+        model::mixed_lock_join::<Mutex<_>>();
     }
 }