Put loom behind cfg(oneshot_loom) again

.github/workflows/build-and-test.yml

@@ -40,10 +40,10 @@ jobs:
         shell: bash
         run: cargo build --locked
 
-      # Run `cargo test` with all combinations of features. Including loom correctness testing
+      # Run through all tests with all combinations of features
       - name: Test
         shell: bash
-        run: LOOM_MAX_BRANCHES=100000 cargo hack --feature-powerset --exclude-all-features --optional-deps loom test
+        run: cargo hack --feature-powerset --exclude-all-features test
 
       # Run `cargo test` but with artificial delay injected into some code paths. This helps
       # running through some hard-to-time code paths. loom testing is not included since it is not
@@ -52,6 +52,13 @@ jobs:
         shell: bash
         run: RUSTFLAGS+="--cfg oneshot_test_delay" cargo hack --feature-powerset --exclude-all-features test
 
+      # Compile the library against loom to do correctness testing
+      - name: Test with loom
+        shell: bash
+        run: |
+          RUSTFLAGS+="--cfg oneshot_loom" LOOM_MAX_BRANCHES=100000 \
+            cargo hack --feature-powerset --exclude-all-features --features loom test
+
       # Check that the documentation builds and has no warnings
       - name: Build documentation
         shell: bash

CHANGELOG.md

@@ -17,7 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## [Unreleased]
 ### Changed
 - Change how loom concurrency testing is triggered. Make it an optional dependency instead of
-  placed behind a custom `cfg(loom)`. This has the pros that it does not end up in the dependency
+  placed behind a custom `cfg(oneshot_loom)`. This has the pros that it does not end up in the dependency
   tree of `oneshot` (unless the optional feature `loom` is enabled), which makes this library
   way smaller for downstream consumers. This has the downside that the crate now exposes a `loom`
   feature. DOWNSTREAM USERS ARE NOT SUPPOSED TO EVER ENABLE THIS. No stability or semver

Cargo.toml

@@ -26,8 +26,10 @@ std = []
 # Enables async receiving by implementing Future
 async = []
 
-[dependencies]
-# Only used for internal correctness testing. Never enable this feature when using oneshot.
+# Only used for internal correctness testing.
+# Downstream users of oneshot should never enable this feature. Enabling it does nothing.
+# To compile oneshot built against loom one must *also* set RUSTFLAGS="--cfg oneshot_loom"
+[target.'cfg(oneshot_loom)'.dependencies]
 loom = { version = "0.7.2", features = ["futures"], optional = true }
 
 [dev-dependencies]
