From e9dbd10db4e93c86ccfbc180cb0242bf21cdb8e4 Mon Sep 17 00:00:00 2001
From: Michele Caini <michele.caini@gmail.com>
Date: Tue, 2 Aug 2022 09:56:24 +0200
Subject: [PATCH] storage: fix cross range-erase can break when using built-in
 iterators (close #914)

---
 src/entt/entity/storage.hpp  | 13 +++++---
 test/entt/entity/storage.cpp | 60 ++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/src/entt/entity/storage.hpp b/src/entt/entity/storage.hpp
index 47d1fc02a..3ac1868a0 100644
--- a/src/entt/entity/storage.hpp
+++ b/src/entt/entity/storage.hpp
@@ -331,14 +331,17 @@ protected:
      */
     void pop(basic_iterator first, basic_iterator last) override {
         for(; first != last; ++first) {
+            // cannot use first.index() because it would break with cross iterators
+            auto &elem = element_at(base_type::index(*first));
+
             if constexpr(comp_traits::in_place_delete) {
                 base_type::in_place_pop(first);
-                std::destroy_at(std::addressof(element_at(static_cast<size_type>(first.index()))));
-            } else {
-                auto &elem = element_at(base_type::size() - 1u);
-                // destroying on exit allows reentrant destructors
-                [[maybe_unused]] auto unused = std::exchange(element_at(static_cast<size_type>(first.index())), std::move(elem));
                 std::destroy_at(std::addressof(elem));
+            } 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));
+                std::destroy_at(std::addressof(other));
                 base_type::swap_and_pop(first);
             }
         }
diff --git a/test/entt/entity/storage.cpp b/test/entt/entity/storage.cpp
index 598dd76cd..c74a185e0 100644
--- a/test/entt/entity/storage.cpp
+++ b/test/entt/entity/storage.cpp
@@ -364,6 +364,21 @@ TEST(Storage, Erase) {
     ASSERT_EQ(*pool.begin(), 1);
 }
 
+TEST(Storage, CrossErase) {
+    entt::sparse_set set;
+    entt::storage<int> pool;
+    entt::entity entities[2u]{entt::entity{3}, entt::entity{42}};
+
+    pool.emplace(entities[0u], 3);
+    pool.emplace(entities[1u], 42);
+    set.emplace(entities[1u]);
+    pool.erase(set.begin(), set.end());
+
+    ASSERT_TRUE(pool.contains(entities[0u]));
+    ASSERT_FALSE(pool.contains(entities[1u]));
+    ASSERT_EQ(pool.raw()[0u][0u], 3);
+}
+
 TEST(Storage, StableErase) {
     entt::storage<stable_type> pool;
     entt::entity entities[3u]{entt::entity{3}, entt::entity{42}, entt::entity{9}};
@@ -458,6 +473,21 @@ TEST(Storage, StableErase) {
     ASSERT_EQ(pool.get(entities[2u]).value, 1);
 }
 
+TEST(Storage, CrossStableErase) {
+    entt::sparse_set set;
+    entt::storage<stable_type> pool;
+    entt::entity entities[2u]{entt::entity{3}, entt::entity{42}};
+
+    pool.emplace(entities[0u], 3);
+    pool.emplace(entities[1u], 42);
+    set.emplace(entities[1u]);
+    pool.erase(set.begin(), set.end());
+
+    ASSERT_TRUE(pool.contains(entities[0u]));
+    ASSERT_FALSE(pool.contains(entities[1u]));
+    ASSERT_EQ(pool.raw()[0u][0u].value, 3);
+}
+
 TEST(Storage, Remove) {
     entt::storage<int> pool;
     entt::entity entities[3u]{entt::entity{3}, entt::entity{42}, entt::entity{9}};
@@ -492,6 +522,21 @@ TEST(Storage, Remove) {
     ASSERT_EQ(*pool.begin(), 1);
 }
 
+TEST(Storage, CrossRemove) {
+    entt::sparse_set set;
+    entt::storage<int> pool;
+    entt::entity entities[2u]{entt::entity{3}, entt::entity{42}};
+
+    pool.emplace(entities[0u], 3);
+    pool.emplace(entities[1u], 42);
+    set.emplace(entities[1u]);
+    pool.remove(set.begin(), set.end());
+
+    ASSERT_TRUE(pool.contains(entities[0u]));
+    ASSERT_FALSE(pool.contains(entities[1u]));
+    ASSERT_EQ(pool.raw()[0u][0u], 3);
+}
+
 TEST(Storage, StableRemove) {
     entt::storage<stable_type> pool;
     entt::entity entities[3u]{entt::entity{3}, entt::entity{42}, entt::entity{9}};
@@ -589,6 +634,21 @@ TEST(Storage, StableRemove) {
     ASSERT_EQ(pool.get(entities[2u]).value, 1);
 }
 
+TEST(Storage, CrossStableRemove) {
+    entt::sparse_set set;
+    entt::storage<stable_type> pool;
+    entt::entity entities[2u]{entt::entity{3}, entt::entity{42}};
+
+    pool.emplace(entities[0u], 3);
+    pool.emplace(entities[1u], 42);
+    set.emplace(entities[1u]);
+    pool.remove(set.begin(), set.end());
+
+    ASSERT_TRUE(pool.contains(entities[0u]));
+    ASSERT_FALSE(pool.contains(entities[1u]));
+    ASSERT_EQ(pool.raw()[0u][0u].value, 3);
+}
+
 TEST(Storage, TypeFromBase) {
     entt::storage<int> pool;
     entt::sparse_set &base = pool;