storage: return iterator to elements rather than entities and only if it makes sense

src/entt/entity/storage.hpp

@@ -711,15 +711,15 @@ public:
      * @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 An instance of the object to construct.
-     * @return Iterator pointing to the first entity inserted, if any.
+     * @return Iterator pointing to the first element inserted, if any.
      */
     template<typename It>
-    typename base_type::iterator insert(It first, It last, const value_type &value = {}) {
+    iterator insert(It first, It last, const value_type &value = {}) {
         for(; first != last; ++first) {
             emplace_element(*first, true, value);
         }
 
-        return base_type::begin();
+        return begin();
     }
 
     /**
@@ -733,15 +733,15 @@ public:
      * @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 from An iterator to the first element of the range of objects.
-     * @return Iterator pointing to the first entity inserted, if any.
+     * @return Iterator pointing to the first element inserted, if any.
      */
     template<typename EIt, typename CIt, typename = std::enable_if_t<std::is_same_v<typename std::iterator_traits<CIt>::value_type, value_type>>>
-    typename base_type::iterator insert(EIt first, EIt last, CIt from) {
+    iterator insert(EIt first, EIt last, CIt from) {
         for(; first != last; ++first, ++from) {
             emplace_element(*first, true, *from);
         }
 
-        return base_type::begin();
+        return begin();
     }
 
     /**
@@ -891,15 +891,12 @@ public:
      * @tparam Args Types of optional arguments.
      * @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.
-     * @return Iterator pointing to the first entity inserted, if any.
      */
     template<typename It, typename... Args>
-    typename base_type::iterator insert(It first, It last, Args &&...) {
+    void insert(It first, It last, Args &&...) {
         for(; first != last; ++first) {
             base_type::try_emplace(*first, true);
         }
-
-        return base_type::begin();
     }
 
     /**
@@ -1124,7 +1121,7 @@ public:
      * @return Iterator pointing to the first entity inserted, if any.
      */
     template<typename It>
-    typename base_type::iterator insert(It first, It last) {
+    void insert(It first, It last) {
         for(const auto sz = base_type::size(); first != last && length != sz; ++first, ++length) {
             *first = base_type::operator[](length);
         }
@@ -1132,8 +1129,6 @@ public:
         for(; first != last; ++first) {
             *first = *base_type::try_emplace(entity_at(length++), true);
         }
-
-        return (base_type::end() - length);
     }
 
     /**

test/entt/entity/storage.cpp

@@ -315,12 +315,11 @@ ENTT_DEBUG_TEST_F(StorageDeathTest, EmptyType) {
 TEST_F(Storage, Insert) {
     entt::storage<stable_type> pool;
     entt::entity entities[2u]{entt::entity{3}, entt::entity{42}};
-    typename entt::sparse_set::iterator it{};
-    const entt::sparse_set &base = pool;
+    entt::storage<stable_type>::iterator it{};
 
     it = pool.insert(std::begin(entities), std::end(entities), stable_type{99});
 
-    ASSERT_EQ(it, base.cbegin());
+    ASSERT_EQ(it, pool.cbegin());
 
     ASSERT_TRUE(pool.contains(entities[0u]));
     ASSERT_TRUE(pool.contains(entities[1u]));
@@ -334,7 +333,7 @@ TEST_F(Storage, Insert) {
     const stable_type values[2u] = {stable_type{42}, stable_type{3}};
     it = pool.insert(std::rbegin(entities), std::rend(entities), std::begin(values));
 
-    ASSERT_EQ(it, base.cbegin());
+    ASSERT_EQ(it, pool.cbegin());
 
     ASSERT_EQ(pool.size(), 4u);
     ASSERT_EQ(pool.at(2u), entities[1u]);
@@ -348,12 +347,9 @@ TEST_F(Storage, Insert) {
 TEST_F(Storage, InsertEmptyType) {
     entt::storage<empty_stable_type> pool;
     entt::entity entities[2u]{entt::entity{3}, entt::entity{42}};
-    typename entt::sparse_set::iterator it{};
     const entt::sparse_set &base = pool;
 
-    it = pool.insert(std::begin(entities), std::end(entities));
-
-    ASSERT_EQ(it, base.cbegin());
+    pool.insert(std::begin(entities), std::end(entities));
 
     ASSERT_TRUE(pool.contains(entities[0u]));
     ASSERT_TRUE(pool.contains(entities[1u]));
@@ -363,9 +359,7 @@ TEST_F(Storage, InsertEmptyType) {
 
     pool.erase(std::begin(entities), std::end(entities));
     const empty_stable_type values[2u]{};
-    it = pool.insert(std::rbegin(entities), std::rend(entities), std::begin(values));
-
-    ASSERT_EQ(it, base.cbegin());
+    pool.insert(std::rbegin(entities), std::rend(entities), std::begin(values));
 
     ASSERT_EQ(pool.size(), 4u);
     ASSERT_EQ(pool.at(2u), entities[1u]);

test/entt/entity/storage_entity.cpp

@@ -227,12 +227,9 @@ TEST(StorageEntity, Insert) {
 
     entt::storage<entt::entity> pool;
     entt::entity entities[2u]{};
-    typename entt::sparse_set::iterator it{};
     const entt::sparse_set &base = pool;
 
-    it = pool.insert(std::begin(entities), std::end(entities));
-
-    ASSERT_EQ(it, base.cbegin());
+    pool.insert(std::begin(entities), std::end(entities));
 
     ASSERT_TRUE(pool.contains(entities[0u]));
     ASSERT_TRUE(pool.contains(entities[1u]));
@@ -247,10 +244,7 @@ TEST(StorageEntity, Insert) {
     ASSERT_EQ(pool.size(), 2u);
     ASSERT_EQ(pool.in_use(), 0u);
 
-    it = pool.insert(entities, entities + 1u);
-
-    ASSERT_NE(it, base.cbegin());
-    ASSERT_EQ(it, base.cbegin() + 1u);
+    pool.insert(entities, entities + 1u);
 
     ASSERT_TRUE(pool.contains(entities[0u]));
     ASSERT_FALSE(pool.contains(entities[1u]));