ソースを参照

table: redesign it as an adaptor rather than a container

Michele Caini 1 年間 前
コミット
2f8b4835ca
3 ファイル変更137 行追加72 行削除
  1. 3 2
      src/entt/entity/fwd.hpp
  2. 94 61
      src/entt/entity/table.hpp
  3. 40 9
      test/entt/entity/table.cpp

+ 3 - 2
src/entt/entity/fwd.hpp

@@ -4,6 +4,7 @@
 #include <cstdint>
 #include <memory>
 #include <type_traits>
+#include <vector>
 #include "../core/fwd.hpp"
 #include "../core/type_traits.hpp"
 
@@ -28,7 +29,7 @@ class basic_sparse_set;
 template<typename Type, typename = entity, typename = std::allocator<Type>, typename = void>
 class basic_storage;
 
-template<typename, typename = std::allocator<void>>
+template<typename...>
 class basic_table;
 
 template<typename, typename>
@@ -79,7 +80,7 @@ using storage = basic_storage<Type>;
  * @tparam Type Element types.
  */
 template<typename... Type>
-using table = basic_table<type_list<Type...>>;
+using table = basic_table<std::vector<Type>...>;
 
 /**
  * @brief Alias declaration for the most common use case.

+ 94 - 61
src/entt/entity/table.hpp

@@ -3,13 +3,10 @@
 
 #include <cstddef>
 #include <iterator>
-#include <memory>
 #include <tuple>
 #include <type_traits>
 #include <utility>
-#include <vector>
 #include "../config/config.h"
-#include "../core/compressed_pair.hpp"
 #include "../core/iterator.hpp"
 #include "fwd.hpp"
 
@@ -146,44 +143,46 @@ template<typename... Lhs, typename... Rhs>
  * no guarantees that objects are returned in the insertion order when iterate
  * a table. Do not make assumption on the order in any case.
  *
- * @tparam Row Element types.
- * @tparam Allocator Type of allocator used to manage memory and elements.
+ * @tparam Container Sequence container row types.
  */
