From d0d85742fc16aedb8e2145ca76fa77bd47ec2e54 Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Sat, 17 Apr 2021 18:39:35 +0200 Subject: [PATCH] BLI: support multiple arguments for value in *_as methods of Map This allows us to build more complex values in-place in the map. Before those values had to be build separately and then moved into the map. Existing calls to the Map API remain unchanged. --- source/blender/blenlib/BLI_map.hh | 80 ++++++++++---------- source/blender/blenlib/BLI_map_slots.hh | 12 +-- source/blender/blenlib/tests/BLI_map_test.cc | 9 +++ 3 files changed, 56 insertions(+), 45 deletions(-) diff --git a/source/blender/blenlib/BLI_map.hh b/source/blender/blenlib/BLI_map.hh index 9fa69853e44..376bf5df06f 100644 --- a/source/blender/blenlib/BLI_map.hh +++ b/source/blender/blenlib/BLI_map.hh @@ -247,11 +247,11 @@ class Map { { this->add_new_as(std::move(key), std::move(value)); } - template - void add_new_as(ForwardKey &&key, ForwardValue &&value) + template + void add_new_as(ForwardKey &&key, ForwardValue &&... value) { this->add_new__impl( - std::forward(key), std::forward(value), hash_(key)); + std::forward(key), hash_(key), std::forward(value)...); } /** @@ -277,11 +277,11 @@ class Map { { return this->add_as(std::move(key), std::move(value)); } - template - bool add_as(ForwardKey &&key, ForwardValue &&value) + template + bool add_as(ForwardKey &&key, ForwardValue &&... value) { return this->add__impl( - std::forward(key), std::forward(value), hash_(key)); + std::forward(key), hash_(key), std::forward(value)...); } /** @@ -307,11 +307,11 @@ class Map { { return this->add_overwrite_as(std::move(key), std::move(value)); } - template - bool add_overwrite_as(ForwardKey &&key, ForwardValue &&value) + template + bool add_overwrite_as(ForwardKey &&key, ForwardValue &&... value) { return this->add_overwrite__impl( - std::forward(key), std::forward(value), hash_(key)); + std::forward(key), hash_(key), std::forward(value)...); } /** @@ -413,12 +413,12 @@ class Map { { return this->pop_default_as(key, std::move(default_value)); } - template - Value pop_default_as(const ForwardKey &key, ForwardValue &&default_value) + template + Value pop_default_as(const ForwardKey &key, ForwardValue &&... default_value) { Slot *slot = this->lookup_slot_ptr(key, hash_(key)); if (slot == nullptr) { - return std::forward(default_value); + return Value(std::forward(default_value)...); } Value value = std::move(*slot->value()); slot->remove(); @@ -525,15 +525,15 @@ class Map { { return this->lookup_default_as(key, default_value); } - template - Value lookup_default_as(const ForwardKey &key, ForwardValue &&default_value) const + template + Value lookup_default_as(const ForwardKey &key, ForwardValue &&... default_value) const { const Value *ptr = this->lookup_ptr_as(key); if (ptr != nullptr) { return *ptr; } else { - return std::forward(default_value); + return Value(std::forward(default_value)...); } } @@ -557,11 +557,11 @@ class Map { { return this->lookup_or_add_as(std::move(key), std::move(value)); } - template - Value &lookup_or_add_as(ForwardKey &&key, ForwardValue &&value) + template + Value &lookup_or_add_as(ForwardKey &&key, ForwardValue &&... value) { return this->lookup_or_add__impl( - std::forward(key), std::forward(value), hash_(key)); + std::forward(key), hash_(key), std::forward(value)...); } /** @@ -982,7 +982,7 @@ class Map { SLOT_PROBING_BEGIN (ProbingStrategy, hash, new_slot_mask, slot_index) { Slot &slot = new_slots[slot_index]; if (slot.is_empty()) { - slot.occupy(std::move(*old_slot.key()), std::move(*old_slot.value()), hash); + slot.occupy(std::move(*old_slot.key()), hash, std::move(*old_slot.value())); return; } } @@ -996,8 +996,8 @@ class Map { new (this) Map(NoExceptConstructor(), allocator); } - template - void add_new__impl(ForwardKey &&key, ForwardValue &&value, uint64_t hash) + template + void add_new__impl(ForwardKey &&key, uint64_t hash, ForwardValue &&... value) { BLI_assert(!this->contains_as(key)); @@ -1005,7 +1005,7 @@ class Map { MAP_SLOT_PROBING_BEGIN (hash, slot) { if (slot.is_empty()) { - slot.occupy(std::forward(key), std::forward(value), hash); + slot.occupy(std::forward(key), hash, std::forward(value)...); occupied_and_removed_slots_++; return; } @@ -1013,14 +1013,14 @@ class Map { MAP_SLOT_PROBING_END(); } - template - bool add__impl(ForwardKey &&key, ForwardValue &&value, uint64_t hash) + template + bool add__impl(ForwardKey &&key, uint64_t hash, ForwardValue &&... value) { this->ensure_can_add(); MAP_SLOT_PROBING_BEGIN (hash, slot) { if (slot.is_empty()) { - slot.occupy(std::forward(key), std::forward(value), hash); + slot.occupy(std::forward(key), hash, std::forward(value)...); occupied_and_removed_slots_++; return true; } @@ -1075,7 +1075,7 @@ class Map { MAP_SLOT_PROBING_BEGIN (hash, slot) { if (slot.is_empty()) { - slot.occupy(std::forward(key), create_value(), hash); + slot.occupy(std::forward(key), hash, create_value()); occupied_and_removed_slots_++; return *slot.value(); } @@ -1086,14 +1086,14 @@ class Map { MAP_SLOT_PROBING_END(); } - template - Value &lookup_or_add__impl(ForwardKey &&key, ForwardValue &&value, uint64_t hash) + template + Value &lookup_or_add__impl(ForwardKey &&key, uint64_t hash, ForwardValue &&... value) { this->ensure_can_add(); MAP_SLOT_PROBING_BEGIN (hash, slot) { if (slot.is_empty()) { - slot.occupy(std::forward(key), std::forward(value), hash); + slot.occupy(std::forward(key), hash, std::forward(value)...); occupied_and_removed_slots_++; return *slot.value(); } @@ -1104,15 +1104,15 @@ class Map { MAP_SLOT_PROBING_END(); } - template - bool add_overwrite__impl(ForwardKey &&key, ForwardValue &&value, uint64_t hash) + template + bool add_overwrite__impl(ForwardKey &&key, uint64_t hash, ForwardValue &&... value) { auto create_func = [&](Value *ptr) { - new (static_cast(ptr)) Value(std::forward(value)); + new (static_cast(ptr)) Value(std::forward(value)...); return true; }; auto modify_func = [&](Value *ptr) { - *ptr = std::forward(value); + *ptr = Value(std::forward(value)...); return false; }; return this->add_or_modify__impl( @@ -1221,16 +1221,18 @@ template class StdUnorderedMapWrapper { map_.reserve(n); } - template - void add_new(ForwardKey &&key, ForwardValue &&value) + template + void add_new(ForwardKey &&key, ForwardValue &&... value) { - map_.insert({std::forward(key), std::forward(value)}); + map_.insert({std::forward(key), Value(std::forward(value)...)}); } - template - bool add(ForwardKey &&key, ForwardValue &&value) + template + bool add(ForwardKey &&key, ForwardValue &&... value) { - return map_.insert({std::forward(key), std::forward(value)}).second; + return map_ + .insert({std::forward(key), Value(std::forward(value)...)}) + .second; } bool contains(const Key &key) const diff --git a/source/blender/blenlib/BLI_map_slots.hh b/source/blender/blenlib/BLI_map_slots.hh index c0cb3091a8e..1b4ca11af41 100644 --- a/source/blender/blenlib/BLI_map_slots.hh +++ b/source/blender/blenlib/BLI_map_slots.hh @@ -195,11 +195,11 @@ template class SimpleMapSlot { * Change the state of this slot from empty/removed to occupied. The key/value has to be * constructed by calling the constructor with the given key/value as parameter. */ - template - void occupy(ForwardKey &&key, ForwardValue &&value, uint64_t hash) + template + void occupy(ForwardKey &&key, uint64_t hash, ForwardValue &&... value) { BLI_assert(!this->is_occupied()); - new (&value_buffer_) Value(std::forward(value)); + new (&value_buffer_) Value(std::forward(value)...); this->occupy_no_value(std::forward(key), hash); state_ = Occupied; } @@ -315,12 +315,12 @@ template class IntrusiveMapSlot return is_equal(key, key_); } - template - void occupy(ForwardKey &&key, ForwardValue &&value, uint64_t hash) + template + void occupy(ForwardKey &&key, uint64_t hash, ForwardValue &&... value) { BLI_assert(!this->is_occupied()); BLI_assert(KeyInfo::is_not_empty_or_removed(key)); - new (&value_buffer_) Value(std::forward(value)); + new (&value_buffer_) Value(std::forward(value)...); this->occupy_no_value(std::forward(key), hash); } diff --git a/source/blender/blenlib/tests/BLI_map_test.cc b/source/blender/blenlib/tests/BLI_map_test.cc index f1ae8fb3921..bb15f7f0d8d 100644 --- a/source/blender/blenlib/tests/BLI_map_test.cc +++ b/source/blender/blenlib/tests/BLI_map_test.cc @@ -604,6 +604,15 @@ TEST(map, GenericAlgorithms) EXPECT_EQ(std::count(map.keys().begin(), map.keys().end(), 7), 1); } +TEST(map, AddAsVariadic) +{ + Map map; + map.add_as(3, "hello", 2); + map.add_as(2, "test", 1); + EXPECT_EQ(map.lookup(3), "he"); + EXPECT_EQ(map.lookup(2), "t"); +} + /** * Set this to 1 to activate the benchmark. It is disabled by default, because it prints a lot. */