sparse_set: review internal functions (perf)

TODO

@@ -36,6 +36,5 @@ TODO:
 * redesign snapshot as a whole
 * use the value type mixin more in the test suite to reduce the number of utility types
 * explore "runtime" mode for hashed string where the source is copied internally
-* swap_only/swap_and_pop/in_place_pop can (should?) accept the entity
 * storage: shrink_to_fit does not work with reentrant destructor?
 * test trivially_destructible optimization

src/entt/entity/sparse_set.hpp

@@ -230,25 +230,25 @@ protected:
 
     /**
      * @brief Erases an entity from a sparse set.
-     * @param it An iterator to the element to pop.
+     * @param entt A valid identifier for the element to pop.
      */
-    void swap_only(const basic_iterator it) {
+    void swap_only(const Entity entt) {
         ENTT_ASSERT(mode == deletion_policy::swap_only, "Deletion policy mismatch");
-        const auto pos = index(*it);
-        bump(traits_type::next(*it));
+        const auto pos = index(entt);
+        bump(traits_type::next(entt));
         swap_at(pos, head -= (pos < head));
     }
 
     /**
      * @brief Erases an entity from a sparse set.
-     * @param it An iterator to the element to pop.
+     * @param entt A valid identifier for the element to pop.
      */
-    void swap_and_pop(const basic_iterator it) {
+    void swap_and_pop(const Entity entt) {
         ENTT_ASSERT(mode == deletion_policy::swap_and_pop, "Deletion policy mismatch");
-        auto &self = sparse_ref(*it);
-        const auto entt = traits_type::to_entity(self);
-        sparse_ref(packed.back()) = traits_type::combine(entt, traits_type::to_integral(packed.back()));
-        packed[static_cast<size_type>(entt)] = packed.back();
+        auto &self = sparse_ref(entt);
+        const auto pos = traits_type::to_entity(self);
+        sparse_ref(packed.back()) = traits_type::combine(pos, traits_type::to_integral(packed.back()));
+        packed[static_cast<size_type>(pos)] = packed.back();
         // unnecessary but it helps to detect nasty bugs
         // NOLINTNEXTLINE(bugprone-assert-side-effect)
         ENTT_ASSERT((packed.back() = null, true), "");
@@ -259,11 +259,11 @@ protected:
 
     /**
      * @brief Erases an entity from a sparse set.
-     * @param it An iterator to the element to pop.
+     * @param entt A valid identifier for the element to pop.
      */
-    void in_place_pop(const basic_iterator it) {
+    void in_place_pop(const Entity entt) {
         ENTT_ASSERT(mode == deletion_policy::in_place, "Deletion policy mismatch");
-        const auto pos = entity_to_pos(std::exchange(sparse_ref(*it), null));
+        const auto pos = entity_to_pos(std::exchange(sparse_ref(entt), null));
         packed[pos] = traits_type::combine(static_cast<traits_type::entity_type>(std::exchange(head, pos)), tombstone);
     }
 
@@ -276,17 +276,17 @@ protected:
         switch(mode) {
         case deletion_policy::swap_and_pop:
             for(; first != last; ++first) {
-                swap_and_pop(first);
+                swap_and_pop(*first);
             }
             break;
         case deletion_policy::in_place:
             for(; first != last; ++first) {
-                in_place_pop(first);
+                in_place_pop(*first);
             }
             break;
         case deletion_policy::swap_only:
             for(; first != last; ++first) {
-                swap_only(first);
+                swap_only(*first);
             }
             break;
         }

src/entt/entity/storage.hpp

@@ -324,17 +324,17 @@ protected:
             auto &elem = element_at(base_type::index(*first));
 
             if constexpr(traits_type::in_place_delete) {
-                base_type::in_place_pop(first);
+                base_type::in_place_pop(*first);
                 alloc_traits::destroy(allocator, std::addressof(elem));
             } else if constexpr(std::is_trivially_destructible_v<element_type>) {
                 elem = std::move(element_at(base_type::size() - 1u));
-                base_type::swap_and_pop(first);
+                base_type::swap_and_pop(*first);
             } else {
                 auto &other = element_at(base_type::size() - 1u);
                 // destroying on exit allows reentrant destructors
                 [[maybe_unused]] auto unused = std::exchange(elem, std::move(other));
                 alloc_traits::destroy(allocator, std::addressof(other));
-                base_type::swap_and_pop(first);
+                base_type::swap_and_pop(*first);
             }
         }
     }
@@ -349,11 +349,11 @@ protected:
             for(auto first = base_type::begin(); !(first.index() < 0); ++first) {
                 if constexpr(traits_type::in_place_delete) {
                     if(*first != tombstone) {
-                        base_type::in_place_pop(first);
+                        base_type::in_place_pop(*first);
                         alloc_traits::destroy(allocator, std::addressof(element_at(static_cast<size_type>(first.index()))));
                     }
                 } else {
-                    base_type::swap_and_pop(first);
+                    base_type::swap_and_pop(*first);
                     alloc_traits::destroy(allocator, std::addressof(element_at(static_cast<size_type>(first.index()))));
                 }
             }

test/entt/entity/component.cpp

@@ -4,7 +4,7 @@
 #include <entt/entity/component.hpp>
 #include "../../common/boxed_type.h"
 #include "../../common/empty.h"
-#include "../../common/non_movable.h"
+#include "../../common/value_type.h"
 
 struct ComponentBase: testing::Test {
     enum class my_entity : std::uint32_t {};