@@ -36,7 +38,7 @@ tokio = { version = "1", features = ["rt", "rt-multi-thread", "macros", "time"]
 async-std = { version = "1", features = ["attributes"] }
 
 [lints.rust]
-unexpected_cfgs = { level = "deny", check-cfg = ['cfg(oneshot_test_delay)'] }
+unexpected_cfgs = { level = "deny", check-cfg = ['cfg(oneshot_loom)', 'cfg(oneshot_test_delay)'] }
 
 [[bench]]
 name = "benches"

src/lib.rs

@@ -116,7 +116,7 @@
 #![deny(rust_2018_idioms)]
 #![cfg_attr(not(feature = "std"), no_std)]
 
-#[cfg(not(feature = "loom"))]
+#[cfg(not(oneshot_loom))]
 extern crate alloc;
 
 use core::{
@@ -125,20 +125,20 @@ use core::{
     ptr::{self, NonNull},
 };
 
-#[cfg(not(feature = "loom"))]
+#[cfg(not(oneshot_loom))]
 use core::{
     cell::UnsafeCell,
     sync::atomic::{fence, AtomicU8, Ordering::*},
 };
-#[cfg(feature = "loom")]
+#[cfg(oneshot_loom)]
 use loom::{
     cell::UnsafeCell,
     sync::atomic::{fence, AtomicU8, Ordering::*},
 };
 
-#[cfg(all(feature = "async", not(feature = "loom")))]
+#[cfg(all(feature = "async", not(oneshot_loom)))]
 use core::hint;
-#[cfg(all(feature = "async", feature = "loom"))]
+#[cfg(all(feature = "async", oneshot_loom))]
 use loom::hint;
 
 #[cfg(feature = "async")]
@@ -151,10 +151,10 @@ use std::time::{Duration, Instant};
 
 #[cfg(feature = "std")]
 mod thread {
-    #[cfg(not(feature = "loom"))]
+    #[cfg(not(oneshot_loom))]
     pub use std::thread::{current, park, park_timeout, Thread};
 
-    #[cfg(feature = "loom")]
+    #[cfg(oneshot_loom)]
     pub use loom::thread::{current, park, Thread};
 
     // loom does not support parking with a timeout. So we just
@@ -163,17 +163,17 @@ mod thread {
     // One thing to note is that very short timeouts are needed
     // when using loom, since otherwise the looping will cause
     // an overflow in loom.
-    #[cfg(feature = "loom")]
+    #[cfg(oneshot_loom)]
     pub fn park_timeout(_timeout: std::time::Duration) {
         loom::thread::yield_now()
     }
 }
 
-#[cfg(feature = "loom")]
+#[cfg(oneshot_loom)]
 mod loombox;
-#[cfg(not(feature = "loom"))]
+#[cfg(not(oneshot_loom))]
 use alloc::boxed::Box;
-#[cfg(feature = "loom")]
+#[cfg(oneshot_loom)]
 use loombox::Box;
 
 mod errors;
@@ -488,7 +488,7 @@ impl<T> Receiver<T> {
                 // Conditionally add a delay here to help the tests trigger the edge cases where
                 // the sender manages to be dropped or send something before we are able to store
                 // our waker object in the channel.
-                #[cfg(all(oneshot_test_delay, not(feature = "loom")))]
+                #[cfg(all(oneshot_test_delay, not(oneshot_loom)))]
                 std::thread::sleep(std::time::Duration::from_millis(10));
 
                 // Write our waker instance to the channel.
@@ -795,7 +795,7 @@ impl<T> Receiver<T> {
                 // Conditionally add a delay here to help the tests trigger the edge cases where
                 // the sender manages to be dropped or send something before we are able to store
                 // our waker object in the channel.
-                #[cfg(all(oneshot_test_delay, not(feature = "loom")))]
+                #[cfg(all(oneshot_test_delay, not(oneshot_loom)))]
                 std::thread::sleep(std::time::Duration::from_millis(10));
 
                 // Write our waker instance to the channel.
@@ -1055,12 +1055,12 @@ impl<T> Channel<T> {
 
     #[inline(always)]
     unsafe fn message(&self) -> &MaybeUninit<T> {
-        #[cfg(feature = "loom")]
+        #[cfg(oneshot_loom)]
         {
             self.message.with(|ptr| &*ptr)
         }
 
-        #[cfg(not(feature = "loom"))]
+        #[cfg(not(oneshot_loom))]
         {
             &*self.message.get()
         }
@@ -1071,12 +1071,12 @@ impl<T> Channel<T> {
     where
         F: FnOnce(&mut MaybeUninit<T>),
     {
-        #[cfg(feature = "loom")]
+        #[cfg(oneshot_loom)]
         {
             self.message.with_mut(|ptr| op(&mut *ptr))
         }
 
-        #[cfg(not(feature = "loom"))]
+        #[cfg(not(oneshot_loom))]
         {
             op(&mut *self.message.get())
         }
@@ -1088,12 +1088,12 @@ impl<T> Channel<T> {
     where
         F: FnOnce(&mut MaybeUninit<ReceiverWaker>),
     {
-        #[cfg(feature = "loom")]
+        #[cfg(oneshot_loom)]
         {
             self.waker.with_mut(|ptr| op(&mut *ptr))
         }
 
-        #[cfg(not(feature = "loom"))]
+        #[cfg(not(oneshot_loom))]
         {
             op(&mut *self.waker.get())
         }
@@ -1106,12 +1106,12 @@ impl<T> Channel<T> {
 
     #[inline(always)]
     unsafe fn take_message(&self) -> T {
-        #[cfg(feature = "loom")]
+        #[cfg(oneshot_loom)]
         {
             self.message.with(|ptr| ptr::read(ptr)).assume_init()
         }
 
-        #[cfg(not(feature = "loom"))]
+        #[cfg(not(oneshot_loom))]
         {
             ptr::read(self.message.get()).assume_init()
         }
@@ -1130,12 +1130,12 @@ impl<T> Channel<T> {
 
     #[inline(always)]
     unsafe fn take_waker(&self) -> ReceiverWaker {
-        #[cfg(feature = "loom")]
+        #[cfg(oneshot_loom)]
         {
             self.waker.with(|ptr| ptr::read(ptr)).assume_init()
         }
 
-        #[cfg(not(feature = "loom"))]
+        #[cfg(not(oneshot_loom))]
         {
             ptr::read(self.waker.get()).assume_init()
         }
@@ -1235,7 +1235,7 @@ impl ReceiverWaker {
     }
 }
 
-#[cfg(not(feature = "loom"))]
+#[cfg(not(oneshot_loom))]
 #[test]
 fn receiver_waker_size() {
     let expected: usize = match (cfg!(feature = "std"), cfg!(feature = "async")) {

tests/async.rs

@@ -1,4 +1,4 @@
-#![cfg(all(feature = "async", not(feature = "loom")))]
+#![cfg(all(feature = "async", not(oneshot_loom)))]
 
 use core::mem;
 use core::time::Duration;

tests/future.rs

@@ -2,9 +2,9 @@
 
 use core::{future, mem, pin, task};
 
-#[cfg(feature = "loom")]
+#[cfg(oneshot_loom)]
 pub use loom::sync::{Arc, Mutex};
-#[cfg(not(feature = "loom"))]
+#[cfg(not(oneshot_loom))]
 pub use std::sync::{Arc, Mutex};
 
 mod helpers;

tests/helpers/mod.rs

@@ -2,23 +2,23 @@
 
 extern crate alloc;
 
-#[cfg(not(feature = "loom"))]
+#[cfg(not(oneshot_loom))]
 use alloc::sync::Arc;
-#[cfg(not(feature = "loom"))]
+#[cfg(not(oneshot_loom))]
 use core::sync::atomic::{AtomicUsize, Ordering::SeqCst};
-#[cfg(feature = "loom")]
+#[cfg(oneshot_loom)]
 use loom::sync::{
     atomic::{AtomicUsize, Ordering::SeqCst},
     Arc,
 };
 
-#[cfg(feature = "loom")]
+#[cfg(oneshot_loom)]
 pub mod waker;
 
 pub fn maybe_loom_model(test: impl Fn() + Sync + Send + 'static) {
-    #[cfg(feature = "loom")]
+    #[cfg(oneshot_loom)]
     loom::model(test);
-    #[cfg(not(feature = "loom"))]
+    #[cfg(not(oneshot_loom))]
     test();
 }
 

tests/loom.rs

@@ -1,4 +1,4 @@
-#![cfg(feature = "loom")]
+#![cfg(oneshot_loom)]
 
 use oneshot::TryRecvError;
 

tests/raw.rs

@@ -1,4 +1,4 @@
-#![cfg(not(feature = "loom"))]
+#![cfg(not(oneshot_loom))]
 
 use oneshot::{channel, Receiver, Sender};
 

tests/sync.rs

@@ -8,12 +8,12 @@ use std::time::{Duration, Instant};
 
 #[cfg(feature = "std")]
 mod thread {
-    #[cfg(feature = "loom")]
+    #[cfg(oneshot_loom)]
     pub use loom::thread::spawn;
-    #[cfg(not(feature = "loom"))]
+    #[cfg(not(oneshot_loom))]
     pub use std::thread::{sleep, spawn};
 
-    #[cfg(feature = "loom")]
+    #[cfg(oneshot_loom)]
     pub fn sleep(_timeout: core::time::Duration) {
         loom::thread::yield_now()
     }
@@ -59,7 +59,7 @@ fn send_before_recv() {
     // FIXME: This test does not work with loom. There is something that happens after the
     // channel object becomes larger than ~500 bytes and that makes an atomic read from the state
     // result in "signal: 10, SIGBUS: access to undefined memory"
-    #[cfg(not(feature = "loom"))]
+    #[cfg(not(oneshot_loom))]
     maybe_loom_model(|| {
         let (sender, receiver) = oneshot::channel::<[u8; 4096]>();
         assert!(sender.send([0b10101010; 4096]).is_ok());
@@ -246,16 +246,16 @@ fn recv_deadline_and_timeout_no_time() {
 }
 
 // This test doesn't give meaningful results when run with oneshot_test_delay and loom
-#[cfg(all(feature = "std", not(all(oneshot_test_delay, feature = "loom"))))]
+#[cfg(all(feature = "std", not(all(oneshot_test_delay, oneshot_loom))))]
 #[test]
 fn recv_deadline_time_should_elapse() {
     maybe_loom_model(|| {
         let (_sender, receiver) = oneshot::channel::<u128>();
 
         let start = Instant::now();
-        #[cfg(not(feature = "loom"))]
+        #[cfg(not(oneshot_loom))]
         let timeout = Duration::from_millis(100);
-        #[cfg(feature = "loom")]
+        #[cfg(oneshot_loom)]
         let timeout = Duration::from_millis(1);
         assert_eq!(
             receiver.recv_deadline(start + timeout),
@@ -266,16 +266,16 @@ fn recv_deadline_time_should_elapse() {
     })
 }
 
-#[cfg(all(feature = "std", not(all(oneshot_test_delay, feature = "loom"))))]
+#[cfg(all(feature = "std", not(all(oneshot_test_delay, oneshot_loom))))]
 #[test]
 fn recv_timeout_time_should_elapse() {
     maybe_loom_model(|| {
         let (_sender, receiver) = oneshot::channel::<u128>();
 
         let start = Instant::now();
-        #[cfg(not(feature = "loom"))]
+        #[cfg(not(oneshot_loom))]
         let timeout = Duration::from_millis(100);
-        #[cfg(feature = "loom")]
+        #[cfg(oneshot_loom)]
         let timeout = Duration::from_millis(1);
 
         assert_eq!(
@@ -287,7 +287,7 @@ fn recv_timeout_time_should_elapse() {
     })
 }
 
-#[cfg(not(feature = "loom"))]
+#[cfg(not(oneshot_loom))]
 #[test]
 fn non_send_type_can_be_used_on_same_thread() {
     use std::ptr;