From 3b50672b70b945b1d4c9ee7d1fda0092c23ef0c0 Mon Sep 17 00:00:00 2001
From: Michele Caini <michele.caini@gmail.com>
Date: Wed, 28 Dec 2022 17:32:05 +0100
Subject: [PATCH] storage: restore storage_for/storage_type duality, it turned
 out to be very useful in practice

---
 src/entt/entity/fwd.hpp      | 32 ++++++++++++++++++++++----------
 src/entt/entity/registry.hpp | 32 ++++++++++++++++----------------
 test/entt/entity/storage.cpp | 10 ++++++++--
 3 files changed, 46 insertions(+), 28 deletions(-)

diff --git a/src/entt/entity/fwd.hpp b/src/entt/entity/fwd.hpp
index 7f9607d88..18e71c078 100644
--- a/src/entt/entity/fwd.hpp
+++ b/src/entt/entity/fwd.hpp
@@ -26,19 +26,12 @@ class sigh_mixin;
  * @tparam Entity A valid entity type.
  * @tparam Allocator Type of allocator used to manage memory and elements.
  */
-template<typename Type, typename Entity = entity, typename Allocator = std::allocator<std::remove_const_t<Type>>, typename = void>
+template<typename Type, typename Entity = entity, typename Allocator = std::allocator<Type>, typename = void>
 struct storage_type {
     /*! @brief Type-to-storage conversion result. */
     using type = sigh_mixin<basic_storage<Type, Entity, Allocator>>;
 };
 
-/*! @copydoc storage_type */
-template<typename Type, typename Entity, typename Allocator>
-struct storage_type<const Type, Entity, Allocator> {
-    /*! @brief Type-to-storage conversion result. */
-    using type = const typename storage_type<Type, Entity, Allocator>::type;
-};
-
 /**
  * @brief Helper type.
  * @tparam Args Arguments to forward.
@@ -46,6 +39,25 @@ struct storage_type<const Type, Entity, Allocator> {
 template<typename... Args>
 using storage_type_t = typename storage_type<Args...>::type;
 
+/**
+ * Type-to-storage conversion utility that preserves constness.
+ * @tparam Type Storage value type, eventually const.
+ * @tparam Entity A valid entity type.
+ * @tparam Allocator Type of allocator used to manage memory and elements.
+ */
+template<typename Type, typename Entity = entity, typename Allocator = std::allocator<std::remove_const_t<Type>>>
+struct storage_for {
+    /*! @brief Type-to-storage conversion result. */
+    using type = constness_as_t<storage_type_t<std::remove_const_t<Type>, Entity, Allocator>, Type>;
+};
+
+/**
+ * @brief Helper type.
+ * @tparam Args Arguments to forward.
+ */
+template<typename... Args>
+using storage_for_t = typename storage_for<Args...>::type;
+
 template<typename Entity = entity, typename = std::allocator<Entity>>
 class basic_registry;
 
@@ -172,7 +184,7 @@ using continuous_loader = basic_continuous_loader<registry>;
  * @tparam Exclude Types of storage used to filter the view.
  */
 template<typename Get, typename Exclude = exclude_t<>>
-using view = basic_view<type_list_transform_t<Get, storage_type>, type_list_transform_t<Exclude, storage_type>>;
+using view = basic_view<type_list_transform_t<Get, storage_for>, type_list_transform_t<Exclude, storage_for>>;
 
 /*! @brief Alias declaration for the most common use case. */
 using runtime_view = basic_runtime_view<sparse_set>;
@@ -187,7 +199,7 @@ using const_runtime_view = basic_runtime_view<const sparse_set>;
  * @tparam Exclude Types of storage used to filter the group.
  */
 template<typename Owned, typename Get, typename Exclude>
-using group = basic_group<type_list_transform_t<Owned, storage_type>, type_list_transform_t<Get, storage_type>, type_list_transform_t<Exclude, storage_type>>;
+using group = basic_group<type_list_transform_t<Owned, storage_for>, type_list_transform_t<Get, storage_for>, type_list_transform_t<Exclude, storage_for>>;
 
 } // namespace entt
 
