From 044862dbf776c03c697abb1f6d2d8a16cf3e0dbc Mon Sep 17 00:00:00 2001 From: Shahar Dawn Or Date: Sat, 18 Jun 2022 14:59:03 +0700 Subject: [PATCH] feat: return type does not have to be Clone BREAKING CHANGE: Signatures of `MemoizationStore::insert` and `MemoizationStore::get` altered. Co-authored-by: Roland Fredenhagen --- README.md | 4 +-- macro/src/lib.rs | 5 ++-- src/lib.rs | 24 ++++++++++------- ...nit_omitted_and_default_not_implemented.rs | 6 +++-- ...omitted_and_default_not_implemented.stderr | 4 +-- tests/test.rs | 26 ++++++++++++------- 6 files changed, 40 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 2e16533..4f603ef 100644 --- a/README.md +++ b/README.md @@ -128,12 +128,10 @@ The following apply to the key type and to the function's return type: - [`'static`]: key and return values are owned by a store which is owned by a static. - [`Send`] and [`Sync`]: for parallel access. -And the return type must be [`Clone`] because it is cloned for insertion into the store. - ## Store bounds Another source of bounds on the key type and the return type is the implementation of [`MemoizationStore`] for the store type. -By the way, the provided implementation of [`MemoizationStore`] for the default store type [`HashMap`] bounds `K: Eq + Hash`. +By the way, the provided implementation of [`MemoizationStore`] for the default store type [`HashMap`] bounds `K: Eq + Hash, R: Clone`. # Generic functions diff --git a/macro/src/lib.rs b/macro/src/lib.rs index 2211b96..0dbcc75 100644 --- a/macro/src/lib.rs +++ b/macro/src/lib.rs @@ -146,7 +146,7 @@ fn expand_fn_block(original_fn_block: Block, return_type: Type, attr_args: AttrA // However, since the concrete store is already obtained and since presumably the // following `::get` should be cheap, releasing the exclusive lock, obtaining a read lock // and obtaining the store again does not seem reasonable. - let attempt: ::core::option::Option<#return_type> = ::michie::MemoizationStore::get(store, #key_ref).cloned(); + let attempt: ::core::option::Option<#return_type> = ::michie::MemoizationStore::get(store, #key_ref); ::core::mem::drop(type_map_mutex_guard); if let ::core::option::Option::Some(hit) = attempt { hit @@ -170,8 +170,7 @@ fn expand_fn_block(original_fn_block: Block, return_type: Type, attr_args: AttrA } downcast_mut_with_inference_hint::<#store_type>(store, || #store_init).unwrap() }; - ::michie::MemoizationStore::insert(store, #key, ::core::clone::Clone::clone(&miss)); - miss + ::michie::MemoizationStore::insert(store, #key, miss) } }} } diff --git a/src/lib.rs b/src/lib.rs index 4c2b38c..1191936 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,30 +9,34 @@ use std::{ pub use michie_macro::memoized; pub trait MemoizationStore { - fn insert(&mut self, key: K, value: R); - fn get(&self, key: &K) -> Option<&R>; + fn insert(&mut self, key: K, value: R) -> R; + fn get(&self, key: &K) -> Option; } impl MemoizationStore for HashMap where K: Eq + Hash, + R: Clone, { - fn insert(&mut self, key: K, value: R) { - HashMap::insert(self, key, value); + fn insert(&mut self, key: K, value: R) -> R { + HashMap::insert(self, key, value.clone()); + value } - fn get(&self, key: &K) -> Option<&R> { - HashMap::get(self, key) + fn get(&self, key: &K) -> Option { + HashMap::get(self, key).cloned() } } impl MemoizationStore for BTreeMap where K: Ord, + R: Clone, { - fn insert(&mut self, key: K, value: R) { - BTreeMap::insert(self, key, value); + fn insert(&mut self, key: K, value: R) -> R { + BTreeMap::insert(self, key, value.clone()); + value } - fn get(&self, key: &K) -> Option<&R> { - BTreeMap::get(self, key) + fn get(&self, key: &K) -> Option { + BTreeMap::get(self, key).cloned() } } diff --git a/tests/compile_fail/store_init_omitted_and_default_not_implemented.rs b/tests/compile_fail/store_init_omitted_and_default_not_implemented.rs index 637c176..262c46d 100644 --- a/tests/compile_fail/store_init_omitted_and_default_not_implemented.rs +++ b/tests/compile_fail/store_init_omitted_and_default_not_implemented.rs @@ -2,8 +2,10 @@ use michie::{memoized, MemoizationStore}; struct Store; impl MemoizationStore for Store { - fn insert(&mut self, _key: usize, _value: usize) {} - fn get(&self, _key: &usize) -> Option<&usize> { + fn insert(&mut self, _key: usize, value: usize) -> usize { + value + } + fn get(&self, _key: &usize) -> Option { None } } diff --git a/tests/compile_fail/store_init_omitted_and_default_not_implemented.stderr b/tests/compile_fail/store_init_omitted_and_default_not_implemented.stderr index 7ec522f..4cdc9cd 100644 --- a/tests/compile_fail/store_init_omitted_and_default_not_implemented.stderr +++ b/tests/compile_fail/store_init_omitted_and_default_not_implemented.stderr @@ -1,7 +1,7 @@ error[E0277]: the trait bound `Store: Default` is not satisfied - --> tests/compile_fail/store_init_omitted_and_default_not_implemented.rs:10:1 + --> tests/compile_fail/store_init_omitted_and_default_not_implemented.rs:12:1 | -10 | #[memoized(key_expr = input, store_type = Store)] +12 | #[memoized(key_expr = input, store_type = Store)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Default` is not implemented for `Store` | = note: this error originates in the attribute macro `memoized` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/test.rs b/tests/test.rs index 28bca13..6a0123f 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -83,8 +83,10 @@ fn store_init_is_omitted() { } } impl MemoizationStore for Store { - fn insert(&mut self, _key: usize, _value: usize) {} - fn get(&self, _key: &usize) -> Option<&usize> { + fn insert(&mut self, _key: usize, value: usize) -> usize { + value + } + fn get(&self, _key: &usize) -> Option { None } } @@ -110,8 +112,10 @@ fn store_init_is_used_instead_of_implementation_of_the_default_trait() { } } impl MemoizationStore for Store { - fn insert(&mut self, _key: usize, _value: usize) {} - fn get(&self, _key: &usize) -> Option<&usize> { + fn insert(&mut self, _key: usize, value: usize) -> usize { + value + } + fn get(&self, _key: &usize) -> Option { None } } @@ -142,8 +146,10 @@ fn store_init_includes_a_concrete_store_type() { } } impl MemoizationStore for Store { - fn insert(&mut self, _key: K, _value: R) {} - fn get(&self, _key: &K) -> Option<&R> { + fn insert(&mut self, _key: K, value: R) -> R { + value + } + fn get(&self, _key: &K) -> Option { None } } @@ -165,8 +171,10 @@ fn store_init_includes_function_from_impl_block_that_has_bound_on_k_and_v() { } } impl MemoizationStore for Store<()> { - fn insert(&mut self, _key: usize, _value: usize) {} - fn get(&self, _key: &usize) -> Option<&usize> { + fn insert(&mut self, _key: usize, value: usize) -> usize { + value + } + fn get(&self, _key: &usize) -> Option { None } } @@ -193,7 +201,7 @@ fn trait_functions_are_called_explicitly() { } impl MemoizationStore<(), ()> for Store { fn insert(&mut self, _key: (), _value: ()) {} - fn get(&self, _key: &()) -> Option<&()> { + fn get(&self, _key: &()) -> Option<()> { None } }