sparse_set: revert optmized range push, it prevents self-managed storage classes

src/entt/entity/sparse_set.hpp

@@ -274,27 +274,6 @@ protected:
         packed[static_cast<size_type>(entt)] = std::exchange(free_list, traits_type::combine(entt, tombstone));
     }
 
-    /**
-     * @brief Assigns an entity to a sparse set.
-     * @param entt A valid identifier.
-     * @param force_back Force back insertion.
-     * @return Iterator pointing to the emplaced element.
-     */
-    basic_iterator try_emplace(const Entity entt, const bool force_back) {
-        ENTT_ASSERT(!contains(entt), "Set already contains entity");
-
-        if(auto &elem = assure_at_least(entt); free_list == null || force_back) {
-            packed.push_back(entt);
-            elem = traits_type::combine(static_cast<typename traits_type::entity_type>(packed.size() - 1u), traits_type::to_integral(entt));
-            return begin();
-        } else {
-            const auto pos = static_cast<size_type>(traits_type::to_entity(free_list));
-            elem = traits_type::combine(traits_type::to_integral(free_list), traits_type::to_integral(entt));
-            free_list = std::exchange(packed[pos], entt);
-            return --(end() - pos);
-        }
-    }
-
 protected:
     /**
      * @brief Erases entities from a sparse set.
@@ -314,14 +293,24 @@ protected:
     }
 
     /**
-     * @brief Assigns one or more entities to a sparse set.
-     * @param first An iterator to the first element of the range of entities.
-     * @param last An iterator past the last element of the range of entities.
-     * @param value Optional opaque value.
-     * @return Iterator pointing to the emplaced elements.
+     * @brief Assigns an entity to a sparse set.
+     * @param entt A valid identifier.
+     * @param force_back Force back insertion.
+     * @return Iterator pointing to the emplaced element.
      */