diff --git a/src/entt/entity/registry.hpp b/src/entt/entity/registry.hpp
index 0c2c7d9fc..1727c60df 100644
--- a/src/entt/entity/registry.hpp
+++ b/src/entt/entity/registry.hpp
@@ -243,7 +243,7 @@ class basic_registry {
     using basic_common_type = basic_sparse_set<Entity, Allocator>;
 
     template<typename Type>
-    using storage_for = typename storage_type<Type, Entity, typename alloc_traits::template rebind_alloc<std::remove_const_t<Type>>>::type;
+    using storage_for_type = typename storage_for<Type, Entity, typename alloc_traits::template rebind_alloc<std::remove_const_t<Type>>>::type;
 
     template<typename...>
     struct group_handler;
@@ -251,7 +251,7 @@ class basic_registry {
     template<typename... Exclude, typename... Get, typename... Owned>
     struct group_handler<exclude_t<Exclude...>, get_t<Get...>, Owned...> {
         // nasty workaround for an issue with the toolset v141 that doesn't accept a fold expression here
-        static_assert(!std::disjunction_v<std::bool_constant<storage_for<Owned>::traits_type::in_place_delete>...>, "Groups do not support in-place delete");
+        static_assert(!std::disjunction_v<std::bool_constant<storage_for_type<Owned>::traits_type::in_place_delete>...>, "Groups do not support in-place delete");
         using value_type = std::conditional_t<sizeof...(Owned) == 0, basic_common_type, std::size_t>;
         value_type current{};
 
@@ -263,7 +263,7 @@ class basic_registry {
         void maybe_valid_if(basic_registry &owner, const Entity entt) {
             [[maybe_unused]] const auto cpools = std::forward_as_tuple(owner.assure<Owned>()...);
 
-            const auto is_valid = ((std::is_same_v<Type, Owned> || std::get<storage_for<Owned> &>(cpools).contains(entt)) && ...)
+            const auto is_valid = ((std::is_same_v<Type, Owned> || std::get<storage_for_type<Owned> &>(cpools).contains(entt)) && ...)
                                   && ((std::is_same_v<Type, Get> || owner.assure<Get>().contains(entt)) && ...)
                                   && ((std::is_same_v<Type, Exclude> || !owner.assure<Exclude>().contains(entt)) && ...);
 
@@ -274,7 +274,7 @@ class basic_registry {
             } else {
                 if(is_valid && !(std::get<0>(cpools).index(entt) < current)) {
                     const auto pos = current++;
-                    (std::get<storage_for<Owned> &>(cpools).swap_elements(std::get<storage_for<Owned> &>(cpools).data()[pos], entt), ...);
+                    (std::get<storage_for_type<Owned> &>(cpools).swap_elements(std::get<storage_for_type<Owned> &>(cpools).data()[pos], entt), ...);
                 }
             }
         }
@@ -285,7 +285,7 @@ class basic_registry {
             } else {
                 if(const auto cpools = std::forward_as_tuple(owner.assure<Owned>()...); std::get<0>(cpools).contains(entt) && (std::get<0>(cpools).index(entt) < current)) {
                     const auto pos = --current;
-                    (std::get<storage_for<Owned> &>(cpools).swap_elements(std::get<storage_for<Owned> &>(cpools).data()[pos], entt), ...);
+                    (std::get<storage_for_type<Owned> &>(cpools).swap_elements(std::get<storage_for_type<Owned> &>(cpools).data()[pos], entt), ...);
                 }
             }
         }
@@ -305,20 +305,20 @@ class basic_registry {
         auto &cpool = pools[id];
 
         if(!cpool) {
-            using alloc_type = typename storage_for<std::remove_const_t<Type>>::allocator_type;
+            using alloc_type = typename storage_for_type<std::remove_const_t<Type>>::allocator_type;
 
             if constexpr(std::is_same_v<Type, void> && !std::is_constructible_v<alloc_type, allocator_type>) {
                 // std::allocator<void> has no cross constructors (waiting for C++20)
-                cpool = std::allocate_shared<storage_for<std::remove_const_t<Type>>>(get_allocator(), alloc_type{});
+                cpool = std::allocate_shared<storage_for_type<std::remove_const_t<Type>>>(get_allocator(), alloc_type{});
             } else {
-                cpool = std::allocate_shared<storage_for<std::remove_const_t<Type>>>(get_allocator(), get_allocator());
+                cpool = std::allocate_shared<storage_for_type<std::remove_const_t<Type>>>(get_allocator(), get_allocator());
             }
 
             cpool->bind(forward_as_any(*this));
         }
 
         ENTT_ASSERT(cpool->type() == type_id<Type>(), "Unexpected type");
-        return static_cast<storage_for<Type> &>(*cpool);
+        return static_cast<storage_for_type<Type> &>(*cpool);
     }
 
     template<typename Type>
@@ -327,10 +327,10 @@ class basic_registry {
 
         if(const auto it = pools.find(id); it != pools.cend()) {
             ENTT_ASSERT(it->second->type() == type_id<Type>(), "Unexpected type");
-            return static_cast<const storage_for<Type> &>(*it->second);
+            return static_cast<const storage_for_type<Type> &>(*it->second);
         }
 
-        static storage_for<Type> placeholder{};
+        static storage_for_type<Type> placeholder{};
         return placeholder;
     }
 
@@ -1268,14 +1268,14 @@ public:
      * @return A newly created view.
      */
     template<typename Type, typename... Other, typename... Exclude>
-    [[nodiscard]] basic_view<get_t<storage_for<const Type>, storage_for<const Other>...>, exclude_t<storage_for<const Exclude>...>>
+    [[nodiscard]] basic_view<get_t<storage_for_type<const Type>, storage_for_type<const Other>...>, exclude_t<storage_for_type<const Exclude>...>>
     view(exclude_t<Exclude...> = {}) const {
         return {assure<std::remove_const_t<Type>>(), assure<std::remove_const_t<Other>>()..., assure<std::remove_const_t<Exclude>>()...};
     }
 
     /*! @copydoc view */
     template<typename Type, typename... Other, typename... Exclude>
-    [[nodiscard]] basic_view<get_t<storage_for<Type>, storage_for<Other>...>, exclude_t<storage_for<Exclude>...>>
+    [[nodiscard]] basic_view<get_t<storage_for_type<Type>, storage_for_type<Other>...>, exclude_t<storage_for_type<Exclude>...>>
     view(exclude_t<Exclude...> = {}) {
         return {assure<std::remove_const_t<Type>>(), assure<std::remove_const_t<Other>>()..., assure<std::remove_const_t<Exclude>>()...};
     }
@@ -1305,7 +1305,7 @@ public:
      * @return A newly created group.
      */
     template<typename... Owned, typename... Get, typename... Exclude>
-    [[nodiscard]] basic_group<owned_t<storage_for<Owned>...>, get_t<storage_for<Get>...>, exclude_t<storage_for<Exclude>...>>
+    [[nodiscard]] basic_group<owned_t<storage_for_type<Owned>...>, get_t<storage_for_type<Get>...>, exclude_t<storage_for_type<Exclude>...>>
     group(get_t<Get...> = {}, exclude_t<Exclude...> = {}) {
         static_assert(sizeof...(Owned) + sizeof...(Get) > 0, "Exclusion-only groups are not supported");
         static_assert(sizeof...(Owned) + sizeof...(Get) + sizeof...(Exclude) > 1, "Single component groups are not allowed");
@@ -1383,12 +1383,12 @@ public:
             }
         }
 
-        return {handler->current, std::get<storage_for<std::remove_const_t<Owned>> &>(cpools)..., std::get<storage_for<std::remove_const_t<Get>> &>(cpools)..., assure<std::remove_const_t<Exclude>>()...};
+        return {handler->current, std::get<storage_for_type<std::remove_const_t<Owned>> &>(cpools)..., std::get<storage_for_type<std::remove_const_t<Get>> &>(cpools)..., assure<std::remove_const_t<Exclude>>()...};
     }
 
     /*! @copydoc group */
     template<typename... Owned, typename... Get, typename... Exclude>
-    [[nodiscard]] basic_group<owned_t<storage_for<const Owned>...>, get_t<storage_for<const Get>...>, exclude_t<storage_for<const Exclude>...>>
+    [[nodiscard]] basic_group<owned_t<storage_for_type<const Owned>...>, get_t<storage_for_type<const Get>...>, exclude_t<storage_for_type<const Exclude>...>>
     group_if_exists(get_t<Get...> = {}, exclude_t<Exclude...> = {}) const {
         auto it = std::find_if(groups.cbegin(), groups.cend(), [](const auto &gdata) {
             return gdata.size == (sizeof...(Owned) + sizeof...(Get) + sizeof...(Exclude))
diff --git a/test/entt/entity/storage.cpp b/test/entt/entity/storage.cpp
index 80f7dfa9d..81a3933e7 100644
--- a/test/entt/entity/storage.cpp
+++ b/test/entt/entity/storage.cpp
@@ -1962,10 +1962,16 @@ TEST(Storage, UsesAllocatorConstruction) {
 
 TEST(Storage, StorageType) {
     // just a bunch of static asserts to avoid regressions
-    static_assert(std::is_same_v<entt::storage_type_t<const double, entt::entity>, const entt::sigh_mixin<entt::basic_storage<double, entt::entity>>>);
     static_assert(std::is_same_v<entt::storage_type_t<char, entt::entity>, entt::sigh_mixin<entt::basic_storage<char, entt::entity>>>);
-    static_assert(std::is_same_v<entt::storage_type_t<const bool>, const entt::sigh_mixin<entt::storage<bool>>>);
     static_assert(std::is_same_v<entt::storage_type_t<int>, entt::sigh_mixin<entt::storage<int>>>);
 }
 
+TEST(Storage, StorageFor) {
+    // just a bunch of static asserts to avoid regressions
+    static_assert(std::is_same_v<entt::storage_for_t<const double, entt::entity>, const entt::sigh_mixin<entt::basic_storage<double, entt::entity>>>);
+    static_assert(std::is_same_v<entt::storage_for_t<char, entt::entity>, entt::sigh_mixin<entt::basic_storage<char, entt::entity>>>);
+    static_assert(std::is_same_v<entt::storage_for_t<const bool>, const entt::sigh_mixin<entt::storage<bool>>>);
+    static_assert(std::is_same_v<entt::storage_for_t<int>, entt::sigh_mixin<entt::storage<int>>>);
+}
+
 #endif