From dae9b07a63c4917ced225af9aaa5284d2cc6bb0f Mon Sep 17 00:00:00 2001 From: Tatsuya Kawano Date: Sat, 21 May 2022 16:17:54 +0800 Subject: [PATCH 1/4] Remove unnecessary `K: Clone` bound from some caches when they are `Clone` - `sync::Cache` - `future::Cache` - `dash::Cache` --- src/dash/cache.rs | 14 +++- src/future/cache.rs | 15 ++++- src/lib.rs | 23 ++++--- src/sync/cache.rs | 15 ++++- .../dash/clone/dash_cache_clone.rs | 64 ++++++++++++++++++ .../dash/clone/dash_cache_clone.stderr | 25 +++++++ .../default/clone/sync_cache_clone.rs | 64 ++++++++++++++++++ .../default/clone/sync_cache_clone.stderr | 25 +++++++ .../default/clone/sync_seg_cache_clone.rs | 67 +++++++++++++++++++ .../default/clone/sync_seg_cache_clone.stderr | 25 +++++++ .../future/clone/future_cache_clone.rs | 65 ++++++++++++++++++ .../future/clone/future_cache_clone.stderr | 25 +++++++ 12 files changed, 416 insertions(+), 11 deletions(-) create mode 100644 tests/compile_tests/dash/clone/dash_cache_clone.rs create mode 100644 tests/compile_tests/dash/clone/dash_cache_clone.stderr create mode 100644 tests/compile_tests/default/clone/sync_cache_clone.rs create mode 100644 tests/compile_tests/default/clone/sync_cache_clone.stderr create mode 100644 tests/compile_tests/default/clone/sync_seg_cache_clone.rs create mode 100644 tests/compile_tests/default/clone/sync_seg_cache_clone.stderr create mode 100644 tests/compile_tests/future/clone/future_cache_clone.rs create mode 100644 tests/compile_tests/future/clone/future_cache_clone.stderr diff --git a/src/dash/cache.rs b/src/dash/cache.rs index 29d85a28..3f82945d 100644 --- a/src/dash/cache.rs +++ b/src/dash/cache.rs @@ -226,7 +226,6 @@ use std::{ /// [build-with-hasher-method]: ./struct.CacheBuilder.html#method.build_with_hasher /// [ahash-crate]: https://crates.io/crates/ahash /// -#[derive(Clone)] pub struct Cache { base: BaseCache, } @@ -249,6 +248,19 @@ where { } +// NOTE: We cannot do `#[derive(Clone)]` because it will add `Clone` bound to `K`. +impl Clone for Cache { + /// Makes a clone of this shared cache. + /// + /// This operation is cheap as it only creates thread-safe reference counted + /// pointers to the shared internal data structures. + fn clone(&self) -> Self { + Self { + base: self.base.clone(), + } + } +} + impl fmt::Debug for Cache where K: Eq + Hash + fmt::Debug, diff --git a/src/future/cache.rs b/src/future/cache.rs index 9f7afaaf..46e21563 100644 --- a/src/future/cache.rs +++ b/src/future/cache.rs @@ -277,7 +277,6 @@ use std::{ /// [build-with-hasher-method]: ./struct.CacheBuilder.html#method.build_with_hasher /// [ahash-crate]: https://crates.io/crates/ahash /// -#[derive(Clone)] pub struct Cache { base: BaseCache, value_initializer: Arc>, @@ -301,6 +300,20 @@ where { } +// NOTE: We cannot do `#[derive(Clone)]` because it will add `Clone` bound to `K`. +impl Clone for Cache { + /// Makes a clone of this shared cache. + /// + /// This operation is cheap as it only creates thread-safe reference counted + /// pointers to the shared internal data structures. + fn clone(&self) -> Self { + Self { + base: self.base.clone(), + value_initializer: Arc::clone(&self.value_initializer), + } + } +} + impl Cache where K: Hash + Eq + Send + Sync + 'static, diff --git a/src/lib.rs b/src/lib.rs index 0e9c504d..6227133c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -182,17 +182,24 @@ pub use sync::debug_counters::GlobalDebugCounters; #[cfg(test)] mod tests { - // #[cfg(trybuild)] - // #[test] - // fn ui_trybuild() { - // let t = trybuild::TestCases::new(); - // t.compile_fail("tests/ui/default/*.rs"); - // } + #[cfg(trybuild)] + #[test] + fn trybuild_default() { + let t = trybuild::TestCases::new(); + t.compile_fail("tests/compile_tests/default/clone/*.rs"); + } + + #[cfg(all(trybuild, feature = "dash"))] + #[test] + fn trybuild_dash() { + let t = trybuild::TestCases::new(); + t.compile_fail("tests/compile_tests/dash/clone/*.rs"); + } #[cfg(all(trybuild, feature = "future"))] #[test] - fn ui_trybuild_future() { + fn trybuild_future() { let t = trybuild::TestCases::new(); - t.compile_fail("tests/ui/future/*.rs"); + t.compile_fail("tests/compile_tests/future/clone/*.rs"); } } diff --git a/src/sync/cache.rs b/src/sync/cache.rs index 2029e440..0eb42963 100644 --- a/src/sync/cache.rs +++ b/src/sync/cache.rs @@ -226,7 +226,6 @@ use std::{ /// [build-with-hasher-method]: ./struct.CacheBuilder.html#method.build_with_hasher /// [ahash-crate]: https://crates.io/crates/ahash /// -#[derive(Clone)] pub struct Cache { base: BaseCache, value_initializer: Arc>, @@ -250,6 +249,20 @@ where { } +// NOTE: We cannot do `#[derive(Clone)]` because it will add `Clone` bound to `K`. +impl Clone for Cache { + /// Makes a clone of this shared cache. + /// + /// This operation is cheap as it only creates thread-safe reference counted + /// pointers to the shared internal data structures. + fn clone(&self) -> Self { + Self { + base: self.base.clone(), + value_initializer: Arc::clone(&self.value_initializer), + } + } +} + impl Cache where K: Hash + Eq + Send + Sync + 'static, diff --git a/tests/compile_tests/dash/clone/dash_cache_clone.rs b/tests/compile_tests/dash/clone/dash_cache_clone.rs new file mode 100644 index 00000000..ec46958f --- /dev/null +++ b/tests/compile_tests/dash/clone/dash_cache_clone.rs @@ -0,0 +1,64 @@ +// https://github.com/moka-rs/moka/issues/131 + +use std::{collections::hash_map::DefaultHasher, hash::BuildHasher, sync::Arc}; + +use moka::dash::Cache; + +fn main() { + f1_fail(); + f2_pass(); + f3_fail(); + f4_pass(); +} + +const CAP: u64 = 100; + +fn f1_fail() { + // This should fail because V is not Clone. + let _cache: Cache = Cache::new(CAP); +} + +fn f2_pass() { + let cache: Cache> = Cache::new(CAP); + let _ = cache.clone(); +} + +fn f3_fail() { + // This should fail because S is not Clone. + let _cache: Cache, _> = Cache::builder().build_with_hasher(MyBuildHasher1); +} + +fn f4_pass() { + let cache: Cache, _> = Cache::builder().build_with_hasher(MyBuildHasher2); + let _ = cache.clone(); +} + +// MyKey is not Clone. +#[derive(Hash, PartialEq, Eq)] +pub struct MyKey(i32); + +// MyValue is not Clone. +pub struct MyValue(i32); + +// MyBuildHasher1 is not Clone. +pub struct MyBuildHasher1; + +impl BuildHasher for MyBuildHasher1 { + type Hasher = DefaultHasher; + + fn build_hasher(&self) -> Self::Hasher { + unimplemented!() + } +} + +// MyBuildHasher1 is Clone. +#[derive(Clone)] +pub struct MyBuildHasher2; + +impl BuildHasher for MyBuildHasher2 { + type Hasher = DefaultHasher; + + fn build_hasher(&self) -> Self::Hasher { + unimplemented!() + } +} diff --git a/tests/compile_tests/dash/clone/dash_cache_clone.stderr b/tests/compile_tests/dash/clone/dash_cache_clone.stderr new file mode 100644 index 00000000..c611960c --- /dev/null +++ b/tests/compile_tests/dash/clone/dash_cache_clone.stderr @@ -0,0 +1,25 @@ +error[E0277]: the trait bound `MyValue: Clone` is not satisfied + --> tests/compile_tests/dash/clone/dash_cache_clone.rs:18:41 + | +18 | let _cache: Cache = Cache::new(CAP); + | ^^^^^^^^^^ the trait `Clone` is not implemented for `MyValue` + | +note: required by a bound in `moka::dash::Cache::::new` + --> src/dash/cache.rs + | + | V: Clone + Send + Sync + 'static, + | ^^^^^ required by this bound in `moka::dash::Cache::::new` + +error[E0277]: the trait bound `MyBuildHasher1: Clone` is not satisfied + --> tests/compile_tests/dash/clone/dash_cache_clone.rs:28:84 + | +28 | let _cache: Cache, _> = Cache::builder().build_with_hasher(MyBuildHasher1); + | ----------------- ^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `MyBuildHasher1` + | | + | required by a bound introduced by this call + | +note: required by a bound in `moka::dash::CacheBuilder::>::build_with_hasher` + --> src/dash/builder.rs + | + | S: BuildHasher + Clone + Send + Sync + 'static, + | ^^^^^ required by this bound in `moka::dash::CacheBuilder::>::build_with_hasher` diff --git a/tests/compile_tests/default/clone/sync_cache_clone.rs b/tests/compile_tests/default/clone/sync_cache_clone.rs new file mode 100644 index 00000000..dfe029ae --- /dev/null +++ b/tests/compile_tests/default/clone/sync_cache_clone.rs @@ -0,0 +1,64 @@ +// https://github.com/moka-rs/moka/issues/131 + +use std::{collections::hash_map::DefaultHasher, hash::BuildHasher, sync::Arc}; + +use moka::sync::Cache; + +fn main() { + f1_fail(); + f2_pass(); + f3_fail(); + f4_pass(); +} + +const CAP: u64 = 100; + +fn f1_fail() { + // This should fail because V is not Clone. + let _cache: Cache = Cache::new(CAP); +} + +fn f2_pass() { + let cache: Cache> = Cache::new(CAP); + let _ = cache.clone(); +} + +fn f3_fail() { + // This should fail because S is not Clone. + let _cache: Cache, _> = Cache::builder().build_with_hasher(MyBuildHasher1); +} + +fn f4_pass() { + let cache: Cache, _> = Cache::builder().build_with_hasher(MyBuildHasher2); + let _ = cache.clone(); +} + +// MyKey is not Clone. +#[derive(Hash, PartialEq, Eq)] +pub struct MyKey(i32); + +// MyValue is not Clone. +pub struct MyValue(i32); + +// MyBuildHasher1 is not Clone. +pub struct MyBuildHasher1; + +impl BuildHasher for MyBuildHasher1 { + type Hasher = DefaultHasher; + + fn build_hasher(&self) -> Self::Hasher { + unimplemented!() + } +} + +// MyBuildHasher1 is Clone. +#[derive(Clone)] +pub struct MyBuildHasher2; + +impl BuildHasher for MyBuildHasher2 { + type Hasher = DefaultHasher; + + fn build_hasher(&self) -> Self::Hasher { + unimplemented!() + } +} diff --git a/tests/compile_tests/default/clone/sync_cache_clone.stderr b/tests/compile_tests/default/clone/sync_cache_clone.stderr new file mode 100644 index 00000000..33d5307a --- /dev/null +++ b/tests/compile_tests/default/clone/sync_cache_clone.stderr @@ -0,0 +1,25 @@ +error[E0277]: the trait bound `MyValue: Clone` is not satisfied + --> tests/compile_tests/default/clone/sync_cache_clone.rs:18:41 + | +18 | let _cache: Cache = Cache::new(CAP); + | ^^^^^^^^^^ the trait `Clone` is not implemented for `MyValue` + | +note: required by a bound in `moka::sync::Cache::::new` + --> src/sync/cache.rs + | + | V: Clone + Send + Sync + 'static, + | ^^^^^ required by this bound in `moka::sync::Cache::::new` + +error[E0277]: the trait bound `MyBuildHasher1: Clone` is not satisfied + --> tests/compile_tests/default/clone/sync_cache_clone.rs:28:84 + | +28 | let _cache: Cache, _> = Cache::builder().build_with_hasher(MyBuildHasher1); + | ----------------- ^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `MyBuildHasher1` + | | + | required by a bound introduced by this call + | +note: required by a bound in `moka::sync::CacheBuilder::>::build_with_hasher` + --> src/sync/builder.rs + | + | S: BuildHasher + Clone + Send + Sync + 'static, + | ^^^^^ required by this bound in `moka::sync::CacheBuilder::>::build_with_hasher` diff --git a/tests/compile_tests/default/clone/sync_seg_cache_clone.rs b/tests/compile_tests/default/clone/sync_seg_cache_clone.rs new file mode 100644 index 00000000..b2b4b9b1 --- /dev/null +++ b/tests/compile_tests/default/clone/sync_seg_cache_clone.rs @@ -0,0 +1,67 @@ +// https://github.com/moka-rs/moka/issues/131 + +use std::{collections::hash_map::DefaultHasher, hash::BuildHasher, sync::Arc}; + +use moka::sync::SegmentedCache; + +fn main() { + f1_fail(); + f2_pass(); + f3_fail(); + f4_pass(); +} + +const CAP: u64 = 100; +const SEG: usize = 4; + +fn f1_fail() { + // This should fail because V is not Clone. + let _cache: SegmentedCache = SegmentedCache::new(CAP, SEG); +} + +fn f2_pass() { + let cache: SegmentedCache> = SegmentedCache::new(CAP, SEG); + let _ = cache.clone(); +} + +fn f3_fail() { + // This should fail because S is not Clone. + let _cache: SegmentedCache, _> = + SegmentedCache::builder(SEG).build_with_hasher(MyBuildHasher1); +} + +fn f4_pass() { + let cache: SegmentedCache, _> = + SegmentedCache::builder(SEG).build_with_hasher(MyBuildHasher2); + let _ = cache.clone(); +} + +// MyKey is not Clone. +#[derive(Hash, PartialEq, Eq)] +pub struct MyKey(i32); + +// MyValue is not Clone. +pub struct MyValue(i32); + +// MyBuildHasher1 is not Clone. +pub struct MyBuildHasher1; + +impl BuildHasher for MyBuildHasher1 { + type Hasher = DefaultHasher; + + fn build_hasher(&self) -> Self::Hasher { + unimplemented!() + } +} + +// MyBuildHasher1 is Clone. +#[derive(Clone)] +pub struct MyBuildHasher2; + +impl BuildHasher for MyBuildHasher2 { + type Hasher = DefaultHasher; + + fn build_hasher(&self) -> Self::Hasher { + unimplemented!() + } +} diff --git a/tests/compile_tests/default/clone/sync_seg_cache_clone.stderr b/tests/compile_tests/default/clone/sync_seg_cache_clone.stderr new file mode 100644 index 00000000..587300ef --- /dev/null +++ b/tests/compile_tests/default/clone/sync_seg_cache_clone.stderr @@ -0,0 +1,25 @@ +error[E0277]: the trait bound `MyValue: Clone` is not satisfied + --> tests/compile_tests/default/clone/sync_seg_cache_clone.rs:19:50 + | +19 | let _cache: SegmentedCache = SegmentedCache::new(CAP, SEG); + | ^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `MyValue` + | +note: required by a bound in `SegmentedCache::::new` + --> src/sync/segment.rs + | + | V: Clone + Send + Sync + 'static, + | ^^^^^ required by this bound in `SegmentedCache::::new` + +error[E0277]: the trait bound `MyBuildHasher1: Clone` is not satisfied + --> tests/compile_tests/default/clone/sync_seg_cache_clone.rs:30:56 + | +30 | SegmentedCache::builder(SEG).build_with_hasher(MyBuildHasher1); + | ----------------- ^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `MyBuildHasher1` + | | + | required by a bound introduced by this call + | +note: required by a bound in `moka::sync::CacheBuilder::>::build_with_hasher` + --> src/sync/builder.rs + | + | S: BuildHasher + Clone + Send + Sync + 'static, + | ^^^^^ required by this bound in `moka::sync::CacheBuilder::>::build_with_hasher` diff --git a/tests/compile_tests/future/clone/future_cache_clone.rs b/tests/compile_tests/future/clone/future_cache_clone.rs new file mode 100644 index 00000000..55ebfe71 --- /dev/null +++ b/tests/compile_tests/future/clone/future_cache_clone.rs @@ -0,0 +1,65 @@ +// https://github.com/moka-rs/moka/issues/131 + +use std::{collections::hash_map::DefaultHasher, hash::BuildHasher, sync::Arc}; + +use moka::future::Cache; + +#[tokio::main] +async fn main() { + f1_fail(); + f2_pass(); + f3_fail(); + f4_pass(); +} + +const CAP: u64 = 100; + +fn f1_fail() { + // This should fail because V is not Clone. + let _cache: Cache = Cache::new(CAP); +} + +fn f2_pass() { + let cache: Cache> = Cache::new(CAP); + let _ = cache.clone(); +} + +fn f3_fail() { + // This should fail because S is not Clone. + let _cache: Cache, _> = Cache::builder().build_with_hasher(MyBuildHasher1); +} + +fn f4_pass() { + let cache: Cache, _> = Cache::builder().build_with_hasher(MyBuildHasher2); + let _ = cache.clone(); +} + +// MyKey is not Clone. +#[derive(Hash, PartialEq, Eq)] +pub struct MyKey(i32); + +// MyValue is not Clone. +pub struct MyValue(i32); + +// MyBuildHasher1 is not Clone. +pub struct MyBuildHasher1; + +impl BuildHasher for MyBuildHasher1 { + type Hasher = DefaultHasher; + + fn build_hasher(&self) -> Self::Hasher { + unimplemented!() + } +} + +// MyBuildHasher1 is Clone. +#[derive(Clone)] +pub struct MyBuildHasher2; + +impl BuildHasher for MyBuildHasher2 { + type Hasher = DefaultHasher; + + fn build_hasher(&self) -> Self::Hasher { + unimplemented!() + } +} diff --git a/tests/compile_tests/future/clone/future_cache_clone.stderr b/tests/compile_tests/future/clone/future_cache_clone.stderr new file mode 100644 index 00000000..4bbfd621 --- /dev/null +++ b/tests/compile_tests/future/clone/future_cache_clone.stderr @@ -0,0 +1,25 @@ +error[E0277]: the trait bound `MyValue: Clone` is not satisfied + --> tests/compile_tests/future/clone/future_cache_clone.rs:19:41 + | +19 | let _cache: Cache = Cache::new(CAP); + | ^^^^^^^^^^ the trait `Clone` is not implemented for `MyValue` + | +note: required by a bound in `moka::future::Cache::::new` + --> src/future/cache.rs + | + | V: Clone + Send + Sync + 'static, + | ^^^^^ required by this bound in `moka::future::Cache::::new` + +error[E0277]: the trait bound `MyBuildHasher1: Clone` is not satisfied + --> tests/compile_tests/future/clone/future_cache_clone.rs:29:84 + | +29 | let _cache: Cache, _> = Cache::builder().build_with_hasher(MyBuildHasher1); + | ----------------- ^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `MyBuildHasher1` + | | + | required by a bound introduced by this call + | +note: required by a bound in `moka::future::CacheBuilder::>::build_with_hasher` + --> src/future/builder.rs + | + | S: BuildHasher + Clone + Send + Sync + 'static, + | ^^^^^ required by this bound in `moka::future::CacheBuilder::>::build_with_hasher` From 3c53781e043d6a8fdcf995f958280233a88bba9a Mon Sep 17 00:00:00 2001 From: Tatsuya Kawano Date: Sat, 21 May 2022 17:53:58 +0800 Subject: [PATCH 2/4] Update the CHANGELOG --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dade49a..73f5ccae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Moka Cache — Change Log +## Version 0.8.5 + +### Fixed + +- Remove unnecessary `K: Clone` bound from the following caches when they are `Clone` + ([#133][gh-pull-0133]): + - `sync::Cache` + - `future::Cache` + - Experimental `dash::Cache` + + ## Version 0.8.4 ### Fixed @@ -357,6 +368,7 @@ The minimum supported Rust version (MSRV) is now 1.51.0 (2021-03-25). [gh-issue-0034]: https://github.com/moka-rs/moka/issues/34/ [gh-issue-0031]: https://github.com/moka-rs/moka/issues/31/ +[gh-pull-0133]: https://github.com/moka-rs/moka/pull/133/ [gh-pull-0129]: https://github.com/moka-rs/moka/pull/129/ [gh-pull-0127]: https://github.com/moka-rs/moka/pull/127/ [gh-pull-0126]: https://github.com/moka-rs/moka/pull/126/ From 06251c96de7b2c7da3f879be4b026802b0d9bb4d Mon Sep 17 00:00:00 2001 From: Tatsuya Kawano Date: Sat, 21 May 2022 17:54:14 +0800 Subject: [PATCH 3/4] Bump the version to v0.8.5 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index d27f9ef8..a6309279 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "moka" -version = "0.8.4" +version = "0.8.5" edition = "2018" rust-version = "1.51" From 619bc0020c5705e76af9badff490261eff8d6982 Mon Sep 17 00:00:00 2001 From: Tatsuya Kawano Date: Sat, 21 May 2022 19:07:51 +0800 Subject: [PATCH 4/4] Rename a CI step: UI test -> complile error test --- .github/workflows/Skeptic.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/Skeptic.yml b/.github/workflows/Skeptic.yml index ba974a66..7c021aca 100644 --- a/.github/workflows/Skeptic.yml +++ b/.github/workflows/Skeptic.yml @@ -62,7 +62,7 @@ jobs: env: RUSTFLAGS: '--cfg skeptic' - - name: Run UI tests (future and dash features, trybuild) + - name: Run compile error tests (future and dash features, trybuild) uses: actions-rs/cargo@v1 if: ${{ matrix.rust == 'stable' }} with: