From 6f154ec8d7f3b156a380f57e366063af6d8b6df5 Mon Sep 17 00:00:00 2001 From: Fabian Schiebel Date: Fri, 22 Sep 2023 15:09:22 +0200 Subject: [PATCH 1/4] Add a variant of update() to the champ that does not mutate the tree in case the update function returns the already present value --- immer/detail/hamts/champ.hpp | 322 ++++++++++++++++++++++++++++++++++- immer/detail/util.hpp | 17 +- immer/map.hpp | 63 ++++++- test/algorithm.cpp | 17 ++ test/map/generic.ipp | 52 ++++-- 5 files changed, 448 insertions(+), 23 deletions(-) diff --git a/immer/detail/hamts/champ.hpp b/immer/detail/hamts/champ.hpp index 64a8cb82..4a350f77 100644 --- a/immer/detail/hamts/champ.hpp +++ b/immer/detail/hamts/champ.hpp @@ -12,6 +12,9 @@ #include #include +#include +#include +#include namespace immer { namespace detail { @@ -146,7 +149,8 @@ struct champ champ(node_t* r, size_t sz = 0) : root{r} , size{sz} - {} + { + } champ(const champ& other) : champ{other.root, other.size} @@ -841,6 +845,293 @@ struct champ return {res.node, new_size}; } + using update_mut_result = add_mut_result; + + template + struct TryUpdater + { + static update_result + do_try_update(node_t* node, + byval_if_possible k, + byval_if_possible, Fn&&> fn, + byval_if_possible valueEquals, + hash_t hash, + shift_t shift) + { + if (shift == max_shift) { + auto fst = node->collisions(); + auto lst = fst + node->collision_count(); + for (; fst != lst; ++fst) + if (Equal{}(*fst, k)) { + auto&& new_value = std::forward(fn)( + Project{}(detail::as_const(*fst))); + if (valueEquals(detail::as_const(new_value), + Project{}(detail::as_const(*fst)))) + return {nullptr, false}; + + return {node_t::copy_collision_replace( + node, + fst, + Combine{}(std::forward(k), + std::forward( + new_value))), + false}; + } + return {node_t::copy_collision_insert( + node, + Combine{}(std::forward(k), + std::forward(fn)(Default{}()))), + true}; + } else { + auto idx = (hash & (mask << shift)) >> shift; + auto bit = bitmap_t{1u} << idx; + if (node->nodemap() & bit) { + auto offset = node->children_count(bit); + auto result = do_try_update(node->children()[offset], + k, + std::forward(fn), + valueEquals, + hash, + shift + B); + if (!result.node) + return result; + + IMMER_TRY { + result.node = node_t::copy_inner_replace( + node, offset, result.node); + return result; + } + IMMER_CATCH (...) { + node_t::delete_deep_shift(result.node, shift + B); + IMMER_RETHROW; + } + } else if (node->datamap() & bit) { + auto offset = node->data_count(bit); + auto val = node->values() + offset; + if (Equal{}(*val, k)) { + auto&& new_value = std::forward(fn)( + Project{}(detail::as_const(*val))); + if (detail::as_const(new_value) == + Project{}(detail::as_const(*val))) + return {nullptr, false}; + + return {node_t::copy_inner_replace_value( + node, + offset, + Combine{}(std::forward(k), + std::forward( + new_value))), + false}; + } else { + auto child = node_t::make_merged( + shift + B, + Combine{}(std::forward(k), + std::forward(fn)(Default{}())), + hash, + *val, + Hash{}(*val)); + IMMER_TRY { + return {node_t::copy_inner_replace_merged( + node, bit, offset, child), + true}; + } + IMMER_CATCH (...) { + node_t::delete_deep_shift(child, shift + B); + IMMER_RETHROW; + } + } + } else { + return {node_t::copy_inner_insert_value( + node, + bit, + Combine{}(std::forward(k), + std::forward(fn)(Default{}()))), + true}; + } + } + } + + static update_mut_result + do_try_update_mut(edit_t e, + node_t* node, + byval_if_possible k, + byval_if_possible fn, + byval_if_possible valueEquals, + hash_t hash, + shift_t shift) + { + if (shift == max_shift) { + auto fst = node->collisions(); + auto lst = fst + node->collision_count(); + for (; fst != lst; ++fst) + if (Equal{}(*fst, k)) { + auto&& new_value = std::forward(fn)( + Project{}(detail::as_const(*fst))); + if (valueEquals(detail::as_const(new_value), + Project{}(detail::as_const(*fst)))) { + return {nullptr, false, false}; + } + + if (node->can_mutate(e)) { + *fst = Combine{}( + std::forward(k), + std::forward(new_value)); + return {node, false, true}; + } else { + auto r = node_t::copy_collision_replace( + node, + fst, + Combine{}(std::forward(k), + std::forward( + new_value))); + return {node_t::owned(r, e), false, false}; + } + } + auto v = Combine{}(std::forward(k), + std::forward(fn)(Default{}())); + auto mutate = node->can_mutate(e); + auto r = + mutate ? node_t::move_collision_insert(node, std::move(v)) + : node_t::copy_collision_insert(node, std::move(v)); + return {node_t::owned(r, e), true, mutate}; + } else { + auto idx = (hash & (mask << shift)) >> shift; + auto bit = bitmap_t{1u} << idx; + if (node->nodemap() & bit) { + auto offset = node->children_count(bit); + auto child = node->children()[offset]; + if (node->can_mutate(e)) { + auto result = do_try_update_mut(e, + child, + k, + std::forward(fn), + valueEquals, + hash, + shift + B); + if (!result.node) + return result; + + node->children()[offset] = result.node; + if (!result.mutated && child->dec()) + node_t::delete_deep_shift(child, shift + B); + return {node, result.added, true}; + } else { + auto result = do_try_update(child, + k, + std::forward(fn), + valueEquals, + hash, + shift + B); + if (!result.node) + return {nullptr, false, false}; + + IMMER_TRY { + result.node = node_t::copy_inner_replace( + node, offset, result.node); + node_t::owned(result.node, e); + return {result.node, result.added, false}; + } + IMMER_CATCH (...) { + node_t::delete_deep_shift(result.node, shift + B); + IMMER_RETHROW; + } + } + } else if (node->datamap() & bit) { + auto offset = node->data_count(bit); + auto val = node->values() + offset; + if (Equal{}(*val, k)) { + if (node->can_mutate(e)) { + auto vals = node->ensure_mutable_values(e); + auto&& new_value = std::forward(fn)( + Project{}(detail::as_const(vals[offset]))); + if (valueEquals( + detail::as_const(new_value), + Project{}(detail::as_const(vals[offset])))) + return {nullptr, false, false}; + + vals[offset] = Combine{}( + std::forward(k), + std::forward(new_value)); + return {node, false, true}; + } else { + auto&& new_value = std::forward(fn)( + Project{}(detail::as_const(*val))); + if (valueEquals(detail::as_const(new_value), + Project{}(detail::as_const(*val)))) + return {nullptr, false, false}; + + auto r = node_t::copy_inner_replace_value( + node, + offset, + Combine{}(std::forward(k), + std::forward( + new_value))); + return {node_t::owned_values(r, e), false, false}; + } + } else { + auto mutate = node->can_mutate(e); + auto mutate_values = + mutate && node->can_mutate_values(e); + auto hash2 = Hash{}(*val); + auto child = node_t::make_merged_e( + e, + shift + B, + Combine{}(std::forward(k), + std::forward(fn)(Default{}())), + hash, + mutate_values ? std::move(*val) : *val, + hash2); + IMMER_TRY { + auto r = mutate ? node_t::move_inner_replace_merged( + e, node, bit, offset, child) + : node_t::copy_inner_replace_merged( + node, bit, offset, child); + return { + node_t::owned_values_safe(r, e), true, mutate}; + } + IMMER_CATCH (...) { + node_t::delete_deep_shift(child, shift + B); + IMMER_RETHROW; + } + } + } else { + auto mutate = node->can_mutate(e); + auto v = Combine{}(std::forward(k), + std::forward(fn)(Default{}())); + auto r = mutate ? node_t::move_inner_insert_value( + e, node, bit, std::move(v)) + : node_t::copy_inner_insert_value( + node, bit, std::move(v)); + return {node_t::owned_values(r, e), true, mutate}; + } + } + } + }; + + template > + champ try_update(const K& k, Fn&& fn, ValueEquals valueEquals = {}) const + { + auto hash = Hash{}(k); + auto res = TryUpdater:: + do_try_update( + root, k, std::forward(fn), std::move(valueEquals), hash, 0); + if (!res.node) + return {root->inc(), size}; + + auto new_size = size + size_t(res.added); + return {res.node, new_size}; + } + template node_t* do_update_if_exists( node_t* node, K&& k, Fn&& fn, hash_t hash, shift_t shift) const @@ -909,8 +1200,6 @@ struct champ }; } - using update_mut_result = add_mut_result; - template + void try_update_mut(edit_t e, const K& k, Fn&& fn, ValueEquals valueEquals) + { + auto hash = Hash{}(k); + auto res = TryUpdater:: + do_try_update_mut( + e, root, k, std::forward(fn), valueEquals, hash, 0); + if (!res.node) + return; + + if (!res.mutated && root->dec()) + node_t::delete_deep(root, 0); + root = res.node; + size += res.added ? 1 : 0; + } + struct update_if_exists_mut_result { node_t* node; @@ -1300,13 +1610,15 @@ struct champ , data{a.data} , owned{false} , mutated{false} - {} + { + } sub_result_mut(sub_result a, bool m) : kind{a.kind} , data{a.data} , owned{false} , mutated{m} - {} + { + } sub_result_mut() : kind{kind_t::nothing} , mutated{false} {}; diff --git a/immer/detail/util.hpp b/immer/detail/util.hpp index d6ae246b..279c7b1e 100644 --- a/immer/detail/util.hpp +++ b/immer/detail/util.hpp @@ -102,9 +102,9 @@ auto destroy_n(Iter first, Size n) noexcept template constexpr bool can_trivially_copy = std::is_same::value_type, - typename std::iterator_traits::value_type>::value&& - std::is_trivially_copyable< - typename std::iterator_traits::value_type>::value; + typename std::iterator_traits::value_type>::value && + std::is_trivially_copyable< + typename std::iterator_traits::value_type>::value; template auto uninitialized_move(Iter1 first, Iter1 last, Iter2 out) noexcept @@ -233,7 +233,8 @@ auto static_if(F&& f) -> std::enable_if_t } template auto static_if(F&& f) -> std::enable_if_t -{} +{ +} template auto static_if(F1&& f1, F2&& f2) -> std::enable_if_t @@ -310,5 +311,13 @@ distance(Iterator first, Sentinel last) return last - first; } +template +static constexpr bool can_efficiently_pass_by_value = + sizeof(T) <= 2 * sizeof(void*) && std::is_trivially_copyable::value; + +template +using byval_if_possible = + std::conditional_t, T, OrElse>; + } // namespace detail } // namespace immer diff --git a/immer/map.hpp b/immer/map.hpp index cab8e5d5..902d48f3 100644 --- a/immer/map.hpp +++ b/immer/map.hpp @@ -177,7 +177,8 @@ class map */ map(std::initializer_list values) : impl_{impl_t::from_initializer_list(values)} - {} + { + } /*! * Constructs a map containing the elements in the range @@ -189,7 +190,8 @@ class map bool> = true> map(Iter first, Sent last) : impl_{impl_t::from_range(first, last)} - {} + { + } /*! * Default constructor. It creates a map of `size() == 0`. It @@ -424,6 +426,37 @@ class map return update_move(move_t{}, std::move(k), std::forward(fn)); } + /*! + * Returns a map replacing the association `(k, v)` by the + * association new association `(k, fn(v))`, where `v` is the + * currently associated value for `k` in the map or a default + * constructed value otherwise. It may allocate memory + * and its complexity is *effectively* @f$ O(1) @f$. + * + * If `fn(v) == v`, the map remains unchanged and no memory is allocated. + * You may customize the equality comparison for values by setting the + * ValueEquals callback + */ + template > + IMMER_NODISCARD map try_update(key_type k, + Fn&& fn, + ValueEquals valueEquals = {}) const& + { + return impl_ + .template try_update( + std::move(k), std::forward(fn), std::move(valueEquals)); + } + + template > + IMMER_NODISCARD decltype(auto) + try_update(key_type k, Fn&& fn, ValueEquals valueEquals = {}) && + { + return try_update_move(move_t{}, + std::move(k), + std::forward(fn), + std::move(valueEquals)); + } + /*! * Returns a map replacing the association `(k, v)` by the association new * association `(k, fn(v))`, where `v` is the currently associated value for @@ -516,6 +549,29 @@ class map std::move(k), std::forward(fn)); } + template + map&& try_update_move(std::true_type, + key_type k, + Fn&& fn, + ValueEquals valueEquals) + { + impl_.template try_update_mut( + {}, std::move(k), std::forward(fn), std::move(valueEquals)); + return std::move(*this); + } + template + map try_update_move(std::false_type, + key_type k, + Fn&& fn, + ValueEquals valueEquals) + { + return impl_ + .template try_update( + std::move(k), std::forward(fn), std::move(valueEquals)); + } + template map&& update_if_exists_move(std::true_type, key_type k, Fn&& fn) { @@ -542,7 +598,8 @@ class map map(impl_t impl) : impl_(std::move(impl)) - {} + { + } impl_t impl_ = impl_t::empty(); }; diff --git a/test/algorithm.cpp b/test/algorithm.cpp index 6a785dbd..69cffb40 100644 --- a/test/algorithm.cpp +++ b/test/algorithm.cpp @@ -150,6 +150,23 @@ TEST_CASE("update maps") do_check(immer::table{}); } +TEST_CASE("try update maps") +{ + auto do_check = [](auto v) { + (void) v.try_update(0, [](auto&& x) { + using type_t = std::decay_t; + // for maps, we actually do not make a copy at all but pase the + // original instance directly, as const.. + static_assert(std::is_same::value, ""); + return x; + }); + }; + + do_check(immer::map{}); + // -- tables not supported yet with try_update + // do_check(immer::table{}); +} + TEST_CASE("update_if_exists maps") { auto do_check = [](auto v) { diff --git a/test/map/generic.ipp b/test/map/generic.ipp index e9b6cf58..f3b5af63 100644 --- a/test/map/generic.ipp +++ b/test/map/generic.ipp @@ -171,6 +171,9 @@ TEST_CASE("equals and setting") CHECK(v.set(1234, 42) == v.insert({1234, 42})); CHECK(v.update(1234, [](auto&& x) { return x + 1; }) == v.set(1234, 1)); CHECK(v.update(42, [](auto&& x) { return x + 1; }) == v.set(42, 43)); + CHECK(v.try_update(42, [](auto&& x) { return x + 1; }) == v.set(42, 43)); + CHECK(v.try_update(42, [](auto&& x) { return x; }).identity() == + v.identity()); CHECK(v.update_if_exists(1234, [](auto&& x) { return x + 1; }) == v); CHECK(v.update_if_exists(42, [](auto&& x) { return x + 1; }) == @@ -282,6 +285,22 @@ TEST_CASE("update a lot") } } + SECTION("try_update immutable") + { + for (decltype(v.size()) i = 0; i < v.size(); ++i) { + v = v.try_update(i, [](auto&& x) { return x + 1; }); + CHECK(v[i] == i + 1); + } + } + + SECTION("try_update move") + { + for (decltype(v.size()) i = 0; i < v.size(); ++i) { + v = std::move(v).try_update(i, [](auto&& x) { return x + 1; }); + CHECK(v[i] == i + 1); + } + } + SECTION("erase") { for (decltype(v.size()) i = 0; i < v.size(); ++i) { @@ -439,7 +458,8 @@ TEST_CASE("exception safety") auto s = d.next(); v = v.update(i, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) {} + } catch (dada_error) { + } for (auto i : test_irange(0u, i)) CHECK(v.at(i) == i + 1); for (auto i : test_irange(i, n)) @@ -460,7 +480,8 @@ TEST_CASE("exception safety") auto s = d.next(); v = v.update_if_exists(i, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) {} + } catch (dada_error) { + } for (auto i : test_irange(0u, i)) CHECK(v.at(i) == i + 1); for (auto i : test_irange(i, n)) @@ -482,7 +503,8 @@ TEST_CASE("exception safety") auto s = d.next(); v = v.update(vals[i].first, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) {} + } catch (dada_error) { + } for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -505,7 +527,8 @@ TEST_CASE("exception safety") v = v.update_if_exists(vals[i].first, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) {} + } catch (dada_error) { + } for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -528,7 +551,8 @@ TEST_CASE("exception safety") auto x = vals[i].second; v = v.set(vals[i].first, x + 1); ++i; - } catch (dada_error) {} + } catch (dada_error) { + } for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -551,7 +575,8 @@ TEST_CASE("exception safety") auto x = vals[i].second; v = std::move(v).set(vals[i].first, x + 1); ++i; - } catch (dada_error) {} + } catch (dada_error) { + } for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -574,7 +599,8 @@ TEST_CASE("exception safety") v = std::move(v).update(vals[i].first, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) {} + } catch (dada_error) { + } for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -597,7 +623,8 @@ TEST_CASE("exception safety") v = std::move(v).update_if_exists(vals[i].first, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) {} + } catch (dada_error) { + } for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -619,7 +646,8 @@ TEST_CASE("exception safety") // auto s = d.next(); v = std::move(v).erase(vals[i].first); ++i; - } catch (dada_error) {} + } catch (dada_error) { + } for (auto i : test_irange(0u, i)) CHECK(v.count(vals[i].first) == 0); for (auto i : test_irange(i, n)) @@ -635,7 +663,8 @@ struct KeyType { explicit KeyType(unsigned v) : value(v) - {} + { + } unsigned value; }; @@ -643,7 +672,8 @@ struct LookupType { explicit LookupType(unsigned v) : value(v) - {} + { + } unsigned value; }; From 7eec5f93f33c09d288fc25e4f96975f26a0b0777 Mon Sep 17 00:00:00 2001 From: Fabian Schiebel Date: Sat, 23 Sep 2023 12:50:32 +0200 Subject: [PATCH 2/4] Perfectly forward the key into update and try_update + add more tests --- immer/detail/hamts/champ.hpp | 41 ++++++++------ immer/detail/util.hpp | 4 +- test/map/generic.ipp | 102 ++++++++++++++++++++++++++++++++--- 3 files changed, 123 insertions(+), 24 deletions(-) diff --git a/immer/detail/hamts/champ.hpp b/immer/detail/hamts/champ.hpp index 4a350f77..bf679f25 100644 --- a/immer/detail/hamts/champ.hpp +++ b/immer/detail/hamts/champ.hpp @@ -836,11 +836,11 @@ struct champ typename Combine, typename K, typename Fn> - champ update(const K& k, Fn&& fn) const + champ update(K&& k, Fn&& fn) const { auto hash = Hash{}(k); auto res = do_update( - root, k, std::forward(fn), hash, 0); + root, std::forward(k), std::forward(fn), hash, 0); auto new_size = size + (res.added ? 1 : 0); return {res.node, new_size}; } @@ -857,8 +857,8 @@ struct champ { static update_result do_try_update(node_t* node, - byval_if_possible k, - byval_if_possible, Fn&&> fn, + byval_if_possible k, + byval_if_possible fn, byval_if_possible valueEquals, hash_t hash, shift_t shift) @@ -893,7 +893,7 @@ struct champ if (node->nodemap() & bit) { auto offset = node->children_count(bit); auto result = do_try_update(node->children()[offset], - k, + std::forward(k), std::forward(fn), valueEquals, hash, @@ -959,7 +959,7 @@ struct champ static update_mut_result do_try_update_mut(edit_t e, node_t* node, - byval_if_possible k, + byval_if_possible k, byval_if_possible fn, byval_if_possible valueEquals, hash_t hash, @@ -1008,7 +1008,7 @@ struct champ if (node->can_mutate(e)) { auto result = do_try_update_mut(e, child, - k, + std::forward(k), std::forward(fn), valueEquals, hash, @@ -1022,7 +1022,7 @@ struct champ return {node, result.added, true}; } else { auto result = do_try_update(child, - k, + std::forward(k), std::forward(fn), valueEquals, hash, @@ -1119,12 +1119,16 @@ struct champ typename K, typename Fn, typename ValueEquals = std::equal_to> - champ try_update(const K& k, Fn&& fn, ValueEquals valueEquals = {}) const + champ try_update(K&& k, Fn&& fn, ValueEquals valueEquals = {}) const { auto hash = Hash{}(k); auto res = TryUpdater:: - do_try_update( - root, k, std::forward(fn), std::move(valueEquals), hash, 0); + do_try_update(root, + std::forward(k), + std::forward(fn), + std::move(valueEquals), + hash, + 0); if (!res.node) return {root->inc(), size}; @@ -1326,11 +1330,11 @@ struct champ typename Combine, typename K, typename Fn> - void update_mut(edit_t e, const K& k, Fn&& fn) + void update_mut(edit_t e, K&& k, Fn&& fn) { auto hash = Hash{}(k); auto res = do_update_mut( - e, root, k, std::forward(fn), hash, 0); + e, root, std::forward(k), std::forward(fn), hash, 0); if (!res.mutated && root->dec()) node_t::delete_deep(root, 0); root = res.node; @@ -1343,12 +1347,17 @@ struct champ typename K, typename Fn, typename ValueEquals> - void try_update_mut(edit_t e, const K& k, Fn&& fn, ValueEquals valueEquals) + void try_update_mut(edit_t e, K&& k, Fn&& fn, ValueEquals valueEquals) { auto hash = Hash{}(k); auto res = TryUpdater:: - do_try_update_mut( - e, root, k, std::forward(fn), valueEquals, hash, 0); + do_try_update_mut(e, + root, + std::forward(k), + std::forward(fn), + valueEquals, + hash, + 0); if (!res.node) return; diff --git a/immer/detail/util.hpp b/immer/detail/util.hpp index 279c7b1e..8494b628 100644 --- a/immer/detail/util.hpp +++ b/immer/detail/util.hpp @@ -317,7 +317,9 @@ static constexpr bool can_efficiently_pass_by_value = template using byval_if_possible = - std::conditional_t, T, OrElse>; + std::conditional_t>, + std::decay_t, + OrElse>; } // namespace detail } // namespace immer diff --git a/test/map/generic.ipp b/test/map/generic.ipp index f3b5af63..f893cefd 100644 --- a/test/map/generic.ipp +++ b/test/map/generic.ipp @@ -334,11 +334,11 @@ TEST_CASE("update_if_exists a lot") #if !IMMER_IS_LIBGC_TEST TEST_CASE("update boxed move string") { - constexpr auto N = 666u; - constexpr auto S = 7; + static constexpr auto N = 666u; + static constexpr auto S = 7; auto s = MAP_T>{}; - SECTION("preserve immutability") - { + + auto do_test = [&s](auto Updater) { auto s0 = s; auto i0 = 0u; // insert @@ -347,8 +347,9 @@ TEST_CASE("update boxed move string") s0 = s; i0 = i; } - s = std::move(s).update(std::to_string(i), - [&](auto&&) { return std::to_string(i); }); + s = Updater(std::move(s), std::to_string(i), [&](auto&&) { + return std::to_string(i); + }); { CHECK(s.size() == i + 1); for (auto j : test_irange(0u, i + 1)) { @@ -374,7 +375,7 @@ TEST_CASE("update boxed move string") s0 = s; i0 = i; } - s = std::move(s).update(std::to_string(i), [&](auto&&) { + s = Updater(std::move(s), std::to_string(i), [&](auto&&) { return std::to_string(i + 1); }); { @@ -392,6 +393,24 @@ TEST_CASE("update boxed move string") CHECK(*s0.find(std::to_string(j)) == std::to_string(j)); } } + }; + + SECTION("preserve immutability") + { + do_test([](auto&& map, auto&& key, auto&& cb) { + return std::forward(map).update( + std::forward(key), + std::forward(cb)); + }); + } + + SECTION("preserve immutability (try_update)") + { + do_test([](auto&& map, auto&& key, auto&& cb) { + return std::forward(map).try_update( + std::forward(key), + std::forward(cb)); + }); } } #endif @@ -469,6 +488,28 @@ TEST_CASE("exception safety") IMMER_TRACE_E(d.happenings); } + SECTION("try_update") + { + auto v = dadaist_map_t{}; + auto d = dadaism{}; + for (auto i = 0u; i < n; ++i) + v = std::move(v).set(i, i); + for (auto i = 0u; i < v.size();) { + try { + auto s = d.next(); + v = v.try_update(i, [](auto x) { return x + 1; }); + ++i; + } catch (dada_error) { + } + for (auto i : test_irange(0u, i)) + CHECK(v.at(i) == i + 1); + for (auto i : test_irange(i, n)) + CHECK(v.at(i) == i); + } + CHECK(d.happenings > 0); + IMMER_TRACE_E(d.happenings); + } + SECTION("update_if_exists") { auto v = dadaist_map_t{}; @@ -514,6 +555,29 @@ TEST_CASE("exception safety") IMMER_TRACE_E(d.happenings); } + SECTION("try_update collisisions") + { + auto vals = make_values_with_collisions(n); + auto v = dadaist_conflictor_map_t{}; + auto d = dadaism{}; + for (auto i = 0u; i < n; ++i) + v = v.insert(vals[i]); + for (auto i = 0u; i < v.size();) { + try { + auto s = d.next(); + v = v.try_update(vals[i].first, [](auto x) { return x + 1; }); + ++i; + } catch (dada_error) { + } + for (auto i : test_irange(0u, i)) + CHECK(v.at(vals[i].first) == vals[i].second + 1); + for (auto i : test_irange(i, n)) + CHECK(v.at(vals[i].first) == vals[i].second); + } + CHECK(d.happenings > 0); + IMMER_TRACE_E(d.happenings); + } + SECTION("update_if_exists collisisions") { auto vals = make_values_with_collisions(n); @@ -610,6 +674,30 @@ TEST_CASE("exception safety") IMMER_TRACE_E(d.happenings); } + SECTION("try_update collisisions move") + { + auto vals = make_values_with_collisions(n); + auto v = dadaist_conflictor_map_t{}; + auto d = dadaism{}; + for (auto i = 0u; i < n; ++i) + v = std::move(v).insert(vals[i]); + for (auto i = 0u; i < v.size();) { + try { + auto s = d.next(); + v = std::move(v).try_update(vals[i].first, + [](auto x) { return x + 1; }); + ++i; + } catch (dada_error) { + } + for (auto i : test_irange(0u, i)) + CHECK(v.at(vals[i].first) == vals[i].second + 1); + for (auto i : test_irange(i, n)) + CHECK(v.at(vals[i].first) == vals[i].second); + } + CHECK(d.happenings > 0); + IMMER_TRACE_E(d.happenings); + } + SECTION("update_if_exists collisisions move") { auto vals = make_values_with_collisions(n); From 0ae2cbad5cde63ce6956d0fcb7c5edb540f092e6 Mon Sep 17 00:00:00 2001 From: Fabian Schiebel Date: Thu, 14 Dec 2023 18:04:17 +0100 Subject: [PATCH 3/4] Avoid changes due to auto-formatting --- immer/detail/util.hpp | 9 ++++----- immer/map.hpp | 9 +++------ test/map/generic.ipp | 42 ++++++++++++++---------------------------- 3 files changed, 21 insertions(+), 39 deletions(-) diff --git a/immer/detail/util.hpp b/immer/detail/util.hpp index 8494b628..ec14bd61 100644 --- a/immer/detail/util.hpp +++ b/immer/detail/util.hpp @@ -102,9 +102,9 @@ auto destroy_n(Iter first, Size n) noexcept template constexpr bool can_trivially_copy = std::is_same::value_type, - typename std::iterator_traits::value_type>::value && - std::is_trivially_copyable< - typename std::iterator_traits::value_type>::value; + typename std::iterator_traits::value_type>::value&& + std::is_trivially_copyable< + typename std::iterator_traits::value_type>::value; template auto uninitialized_move(Iter1 first, Iter1 last, Iter2 out) noexcept @@ -233,8 +233,7 @@ auto static_if(F&& f) -> std::enable_if_t } template auto static_if(F&& f) -> std::enable_if_t -{ -} +{} template auto static_if(F1&& f1, F2&& f2) -> std::enable_if_t diff --git a/immer/map.hpp b/immer/map.hpp index 902d48f3..f3172d4e 100644 --- a/immer/map.hpp +++ b/immer/map.hpp @@ -177,8 +177,7 @@ class map */ map(std::initializer_list values) : impl_{impl_t::from_initializer_list(values)} - { - } + {} /*! * Constructs a map containing the elements in the range @@ -190,8 +189,7 @@ class map bool> = true> map(Iter first, Sent last) : impl_{impl_t::from_range(first, last)} - { - } + {} /*! * Default constructor. It creates a map of `size() == 0`. It @@ -598,8 +596,7 @@ class map map(impl_t impl) : impl_(std::move(impl)) - { - } + {} impl_t impl_ = impl_t::empty(); }; diff --git a/test/map/generic.ipp b/test/map/generic.ipp index f893cefd..b91bbed7 100644 --- a/test/map/generic.ipp +++ b/test/map/generic.ipp @@ -477,8 +477,7 @@ TEST_CASE("exception safety") auto s = d.next(); v = v.update(i, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.at(i) == i + 1); for (auto i : test_irange(i, n)) @@ -499,8 +498,7 @@ TEST_CASE("exception safety") auto s = d.next(); v = v.try_update(i, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.at(i) == i + 1); for (auto i : test_irange(i, n)) @@ -521,8 +519,7 @@ TEST_CASE("exception safety") auto s = d.next(); v = v.update_if_exists(i, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.at(i) == i + 1); for (auto i : test_irange(i, n)) @@ -544,8 +541,7 @@ TEST_CASE("exception safety") auto s = d.next(); v = v.update(vals[i].first, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -567,8 +563,7 @@ TEST_CASE("exception safety") auto s = d.next(); v = v.try_update(vals[i].first, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -591,8 +586,7 @@ TEST_CASE("exception safety") v = v.update_if_exists(vals[i].first, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -615,8 +609,7 @@ TEST_CASE("exception safety") auto x = vals[i].second; v = v.set(vals[i].first, x + 1); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -639,8 +632,7 @@ TEST_CASE("exception safety") auto x = vals[i].second; v = std::move(v).set(vals[i].first, x + 1); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -663,8 +655,7 @@ TEST_CASE("exception safety") v = std::move(v).update(vals[i].first, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -687,8 +678,7 @@ TEST_CASE("exception safety") v = std::move(v).try_update(vals[i].first, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -711,8 +701,7 @@ TEST_CASE("exception safety") v = std::move(v).update_if_exists(vals[i].first, [](auto x) { return x + 1; }); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.at(vals[i].first) == vals[i].second + 1); for (auto i : test_irange(i, n)) @@ -734,8 +723,7 @@ TEST_CASE("exception safety") // auto s = d.next(); v = std::move(v).erase(vals[i].first); ++i; - } catch (dada_error) { - } + } catch (dada_error) {} for (auto i : test_irange(0u, i)) CHECK(v.count(vals[i].first) == 0); for (auto i : test_irange(i, n)) @@ -751,8 +739,7 @@ struct KeyType { explicit KeyType(unsigned v) : value(v) - { - } + {} unsigned value; }; @@ -760,8 +747,7 @@ struct LookupType { explicit LookupType(unsigned v) : value(v) - { - } + {} unsigned value; }; From 7f77e9bc85bc71d20db3351c4323ab1f40cfc176 Mon Sep 17 00:00:00 2001 From: Fabian Schiebel Date: Thu, 14 Dec 2023 18:06:48 +0100 Subject: [PATCH 4/4] minor reformatting --- immer/detail/hamts/champ.hpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/immer/detail/hamts/champ.hpp b/immer/detail/hamts/champ.hpp index bf679f25..6ab8a563 100644 --- a/immer/detail/hamts/champ.hpp +++ b/immer/detail/hamts/champ.hpp @@ -149,8 +149,7 @@ struct champ champ(node_t* r, size_t sz = 0) : root{r} , size{sz} - { - } + {} champ(const champ& other) : champ{other.root, other.size} @@ -1619,15 +1618,13 @@ struct champ , data{a.data} , owned{false} , mutated{false} - { - } + {} sub_result_mut(sub_result a, bool m) : kind{a.kind} , data{a.data} , owned{false} , mutated{m} - { - } + {} sub_result_mut() : kind{kind_t::nothing} , mutated{false} {};