Merge pull request #133 from moka-rs/remove-clone-from-key

Remove unnecessary `K: Clone` bound from some caches when they are `Clone`

.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:

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/

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "moka"
-version = "0.8.4"
+version = "0.8.5"
 edition = "2018"
 rust-version = "1.51"
 

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<K, V, S = RandomState> {
     base: BaseCache<K, V, S>,
 }
@@ -249,6 +248,19 @@ where
 {
 }
 
+// NOTE: We cannot do `#[derive(Clone)]` because it will add `Clone` bound to `K`.
+impl<K, V, S> Clone for Cache<K, V, S> {
+    /// 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<K, V, S> fmt::Debug for Cache<K, V, S>
 where
     K: Eq + Hash + fmt::Debug,

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<K, V, S = RandomState> {
     base: BaseCache<K, V, S>,
     value_initializer: Arc<ValueInitializer<K, V, S>>,
@@ -301,6 +300,20 @@ where
 {
 }
 
+// NOTE: We cannot do `#[derive(Clone)]` because it will add `Clone` bound to `K`.
+impl<K, V, S> Clone for Cache<K, V, S> {
+    /// 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<K, V> Cache<K, V, RandomState>
 where
     K: Hash + Eq + Send + Sync + 'static,

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");
     }
 }

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<K, V, S = RandomState> {
     base: BaseCache<K, V, S>,
     value_initializer: Arc<ValueInitializer<K, V, S>>,
@@ -250,6 +249,20 @@ where
 {
 }
 
+// NOTE: We cannot do `#[derive(Clone)]` because it will add `Clone` bound to `K`.
+impl<K, V, S> Clone for Cache<K, V, S> {
+    /// 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<K, V> Cache<K, V, RandomState>
 where
     K: Hash + Eq + Send + Sync + 'static,

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<MyKey, MyValue> = Cache::new(CAP);
+}
+
+fn f2_pass() {
+    let cache: Cache<MyKey, Arc<MyValue>> = Cache::new(CAP);
+    let _ = cache.clone();
+}
+
+fn f3_fail() {
+    // This should fail because S is not Clone.
+    let _cache: Cache<MyKey, Arc<MyValue>, _> = Cache::builder().build_with_hasher(MyBuildHasher1);
+}
+
+fn f4_pass() {
+    let cache: Cache<MyKey, Arc<MyValue>, _> = 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!()
+    }
+}

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<MyKey, MyValue> = Cache::new(CAP);
+    |                                         ^^^^^^^^^^ the trait `Clone` is not implemented for `MyValue`
+    |
+note: required by a bound in `moka::dash::Cache::<K, V>::new`
+   --> src/dash/cache.rs
+    |
+    |     V: Clone + Send + Sync + 'static,
+    |        ^^^^^ required by this bound in `moka::dash::Cache::<K, V>::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<MyKey, Arc<MyValue>, _> = 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::<K, V, moka::dash::Cache<K, V>>::build_with_hasher`
+   --> src/dash/builder.rs
+    |
+    |         S: BuildHasher + Clone + Send + Sync + 'static,
+    |                          ^^^^^ required by this bound in `moka::dash::CacheBuilder::<K, V, moka::dash::Cache<K, V>>::build_with_hasher`

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<MyKey, MyValue> = Cache::new(CAP);
+}
+
+fn f2_pass() {
+    let cache: Cache<MyKey, Arc<MyValue>> = Cache::new(CAP);
+    let _ = cache.clone();
+}
+
+fn f3_fail() {
+    // This should fail because S is not Clone.
+    let _cache: Cache<MyKey, Arc<MyValue>, _> = Cache::builder().build_with_hasher(MyBuildHasher1);
+}
+
+fn f4_pass() {
+    let cache: Cache<MyKey, Arc<MyValue>, _> = 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!()
+    }
+}

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<MyKey, MyValue> = Cache::new(CAP);
+    |                                         ^^^^^^^^^^ the trait `Clone` is not implemented for `MyValue`
+    |
+note: required by a bound in `moka::sync::Cache::<K, V>::new`
+   --> src/sync/cache.rs
+    |
+    |     V: Clone + Send + Sync + 'static,
+    |        ^^^^^ required by this bound in `moka::sync::Cache::<K, V>::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<MyKey, Arc<MyValue>, _> = 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::<K, V, moka::sync::Cache<K, V>>::build_with_hasher`
+   --> src/sync/builder.rs
+    |
+    |         S: BuildHasher + Clone + Send + Sync + 'static,
+    |                          ^^^^^ required by this bound in `moka::sync::CacheBuilder::<K, V, moka::sync::Cache<K, V>>::build_with_hasher`