Use RUSTFLAGS instead of feature to gate benching

Currently we use the `"_bench_unstable"` feature to gate benching code.
This results in features not being additive for stable toolchains i.e.,
one cannot currently run `cargo +stable check --all-features`. Although
this method of gating bench code is widely proposed as a good solution
online there is another solution that keeps all features additive.

Guard the bench code with `#[cfg(bench)]` and when running benches use
`RUSTFLAGS='--cfg=bench'` to turn on `bench`.

Author: tcharding

.github/workflows/build.yml

@@ -273,7 +273,8 @@ jobs:
           cd ..
       - name: Run benchmarks on Rust ${{ matrix.toolchain }}
         run: |
-          RUSTC_BOOTSTRAP=1 cargo bench --features _bench_unstable
+
+          RUSTC_BOOTSTRAP=1 RUSTFLAGS='--cfg=bench' cargo bench
 
   check_commits:
     runs-on: ubuntu-latest

lightning-persister/Cargo.toml

@@ -14,7 +14,6 @@ all-features = true
 rustdoc-args = ["--cfg", "docsrs"]
 
 [features]
-_bench_unstable = ["lightning/_bench_unstable"]
 
 [dependencies]
 bitcoin = "0.29.0"

lightning-persister/src/lib.rs

@@ -8,8 +8,8 @@
 
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 
-#![cfg_attr(all(test, feature = "_bench_unstable"), feature(test))]
-#[cfg(all(test, feature = "_bench_unstable"))] extern crate test;
+#![cfg_attr(bench, feature(test))]
+#[cfg(bench)] extern crate test;
 
 mod util;
 
@@ -316,7 +316,7 @@ mod tests {
 	}
 }
 
-#[cfg(all(test, feature = "_bench_unstable"))]
+#[cfg(bench)]
 pub mod bench {
 	use test::Bencher;
 

lightning-rapid-gossip-sync/Cargo.toml

@@ -13,7 +13,6 @@ Utility to process gossip routing data from Rapid Gossip Sync Server.
 default = ["std"]
 no-std = ["lightning/no-std"]
 std = ["lightning/std"]
-_bench_unstable = []
 
 [dependencies]
 lightning = { version = "0.0.112", path = "../lightning", default-features = false }

lightning-rapid-gossip-sync/src/lib.rs

@@ -62,10 +62,8 @@
 
 #![cfg_attr(all(not(feature = "std"), not(test)), no_std)]
 
-// Allow and import test features for benching
-#![cfg_attr(all(test, feature = "_bench_unstable"), feature(test))]
-#[cfg(all(test, feature = "_bench_unstable"))]
-extern crate test;
+#![cfg_attr(bench, feature(test))]
+#[cfg(bench)] extern crate test;
 
 #[cfg(not(feature = "std"))]
 extern crate alloc;
@@ -278,7 +276,7 @@ mod tests {
 	}
 }
 
-#[cfg(all(test, feature = "_bench_unstable"))]
+#[cfg(bench)]
 pub mod bench {
 	use test::Bencher;
 

lightning/Cargo.toml

@@ -28,7 +28,6 @@ max_level_trace = []
 # Allow signing of local transactions that may have been revoked or will be revoked, for functional testing (e.g. justice tx handling).
 # This is unsafe to use in production because it may result in the counterparty publishing taking our funds.
 unsafe_revoked_tx_signing = []
-_bench_unstable = []
 
 no-std = ["hashbrown", "bitcoin/no-std", "core2/alloc"]
 std = ["bitcoin/std"]

lightning/src/lib.rs

@@ -54,8 +54,8 @@
 
 #![cfg_attr(all(not(feature = "std"), not(test)), no_std)]
 
-#![cfg_attr(all(any(test, feature = "_test_utils"), feature = "_bench_unstable"), feature(test))]
-#[cfg(all(any(test, feature = "_test_utils"), feature = "_bench_unstable"))] extern crate test;
+#![cfg_attr(bench, feature(test))]
+#[cfg(bench)] extern crate test;
 
 #[cfg(not(any(feature = "std", feature = "no-std")))]
 compile_error!("at least one of the `std` or `no-std` features must be enabled");
@@ -175,18 +175,18 @@ mod prelude {
 	pub use alloc::string::ToString;
 }
 
-#[cfg(all(not(feature = "_bench_unstable"), feature = "std", test))]
+#[cfg(all(not(bench), feature = "std", test))]
 mod debug_sync;
-#[cfg(all(not(feature = "_bench_unstable"), feature = "backtrace", feature = "std", test))]
+#[cfg(all(not(bench), feature = "backtrace", feature = "std", test))]
 extern crate backtrace;
 
 #[cfg(feature = "std")]
 mod sync {
-	#[cfg(all(not(feature = "_bench_unstable"), test))]
+	#[cfg(all(not(bench), test))]
 	pub use crate::debug_sync::*;
-	#[cfg(any(feature = "_bench_unstable", not(test)))]
+	#[cfg(any(bench, not(test)))]
 	pub use ::std::sync::{Arc, Mutex, Condvar, MutexGuard, RwLock, RwLockReadGuard, RwLockWriteGuard};
-	#[cfg(any(feature = "_bench_unstable", not(test)))]
+	#[cfg(any(bench, not(test)))]
 	pub use crate::util::fairrwlock::FairRwLock;
 }
 

lightning/src/ln/channelmanager.rs

@@ -8380,7 +8380,7 @@ mod tests {
 	}
 }
 
-#[cfg(all(any(test, feature = "_test_utils"), feature = "_bench_unstable"))]
+#[cfg(bench)]
 pub mod bench {
 	use crate::chain::Listen;
 	use crate::chain::chainmonitor::{ChainMonitor, Persist};

lightning/src/ln/functional_test_utils.rs

@@ -1470,7 +1470,7 @@ macro_rules! expect_pending_htlcs_forwardable_from_events {
 	}}
 }
 #[macro_export]
-#[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))]
+#[cfg(any(test, bench, feature = "_test_utils"))]
 macro_rules! expect_payment_received {
 	($node: expr, $expected_payment_hash: expr, $expected_payment_secret: expr, $expected_recv_value: expr) => {
 		expect_payment_received!($node, $expected_payment_hash, $expected_payment_secret, $expected_recv_value, None, $node.node.get_our_node_id())
@@ -1497,7 +1497,7 @@ macro_rules! expect_payment_received {
 }
 
 #[macro_export]
-#[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))]
+#[cfg(any(test, bench, feature = "_test_utils"))]
 macro_rules! expect_payment_claimed {
 	($node: expr, $expected_payment_hash: expr, $expected_recv_value: expr) => {
 		let events = $node.node.get_and_clear_pending_events();
@@ -1603,7 +1603,7 @@ macro_rules! expect_payment_forwarded {
 	}
 }
 
-#[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))]
+#[cfg(any(test, bench, feature = "_test_utils"))]
 pub fn expect_channel_ready_event<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, expected_counterparty_node_id: &PublicKey) {
 	let events = node.node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 1);

lightning/src/routing/gossip.rs

@@ -3280,7 +3280,7 @@ mod tests {
 	}
 }
 
-#[cfg(all(test, feature = "_bench_unstable"))]
+#[cfg(bench)]
 mod benches {
 	use super::*;