Skip to content

Commit

Permalink
Perfectly forward the key into update and try_update + add more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@fabianbs96
fabianbs96 committed Sep 23, 2023
1 parent 6f154ec commit 7eec5f9
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 24 deletions.
41 changes: 25 additions & 16 deletions immer/detail/hamts/champ.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Project, Default, Combine>(
root, k, std::forward<Fn>(fn), hash, 0);
root, std::forward<K>(k), std::forward<Fn>(fn), hash, 0);
auto new_size = size + (res.added ? 1 : 0);
return {res.node, new_size};
}
Expand All @@ -857,8 +857,8 @@ struct champ
{
static update_result
do_try_update(node_t* node,
byval_if_possible<K> k,
byval_if_possible<std::decay_t<Fn>, Fn&&> fn,
byval_if_possible<K, K&&> k,
byval_if_possible<Fn, Fn&&> fn,
byval_if_possible<ValueEquals> valueEquals,
hash_t hash,
shift_t shift)
Expand Down Expand Up @@ -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>(k),
std::forward<Fn>(fn),
valueEquals,
hash,
Expand Down Expand Up @@ -959,7 +959,7 @@ struct champ
static update_mut_result
do_try_update_mut(edit_t e,
node_t* node,
byval_if_possible<K> k,
byval_if_possible<K, K&&> k,
byval_if_possible<Fn, Fn&&> fn,
byval_if_possible<ValueEquals> valueEquals,
hash_t hash,
Expand Down Expand Up @@ -1008,7 +1008,7 @@ struct champ
if (node->can_mutate(e)) {
auto result = do_try_update_mut(e,
child,
k,
std::forward<K>(k),
std::forward<Fn>(fn),
valueEquals,
hash,
Expand All @@ -1022,7 +1022,7 @@ struct champ
return {node, result.added, true};
} else {
auto result = do_try_update(child,
k,
std::forward<K>(k),
std::forward<Fn>(fn),
valueEquals,
hash,
Expand Down Expand Up @@ -1119,12 +1119,16 @@ struct champ
typename K,
typename Fn,
typename ValueEquals = std::equal_to<T>>
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<Project, Default, Combine, K, Fn, ValueEquals>::
do_try_update(
root, k, std::forward<Fn>(fn), std::move(valueEquals), hash, 0);
do_try_update(root,
std::forward<K>(k),
std::forward<Fn>(fn),
std::move(valueEquals),
hash,
0);
if (!res.node)
return {root->inc(), size};

Expand Down Expand Up @@ -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<Project, Default, Combine>(
e, root, k, std::forward<Fn>(fn), hash, 0);
e, root, std::forward<K>(k), std::forward<Fn>(fn), hash, 0);
if (!res.mutated && root->dec())
node_t::delete_deep(root, 0);
root = res.node;
Expand All @@ -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<Project, Default, Combine, K, Fn, ValueEquals>::
do_try_update_mut(
e, root, k, std::forward<Fn>(fn), valueEquals, hash, 0);
do_try_update_mut(e,
root,
std::forward<K>(k),
std::forward<Fn>(fn),
valueEquals,
hash,
0);
if (!res.node)
return;

Expand Down
4 changes: 3 additions & 1 deletion immer/detail/util.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,9 @@ static constexpr bool can_efficiently_pass_by_value =

template <typename T, typename OrElse = const T&>
using byval_if_possible =
std::conditional_t<can_efficiently_pass_by_value<T>, T, OrElse>;
std::conditional_t<can_efficiently_pass_by_value<std::decay_t<T>>,
std::decay_t<T>,
OrElse>;

} // namespace detail
} // namespace immer
102 changes: 95 additions & 7 deletions test/map/generic.ipp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string, immer::box<std::string, memory_policy_t>>{};
SECTION("preserve immutability")
{

auto do_test = [&s](auto Updater) {
auto s0 = s;
auto i0 = 0u;
// insert
Expand All @@ -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)) {
Expand All @@ -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);
});
{
Expand All @@ -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<decltype(map)>(map).update(
std::forward<decltype(key)>(key),
std::forward<decltype(cb)>(cb));
});
}

SECTION("preserve immutability (try_update)")
{
do_test([](auto&& map, auto&& key, auto&& cb) {
return std::forward<decltype(map)>(map).try_update(
std::forward<decltype(key)>(key),
std::forward<decltype(cb)>(cb));
});
}
}
#endif
Expand Down Expand Up @@ -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{};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 7eec5f9

Please sign in to comment.