-    virtual basic_iterator try_insert(basic_iterator first, [[maybe_unused]] basic_iterator last, [[maybe_unused]] const void *value) {
-        return first;
+    virtual basic_iterator try_emplace(const Entity entt, const bool force_back, const void * = nullptr) {
+        ENTT_ASSERT(!contains(entt), "Set already contains entity");
+
+        if(auto &elem = assure_at_least(entt); free_list == null || force_back) {
+            packed.push_back(entt);
+            elem = traits_type::combine(static_cast<typename traits_type::entity_type>(packed.size() - 1u), traits_type::to_integral(entt));
+            return begin();
+        } else {
+            const auto pos = static_cast<size_type>(traits_type::to_entity(free_list));
+            elem = traits_type::combine(traits_type::to_integral(free_list), traits_type::to_integral(entt));
+            free_list = std::exchange(packed[pos], entt);
+            return --(end() - pos);
+        }
     }
 
 public:
@@ -701,8 +690,7 @@ public:
      * `end()` iterator otherwise.
      */
     iterator push(const entity_type entt, const void *value = nullptr) {
-        const auto it = try_emplace(entt, false);
-        return try_insert(it, it + 1u, value);
+        return try_emplace(entt, false, value);
     }
 
     /**
@@ -720,27 +708,11 @@ public:
      */
     template<typename It>
     iterator push(It first, It last) {
-        if(first == last) {
-            return end();
-        }
-
-        const auto to = begin();
-        auto from = to;
-
-        ENTT_TRY {
-            for(; first != last; ++first) {
-                from = try_emplace(*first, true);
-            }
-        }
-        ENTT_CATCH {
-            for(; from != to; ++from) {
-                swap_and_pop(from);
-            }
-
-            ENTT_THROW;
+        for(auto it = first; it != last; ++it) {
+            try_emplace(*it, true);
         }
 
-        return try_insert(from, to, nullptr);
+        return first == last ? end() : find(*first);
     }
 
     /**

src/entt/entity/storage.hpp

@@ -264,7 +264,9 @@ class basic_storage: public basic_sparse_set<Entity, typename std::allocator_tra
     }
 
     template<typename... Args>
-    void emplace_element(const underlying_iterator it, Args &&...args) {
+    auto emplace_element(const Entity entt, const bool force_back, Args &&...args) {
+        const auto it = base_type::try_emplace(entt, force_back);
+
         ENTT_TRY {
             auto elem = assure_at_least(static_cast<size_type>(it.index()));
             entt::uninitialized_construct_using_allocator(to_address(elem), get_allocator(), std::forward<Args>(args)...);
@@ -273,6 +275,8 @@ class basic_storage: public basic_sparse_set<Entity, typename std::allocator_tra
             base_type::pop(it, it + 1u);
             ENTT_THROW;
         }
+
+        return it;
     }
 
     void shrink_to_size(const std::size_t sz) {
@@ -346,39 +350,23 @@ protected:
     }
 
     /**
-     * @brief Assigns one or more entities to a sparse set.
-     * @param first An iterator to the first element of the range of entities.
-     * @param last An iterator past the last element of the range of entities.
+     * @brief Assigns an entity to a storage.
+     * @param entt A valid identifier.
      * @param value Optional opaque value.
-     * @return Iterator pointing to the emplaced elements.
+     * @param force_back Force back insertion.
+     * @return Iterator pointing to the emplaced element.
      */
-    underlying_iterator try_insert(underlying_iterator first, underlying_iterator last, const void *value = nullptr) override {
+    underlying_iterator try_emplace([[maybe_unused]] const Entity entt, [[maybe_unused]] const bool force_back, const void *value) override {
         if(value) {
             if constexpr(std::is_copy_constructible_v<value_type>) {
-                ENTT_ASSERT(std::next(first) == last, "Opaque emplace does not support ranges");
-                emplace_element(first, *static_cast<const value_type *>(value));
-                return first;
+                return emplace_element(entt, force_back, *static_cast<const value_type *>(value));
             } else {
-                base_type::pop(first, last);
                 return base_type::end();
             }
         } else {
             if constexpr(std::is_default_constructible_v<value_type>) {
-                const auto placeholder = first;
-
-                ENTT_TRY {
-                    for(; first != last; ++first) {
-                        emplace_element(first);
-                    }
-
-                    return placeholder;
-                }
-                ENTT_CATCH {
-                    base_type::pop(++first, last);
-                    ENTT_THROW;
-                }
+                return emplace_element(entt, force_back);
             } else {
-                base_type::pop(first, last);
                 return base_type::end();
             }
         }
@@ -667,15 +655,13 @@ public:
      */
     template<typename... Args>
     value_type &emplace(const entity_type entt, Args &&...args) {
-        const auto it = base_type::try_emplace(entt, false);
-
         if constexpr(std::is_aggregate_v<value_type>) {
-            emplace_element(it, Type{std::forward<Args>(args)...});
+            const auto it = emplace_element(entt, false, Type{std::forward<Args>(args)...});
+            return element_at(static_cast<size_type>(it.index()));
         } else {
-            emplace_element(it, std::forward<Args>(args)...);
+            const auto it = emplace_element(entt, false, std::forward<Args>(args)...);
+            return element_at(static_cast<size_type>(it.index()));
         }
-
-        return element_at(static_cast<size_type>(it.index()));
     }
 
     /**
@@ -709,8 +695,7 @@ public:
     template<typename It>
     void insert(It first, It last, const value_type &value = {}) {
         for(; first != last; ++first) {
-            const auto it = base_type::try_emplace(*first, true);
-            emplace_element(it, value);
+            emplace_element(*first, true, value);
         }
     }
 
@@ -729,8 +714,7 @@ public:
     template<typename EIt, typename CIt, typename = std::enable_if_t<std::is_same_v<typename std::iterator_traits<CIt>::value_type, value_type>>>
     void insert(EIt first, EIt last, CIt from) {
         for(; first != last; ++first, ++from) {
-            const auto it = base_type::try_emplace(*first, true);
-            emplace_element(it, *from);
+            emplace_element(*first, true, *from);
         }
     }
 

src/entt/entity/storage_mixin.hpp

@@ -39,15 +39,10 @@ class sigh_mixin final: public Type {
         }
     }
 
-    underlying_iterator try_insert(underlying_iterator first, underlying_iterator last, const void *value = nullptr) override {
+    underlying_iterator try_emplace(const typename Type::entity_type entt, const bool force_back, const void *value) final {
         ENTT_ASSERT(owner != nullptr, "Invalid pointer to registry");
-        const auto entt = *first;
-
-        for(; first != last; ++first) {
-            Type::try_insert(first, first + 1u, value);
-            construction.publish(*owner, *first);
-        }
-
+        Type::try_emplace(entt, force_back, value);
+        construction.publish(*owner, entt);
         return Type::find(entt);
     }
 

test/entt/entity/sparse_set.cpp

@@ -1417,10 +1417,10 @@ TEST(SparseSet, ThrowingAllocator) {
     ASSERT_THROW(set.push(std::begin(entities), std::end(entities)), test::throwing_allocator<entt::entity>::exception_type);
     ASSERT_EQ(set.extent(), 2 * traits_type::page_size);
     ASSERT_TRUE(set.contains(entt::entity{0}));
-    ASSERT_FALSE(set.contains(entt::entity{1}));
+    ASSERT_TRUE(set.contains(entt::entity{1}));
     ASSERT_FALSE(set.contains(entt::entity{traits_type::page_size}));
     ASSERT_EQ(set.capacity(), 2u);
-    ASSERT_EQ(set.size(), 1u);
+    ASSERT_EQ(set.size(), 2u);
 
     set.push(entities[1u]);