-template<typename... Row, typename Allocator>
-class basic_table<type_list<Row...>, Allocator> {
-    using alloc_traits = std::allocator_traits<Allocator>;
-    static_assert(sizeof...(Row) != 0u, "Empty tables not allowed");
-
-    template<typename Type>
-    using container_for = std::vector<Type, typename alloc_traits::template rebind_alloc<Type>>;
-
-    using container_type = std::tuple<container_for<Row>...>;
+template<typename... Container>
+class basic_table {
+    using container_type = std::tuple<Container...>;
 
 public:
-    /*! @brief Allocator type. */
-    using allocator_type = Allocator;
     /*! @brief Unsigned integer type. */
     using size_type = std::size_t;
     /*! @brief Input iterator type. */
-    using iterator = internal::table_iterator<typename container_for<Row>::iterator...>;
+    using iterator = internal::table_iterator<typename Container::iterator...>;
     /*! @brief Constant input iterator type. */
-    using const_iterator = internal::table_iterator<typename container_for<Row>::const_iterator...>;
+    using const_iterator = internal::table_iterator<typename Container::const_iterator...>;
     /*! @brief Reverse iterator type. */
-    using reverse_iterator = internal::table_iterator<typename container_for<Row>::reverse_iterator...>;
+    using reverse_iterator = internal::table_iterator<typename Container::reverse_iterator...>;
     /*! @brief Constant reverse iterator type. */
-    using const_reverse_iterator = internal::table_iterator<typename container_for<Row>::const_reverse_iterator...>;
+    using const_reverse_iterator = internal::table_iterator<typename Container::const_reverse_iterator...>;
 
     /*! @brief Default constructor. */
     basic_table()
-        : basic_table{allocator_type{}} {
+        : payload{} {
     }
 
     /**
-     * @brief Constructs an empty table with a given allocator.
-     * @param allocator The allocator to use.
+     * @brief Copy constructs the underlying containers.
+     * @param container The containers to copy from.
+     */
+    explicit basic_table(const Container &...container) noexcept
+        : payload{container...} {
+        ENTT_ASSERT((((std::get<Container>(payload).size() * sizeof...(Container)) == (std::get<Container>(payload).size() + ...)) && ...), "Unexpected container size");
+    }
+
+    /**
+     * @brief Move constructs the underlying containers.
+     * @param container The containers to move from.
      */
-    explicit basic_table(const allocator_type &allocator)
-        : payload{container_type{container_for<Row>{allocator}...}, allocator} {}
+    explicit basic_table(Container &&...container) noexcept
+        : payload{std::move(container)...} {
+        ENTT_ASSERT((((std::get<Container>(payload).size() * sizeof...(Container)) == (std::get<Container>(payload).size() + ...)) && ...), "Unexpected container size");
+    }
 
     /**
      * @brief Move constructor.
@@ -192,15 +191,48 @@ public:
     basic_table(basic_table &&other) noexcept
         : payload{std::move(other.payload)} {}
 
+    /**
+     * @brief Constructs the underlying containers using a given allocator.
+     * @tparam Allocator Type of allocator.
+     * @param allocator A valid allocator.
+     */
+    template<typename Allocator>
+    explicit basic_table(const Allocator &allocator)
+        : payload{Container{allocator}...} {}
+
+    /**
+     * @brief Copy constructs the underlying containers using a given allocator.
+     * @tparam Allocator Type of allocator.
+     * @param container The containers to copy from.
+     * @param allocator A valid allocator.
+     */
+    template<class Allocator>
+    basic_table(const Container &...container, const Allocator &allocator) noexcept
+        : payload{Container{container, allocator}...} {
+        ENTT_ASSERT((((std::get<Container>(payload).size() * sizeof...(Container)) == (std::get<Container>(payload).size() + ...)) && ...), "Unexpected container size");
+    }
+
+    /**
+     * @brief Move constructs the underlying containers using a given allocator.
+     * @tparam Allocator Type of allocator.
+     * @param container The containers to move from.
+     * @param allocator A valid allocator.
+     */
+    template<class Allocator>
+    basic_table(Container &&...container, const Allocator &allocator) noexcept
+        : payload{Container{std::move(container), allocator}...} {
+        ENTT_ASSERT((((std::get<Container>(payload).size() * sizeof...(Container)) == (std::get<Container>(payload).size() + ...)) && ...), "Unexpected container size");
+    }
+
     /**
      * @brief Allocator-extended move constructor.
+     * @tparam Allocator Type of allocator.
      * @param other The instance to move from.
      * @param allocator The allocator to use.
      */
-    basic_table(basic_table &&other, const allocator_type &allocator) noexcept
-        : payload{container_type{container_for<Row>{std::move(std::get<container_for<Row>>(other.payload.first())), allocator}...}, allocator} {
-        ENTT_ASSERT(alloc_traits::is_always_equal::value || get_allocator() == other.get_allocator(), "Copying a table is not allowed");
-    }
+    template<class Allocator>
+    basic_table(basic_table &&other, const Allocator &allocator) noexcept
+        : payload{Container{std::move(std::get<Container>(other.payload)), allocator}...} {}
 
     /**
      * @brief Move assignment operator.
@@ -208,7 +240,6 @@ public:
      * @return This table.
      */
     basic_table &operator=(basic_table &&other) noexcept {
-        ENTT_ASSERT(alloc_traits::is_always_equal::value || get_allocator() == other.get_allocator(), "Copying a table is not allowed");
         payload = std::move(other.payload);
         return *this;
     }
@@ -222,14 +253,6 @@ public:
         swap(payload, other.payload);
     }
 
-    /**
-     * @brief Returns the associated allocator.
-     * @return The associated allocator.
-     */
-    [[nodiscard]] constexpr allocator_type get_allocator() const noexcept {
-        return payload.second();
-    }
-
     /**
      * @brief Increases the capacity of a table.
      *
@@ -239,7 +262,7 @@ public:
      * @param cap Desired capacity.
      */
     void reserve(const size_type cap) {
-        (std::get<container_for<Row>>(payload.first()).reserve(cap), ...);
+        (std::get<Container>(payload).reserve(cap), ...);
     }
 
     /**
@@ -248,12 +271,12 @@ public:
      * @return Capacity of the table.
      */
     [[nodiscard]] size_type capacity() const noexcept {
-        return std::get<0>(payload.first()).capacity();
+        return std::get<0>(payload).capacity();
     }
 
     /*! @brief Requests the removal of unused capacity. */
     void shrink_to_fit() {
-        (std::get<container_for<Row>>(payload.first()).shrink_to_fit(), ...);
+        (std::get<Container>(payload).shrink_to_fit(), ...);
     }
 
     /**
@@ -261,7 +284,7 @@ public:
      * @return Number of rows.
      */
     [[nodiscard]] size_type size() const noexcept {
-        return std::get<0>(payload.first()).size();
+        return std::get<0>(payload).size();
     }
 
     /**
@@ -269,7 +292,7 @@ public:
      * @return True if the table is empty, false otherwise.
      */
     [[nodiscard]] bool empty() const noexcept {
-        return std::get<0>(payload.first()).empty();
+        return std::get<0>(payload).empty();
     }
 
     /**
@@ -280,7 +303,7 @@ public:
      * @return An iterator to the first row of the table.
      */
     [[nodiscard]] const_iterator cbegin() const noexcept {
-        return {std::get<container_for<Row>>(payload.first()).cbegin()...};
+        return {std::get<Container>(payload).cbegin()...};
     }
 
     /*! @copydoc cbegin */
@@ -290,7 +313,7 @@ public:
 
     /*! @copydoc begin */
     [[nodiscard]] iterator begin() noexcept {
-        return {std::get<container_for<Row>>(payload.first()).begin()...};
+        return {std::get<Container>(payload).begin()...};
     }
 
     /**
@@ -298,7 +321,7 @@ public:
      * @return An iterator to the element following the last row of the table.
      */
     [[nodiscard]] const_iterator cend() const noexcept {
-        return {std::get<container_for<Row>>(payload.first()).cend()...};
+        return {std::get<Container>(payload).cend()...};
     }
 
     /*! @copydoc cend */
@@ -308,7 +331,7 @@ public:
 
     /*! @copydoc end */
     [[nodiscard]] iterator end() noexcept {
-        return {std::get<container_for<Row>>(payload.first()).end()...};
+        return {std::get<Container>(payload).end()...};
     }
 
     /**
@@ -319,7 +342,7 @@ public:
      * @return An iterator to the first row of the reversed table.
      */
     [[nodiscard]] const_reverse_iterator crbegin() const noexcept {
-        return {std::get<container_for<Row>>(payload.first()).crbegin()...};
+        return {std::get<Container>(payload).crbegin()...};
     }
 
     /*! @copydoc crbegin */
@@ -329,7 +352,7 @@ public:
 
     /*! @copydoc rbegin */
     [[nodiscard]] reverse_iterator rbegin() noexcept {
-        return {std::get<container_for<Row>>(payload.first()).rbegin()...};
+        return {std::get<Container>(payload).rbegin()...};
     }
 
     /**
@@ -338,7 +361,7 @@ public:
      * table.
      */
     [[nodiscard]] const_reverse_iterator crend() const noexcept {
-        return {std::get<container_for<Row>>(payload.first()).crend()...};
+        return {std::get<Container>(payload).crend()...};
     }
 
     /*! @copydoc crend */
@@ -348,7 +371,7 @@ public:
 
     /*! @copydoc rend */
     [[nodiscard]] reverse_iterator rend() noexcept {
-        return {std::get<container_for<Row>>(payload.first()).rend()...};
+        return {std::get<Container>(payload).rend()...};
     }
 
     /**
@@ -358,11 +381,11 @@ public:
      * @return A reference to the newly created row data.
      */
     template<typename... Args>
-    std::tuple<Row &...> emplace(Args &&...args) {
+    std::tuple<typename Container::value_type &...> emplace(Args &&...args) {
         if constexpr(sizeof...(Args) == 0u) {
-            return std::forward_as_tuple(std::get<container_for<Row>>(payload.first()).emplace_back()...);
+            return std::forward_as_tuple(std::get<Container>(payload).emplace_back()...);
         } else {
-            return std::forward_as_tuple(std::get<container_for<Row>>(payload.first()).emplace_back(std::forward<Args>(args))...);
+            return std::forward_as_tuple(std::get<Container>(payload).emplace_back(std::forward<Args>(args))...);
         }
     }
 
@@ -373,7 +396,7 @@ public:
      */
     iterator erase(const_iterator pos) {
         const auto diff = pos - begin();
-        return {std::get<container_for<Row>>(payload.first()).erase(std::get<container_for<Row>>(payload.first()).begin() + diff)...};
+        return {std::get<Container>(payload).erase(std::get<Container>(payload).begin() + diff)...};
     }
 
     /**
@@ -390,26 +413,36 @@ public:
      * @param pos The row for which to return the data.
      * @return The row data at specified location.
      */
-    [[nodiscard]] std::tuple<const Row &...> operator[](const size_type pos) const {
+    [[nodiscard]] std::tuple<const typename Container::value_type &...> operator[](const size_type pos) const {
         ENTT_ASSERT(pos < size(), "Index out of bounds");
-        return std::forward_as_tuple(std::get<container_for<Row>>(payload.first())[pos]...);
+        return std::forward_as_tuple(std::get<Container>(payload)[pos]...);
     }
 
     /*! @copydoc operator[] */
-    [[nodiscard]] std::tuple<Row &...> operator[](const size_type pos) {
+    [[nodiscard]] std::tuple<typename Container::value_type &...> operator[](const size_type pos) {
         ENTT_ASSERT(pos < size(), "Index out of bounds");
-        return std::forward_as_tuple(std::get<container_for<Row>>(payload.first())[pos]...);
+        return std::forward_as_tuple(std::get<Container>(payload)[pos]...);
     }
 
     /*! @brief Clears a table. */
     void clear() {
-        (std::get<container_for<Row>>(payload.first()).clear(), ...);
+        (std::get<Container>(payload).clear(), ...);
     }
 
 private:
-    compressed_pair<container_type, allocator_type> payload;
+    container_type payload;
 };
 
 } // namespace entt
 
+/*! @cond TURN_OFF_DOXYGEN */
+namespace std {
+
+template<typename... Container, typename Allocator>
+struct uses_allocator<entt::basic_table<Container...>, Allocator>
+    : std::bool_constant<(std::uses_allocator_v<Container, Allocator> && ...)> {};
+
+} // namespace std
+/*! @endcond */
+
 #endif

+ 40 - 9
test/entt/entity/table.cpp

@@ -10,13 +10,43 @@
 #include "../../common/throwing_allocator.hpp"
 
 TEST(Table, Constructors) {
-    entt::table<int, char> table;
+    const std::vector<int> vec_of_int{1};
+    const std::vector<char> vec_of_char{'a'};
+    entt::table<int, char> table{};
 
-    ASSERT_NO_THROW([[maybe_unused]] auto alloc = table.get_allocator());
+    ASSERT_TRUE(table.empty());
 
     table = entt::table<int, char>{std::allocator<void>{}};
 
-    ASSERT_NO_THROW([[maybe_unused]] auto alloc = table.get_allocator());
+    ASSERT_TRUE(table.empty());
+
+    table = entt::table<int, char>{vec_of_int, vec_of_char};
+
+    ASSERT_EQ(table.size(), 1);
+
+    table = entt::table<int, char>{std::vector<int>{1, 2}, std::vector<char>{'a', 'b'}};
+
+    ASSERT_EQ(table.size(), 2);
+
+    table = entt::table<int, char>{vec_of_int, vec_of_char, std::allocator<void>{}};
+
+    ASSERT_EQ(table.size(), 1);
+
+    table = entt::table<int, char>{std::vector<int>{1, 2}, std::vector<char>{'a', 'b'}, std::allocator<void>{}};
+
+    ASSERT_EQ(table.size(), 2);
+}
+
+ENTT_DEBUG_TEST(TableDeathTest, Constructors) {
+    const std::vector<int> vec_of_int{0};
+    const std::vector<char> vec_of_char{};
+    entt::table<int, char> table{};
+
+    ASSERT_DEATH((table = entt::table<int, char>{vec_of_int, vec_of_char}), "");
+    ASSERT_DEATH((table = entt::table<int, char>{std::vector<int>{}, std::vector<char>{'\0'}}), "");
+
+    ASSERT_DEATH((table = entt::table<int, char>{vec_of_int, vec_of_char, std::allocator<void>{}}), "");
+    ASSERT_DEATH((table = entt::table<int, char>{std::vector<int>{}, std::vector<char>{'\0'}, std::allocator<void>{}}), "");
 }
 
 TEST(Table, Move) {
@@ -399,7 +429,7 @@ TEST(Table, Erase) {
     ASSERT_EQ(table.size(), 0u);
 }
 
-TEST(TableDeathTest, Erase) {
+ENTT_DEBUG_TEST(TableDeathTest, Erase) {
     entt::table<int, char> table;
 
     ASSERT_DEATH(table.erase(0u), "");
@@ -451,7 +481,7 @@ TEST(Table, Clear) {
 
 TEST(Table, CustomAllocator) {
     const test::throwing_allocator<void> allocator{};
-    entt::basic_table<entt::type_list<int, char>, test::throwing_allocator<void>> table{allocator};
+    entt::basic_table<std::vector<int, test::throwing_allocator<int>>, std::vector<char, test::throwing_allocator<char>>> table{allocator};
 
     table.reserve(1u);
 
@@ -493,14 +523,15 @@ TEST(Table, CustomAllocator) {
 }
 
 TEST(Table, ThrowingAllocator) {
-    entt::basic_table<entt::type_list<int, char>, test::throwing_allocator<void>> table{};
+    test::throwing_allocator<void> allocator{};
+    entt::basic_table<std::vector<int, test::throwing_allocator<int>>, std::vector<char, test::throwing_allocator<char>>> table{allocator};
 
-    table.get_allocator().template throw_counter<int>(0u);
+    allocator.throw_counter<int>(0u);
 
     ASSERT_THROW(table.reserve(1u), test::throwing_allocator_exception);
 
-    table.get_allocator().template throw_counter<int>(0u);
-    table.get_allocator().template throw_counter<char>(0u);
+    allocator.throw_counter<int>(0u);
+    allocator.throw_counter<char>(0u);
 
     ASSERT_THROW(table.emplace(), test::throwing_allocator_exception);
     ASSERT_THROW(table.emplace(3, 'c'), test::throwing_allocator_exception);