dense set:

* cleaned up implementation
* uses-allocator construction support
* a few more tests

TODO

@@ -4,7 +4,7 @@
 * add examples (and credits) from @alanjfs :)
 
 WIP:
-* uses-allocator construction: dense set (review), dense map, compressed pair, any (with allocator support), cache, dispatcher, poly, ...
+* uses-allocator construction: dense map, compressed pair, any (with allocator support), cache, dispatcher, poly, ...
 * process scheduler: reviews, use free lists internally
 * runtime events (emitter)
 * iterator based try_emplace vs try_insert for perf reasons

src/entt/container/dense_set.hpp

@@ -27,63 +27,11 @@ namespace entt {
 
 namespace internal {
 
-template<typename Type, typename Allocator>
-struct dense_set_node final: Allocator {
-    using allocator_type = Allocator;
-
-    template<typename... Args>
-    dense_set_node(const std::size_t pos, Args &&...args)
-        : dense_set_node{std::allocator_arg, Allocator{}, pos, std::forward<Args>(args)...} {
-    }
-
-    template<typename... Args>
-    dense_set_node(std::allocator_arg_t, const allocator_type &alloc, const std::size_t pos, Args &&...args)
-        : Allocator{alloc},
-          next{pos} {
-        std::allocator_traits<allocator_type>::construct(*this, reinterpret_cast<Type *>(&instance), std::forward<Args>(args)...);
-    }
-
-    dense_set_node(const dense_set_node &other)
-        : dense_set_node{std::allocator_arg, Allocator{}, other} {
-    }
-
-    dense_set_node(std::allocator_arg_t, const allocator_type &alloc, const dense_set_node &other)
-        : Allocator{alloc},
-          next{other.next} {
-        std::allocator_traits<allocator_type>::construct(*this, reinterpret_cast<Type *>(&instance), other.element());
-    }
-
-    dense_set_node(dense_set_node &&other)
-        : dense_set_node{std::allocator_arg, Allocator{}, std::move(other)} {
-    }
-
-    dense_set_node(std::allocator_arg_t, const allocator_type &alloc, dense_set_node &&other)
-        : Allocator{alloc},
-          next{other.next} {
-        std::allocator_traits<allocator_type>::construct(*this, reinterpret_cast<Type *>(&instance), std::move(other.element()));
-    }
-
-    ~dense_set_node() {
-        std::destroy_at(reinterpret_cast<const Type *>(&instance));
-    }
-
-    const Type &element() const ENTT_NOEXCEPT {
-        return *reinterpret_cast<const Type *>(&instance);
-    }
-
-    Type &element() ENTT_NOEXCEPT {
-        return *reinterpret_cast<Type *>(&instance);
-    }
-
-    std::size_t next;
-    std::aligned_storage_t<sizeof(Type)> instance;
-};
-
 template<typename It>
 class dense_set_iterator final {
     friend dense_set_iterator<const std::remove_pointer_t<It> *>;
 
-    using iterator_traits = std::iterator_traits<decltype(std::addressof(std::as_const(std::declval<It>()->element())))>;
+    using iterator_traits = std::iterator_traits<decltype(std::addressof(std::as_const(std::declval<It>()->second)))>;
 
 public:
     using value_type = typename iterator_traits::value_type;
@@ -138,11 +86,11 @@ public:
     }
 
     [[nodiscard]] reference operator[](const difference_type value) const ENTT_NOEXCEPT {
-        return it[value].element();
+        return it[value].second;
     }
 
     [[nodiscard]] pointer operator->() const ENTT_NOEXCEPT {
-        return std::addressof(it->element());
+        return std::addressof(it->second);
     }
 
     [[nodiscard]] reference operator*() const ENTT_NOEXCEPT {
@@ -201,7 +149,7 @@ template<typename It>
 class dense_set_local_iterator final {
     friend dense_set_local_iterator<const std::remove_pointer_t<It> *>;
 
-    using iterator_traits = std::iterator_traits<decltype(std::addressof(std::as_const(std::declval<It>()->element())))>;
+    using iterator_traits = std::iterator_traits<decltype(std::addressof(std::as_const(std::declval<It>()->second)))>;
 
 public:
     using value_type = typename iterator_traits::value_type;
@@ -222,7 +170,7 @@ public:
           offset{other.offset} {}
 
     dense_set_local_iterator &operator++() ENTT_NOEXCEPT {
-        return offset = it[offset].next, *this;
+        return offset = it[offset].first, *this;
     }
 
     dense_set_local_iterator operator++(int) ENTT_NOEXCEPT {
@@ -231,7 +179,7 @@ public:
     }
 
     [[nodiscard]] pointer operator->() const ENTT_NOEXCEPT {
-        return std::addressof(it[offset].element());
+        return std::addressof(it[offset].second);
     }
 
     [[nodiscard]] reference operator*() const ENTT_NOEXCEPT {
@@ -284,7 +232,7 @@ class dense_set {
     using alloc_traits = std::allocator_traits<Allocator>;
     static_assert(std::is_same_v<typename alloc_traits::value_type, Type>);
 
-    using node_type = internal::dense_set_node<Type, Allocator>;
+    using node_type = std::pair<std::size_t, Type>;
     using sparse_container_type = std::vector<std::size_t, typename alloc_traits::template rebind_alloc<std::size_t>>;
     using packed_container_type = std::vector<node_type, typename alloc_traits::template rebind_alloc<node_type>>;
 
@@ -316,10 +264,10 @@ class dense_set {
 
     template<typename Other>
     bool do_erase(const Other &value) {
-        for(size_type *curr = sparse.first().data() + bucket(value); *curr != std::numeric_limits<size_type>::max(); curr = &packed.first()[*curr].next) {
-            if(packed.second()(packed.first()[*curr].element(), value)) {
+        for(size_type *curr = sparse.first().data() + bucket(value); *curr != std::numeric_limits<size_type>::max(); curr = &packed.first()[*curr].first) {
+            if(packed.second()(packed.first()[*curr].second, value)) {
                 const auto index = *curr;
-                *curr = packed.first()[*curr].next;
+                *curr = packed.first()[*curr].first;
                 move_and_pop(index);
                 return true;
             }
@@ -330,13 +278,13 @@ class dense_set {
 
     void move_and_pop(const std::size_t pos) {
         if(const auto last = size() - 1u; pos != last) {
-            size_type *curr = sparse.first().data() + bucket(packed.first().back().element());
-            for(; *curr != last; curr = &packed.first()[*curr].next) {}
+            size_type *curr = sparse.first().data() + bucket(packed.first().back().second);
+            for(; *curr != last; curr = &packed.first()[*curr].first) {}
             *curr = pos;
 
             // basic exception guarantees when value type has a throwing move constructor
-            packed.first()[pos].element() = std::move(packed.first().back().element());
-            packed.first()[pos].next = packed.first().back().next;
+            packed.first()[pos].second = std::move(packed.first().back().second);
+            packed.first()[pos].first = packed.first().back().first;
         }
 
         packed.first().pop_back();
@@ -591,21 +539,19 @@ public:
      */
     template<typename... Args>
     std::pair<iterator, bool> emplace(Args &&...args) {
-        auto &node = packed.first().emplace_back(packed.first().size(), std::forward<Args>(args)...);
-        const auto hash = sparse.second()(node.element());
-        auto index = hash_to_bucket(hash);
+        auto &node = packed.first().emplace_back(std::piecewise_construct, std::make_tuple(packed.first().size()), std::forward_as_tuple(std::forward<Args>(args)...));
+        const auto hash = sparse.second()(node.second);
+        const auto index = hash_to_bucket(hash);
 
-        if(auto it = constrained_find(node.element(), index); it != end()) {
+        if(auto it = constrained_find(node.second, index); it != end()) {
             packed.first().pop_back();
             return std::make_pair(it, false);
         }
 
-        // update goes after emplace to enforce exception guarantees
-        node.next = std::exchange(sparse.first()[index], size() - 1u);
+        node.first = std::exchange(sparse.first()[index], size() - 1u);
 
-        if(const auto count = size() + 1u; count > (bucket_count() * max_load_factor())) {
+        if(size() > (bucket_count() * max_load_factor())) {
             rehash(bucket_count() * 2u);
-            index = hash_to_bucket(hash);
         }
 
         return std::make_pair(--end(), true);
@@ -843,8 +789,8 @@ public:
             std::fill(sparse.first().begin(), sparse.first().end(), std::numeric_limits<size_type>::max());
 
             for(size_type pos{}, last = size(); pos < last; ++pos) {
-                const auto index = bucket(packed.first()[pos].element());
-                packed.first()[pos].next = std::exchange(sparse.first()[index], pos);
+                const auto index = bucket(packed.first()[pos].second);
+                packed.first()[pos].first = std::exchange(sparse.first()[index], pos);
             }
         }
     }

test/entt/container/dense_set.cpp

@@ -821,7 +821,7 @@ TEST(DenseSet, Reserve) {
 
 TEST(DenseSet, ThrowingAllocator) {
     using allocator = test::throwing_allocator<std::size_t>;
-    using packed_allocator = test::throwing_allocator<entt::internal::dense_set_node<std::size_t, test::throwing_allocator<std::size_t>>>;
+    using packed_allocator = test::throwing_allocator<std::pair<std::size_t, std::size_t>>;
     using packed_exception = typename packed_allocator::exception_type;
 
     static constexpr std::size_t minimum_bucket_count = 8u;
@@ -836,22 +836,33 @@ TEST(DenseSet, ThrowingAllocator) {
 
 #if defined(ENTT_HAS_TRACKED_MEMORY_RESOURCE)
 
+TEST(DenseSet, NoUsesAllocatorConstruction) {
+    using allocator = std::pmr::polymorphic_allocator<int>;
+
+    test::tracked_memory_resource memory_resource{};
+    entt::dense_set<int, std::hash<int>, std::equal_to<int>, allocator> set{&memory_resource};
+
+    set.reserve(1u);
+    memory_resource.reset();
+    set.emplace(0);
+
+    ASSERT_EQ(memory_resource.do_allocate_counter(), 0u);
+    ASSERT_EQ(memory_resource.do_deallocate_counter(), 0u);
+}
+
 TEST(DenseSet, UsesAllocatorConstruction) {
     using string_type = typename test::tracked_memory_resource::string_type;
     using allocator = std::pmr::polymorphic_allocator<string_type>;
 
     test::tracked_memory_resource memory_resource{};
     entt::dense_set<string_type, std::hash<string_type>, std::equal_to<string_type>, allocator> set{&memory_resource};
-    const char *str = "a string long enough to force an allocation (hopefully)";
 
     set.reserve(1u);
     memory_resource.reset();
-    set.emplace(str);
+    set.emplace(test::tracked_memory_resource::default_value);
 
     ASSERT_GT(memory_resource.do_allocate_counter(), 0u);
     ASSERT_EQ(memory_resource.do_deallocate_counter(), 0u);
-
-    entt::dense_set<string_type, std::hash<string_type>, std::equal_to<string_type>, allocator> other{std::move(set)};
 }
 
 #endif