Implement an alternative erase function that doesn't return an iterator.

Ref #3693

[SVN r58403]

doc/changes.qbk

@@ -107,6 +107,10 @@ First official release.
 
 * Support instantiating the containers with incomplete value types.
 * Reduced the number of warnings (mostly in tests).
+* [@http://svn.boost.org/trac/boost/ticket/3693 Ticket 3693]:
+  Add `erase_return_void` as a temporary workaround for the current
+  `erase` which can be inefficient because it has to find the next
+  element to return an iterator.
 * [@http://svn.boost.org/trac/boost/ticket/3773 Ticket 3773]:
   Add missing `std` qualifier to `ptrdiff_t`.
 

doc/ref.xml

@@ -459,6 +459,15 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
                 <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
                 <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
               </throws>
+              <notes>
+                <para>
+                  When the number of elements is a lot smaller than the number of buckets
+                  this function can be very inefficient as it has to search through empty
+                  buckets for the next element, in order to return the iterator.
+                  As a temporary workaround, the container has the method
+                  <methodname>erase_return_void</methodname> which will be faster.
+                </para>
+              </notes>
             </method>
             <method name="erase">
               <parameter name="k">
@@ -494,6 +503,27 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
                 <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
               </throws>
             </method>
+            <method name="erase_return_void">
+              <parameter name="position">
+                <paramtype>const_iterator</paramtype>
+              </parameter>
+              <type>iterator</type>
+              <description>
+                <para>Erase the element pointed to by <code>position</code>.</para>
+              </description>
+              <throws>
+                <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
+                <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
+              </throws>
+              <notes>
+                <para>
+                  This is a temporary workaround for the inefficient
+                  <methodname>erase</methodname> method. Hopefully, in a future
+                  version the signature of <methodname>erase</methodname> will
+                  be changed and this will be deprecated.
+                </para>
+              </notes>
+            </method>
             <method name="clear">
               <type>void</type>
               <description>
@@ -1252,6 +1282,15 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
                 <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
                 <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
               </throws>
+              <notes>
+                <para>
+                  When the number of elements is a lot smaller than the number of buckets
+                  this function can be very inefficient as it has to search through empty
+                  buckets for the next element, in order to return the iterator.
+                  As a temporary workaround, the container has the method
+                  <methodname>erase_return_void</methodname> which will be faster.
+                </para>
+              </notes>
             </method>
             <method name="erase">
               <parameter name="k">
@@ -1287,6 +1326,27 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
                 <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
               </throws>
             </method>
+            <method name="erase_return_void">
+              <parameter name="position">
+                <paramtype>const_iterator</paramtype>
+              </parameter>
+              <type>iterator</type>
+              <description>
+                <para>Erase the element pointed to by <code>position</code>.</para>
+              </description>
+              <throws>
+                <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
+                <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
+              </throws>
+              <notes>
+                <para>
+                  This is a temporary workaround for the inefficient
+                  <methodname>erase</methodname> method. Hopefully, in a future
+                  version the signature of <methodname>erase</methodname> will
+                  be changed and this will be deprecated.
+                </para>
+              </notes>
+            </method>
             <method name="clear">
               <type>void</type>
               <description>
@@ -2059,6 +2119,15 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
                 <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
                 <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
               </throws>
+              <notes>
+                <para>
+                  When the number of elements is a lot smaller than the number of buckets
+                  this function can be very inefficient as it has to search through empty
+                  buckets for the next element, in order to return the iterator.
+                  As a temporary workaround, the container has the method
+                  <methodname>erase_return_void</methodname> which will be faster.
+                </para>
+              </notes>
             </method>
             <method name="erase">
               <parameter name="k">
@@ -2094,6 +2163,27 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
                 <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
               </throws>
             </method>
+            <method name="erase_return_void">
+              <parameter name="position">
+                <paramtype>const_iterator</paramtype>
+              </parameter>
+              <type>iterator</type>
+              <description>
+                <para>Erase the element pointed to by <code>position</code>.</para>
+              </description>
+              <throws>
+                <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
+                <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
+              </throws>
+              <notes>
+                <para>
+                  This is a temporary workaround for the inefficient
+                  <methodname>erase</methodname> method. Hopefully, in a future
+                  version the signature of <methodname>erase</methodname> will
+                  be changed and this will be deprecated.
+                </para>
+              </notes>
+            </method>
             <method name="clear">
               <type>void</type>
               <description>
@@ -2901,6 +2991,15 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
                 <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
                 <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
               </throws>
+              <notes>
+                <para>
+                  When the number of elements is a lot smaller than the number of buckets
+                  this function can be very inefficient as it has to search through empty
+                  buckets for the next element, in order to return the iterator.
+                  As a temporary workaround, the container has the method
+                  <methodname>erase_return_void</methodname> which will be faster.
+                </para>
+              </notes>
             </method>
             <method name="erase">
               <parameter name="k">
@@ -2936,6 +3035,27 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
                 <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
               </throws>
             </method>
+            <method name="erase_return_void">
+              <parameter name="position">
+                <paramtype>const_iterator</paramtype>
+              </parameter>
+              <type>iterator</type>
+              <description>
+                <para>Erase the element pointed to by <code>position</code>.</para>
+              </description>
+              <throws>
+                <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
+                <para>In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations.</para>
+              </throws>
+              <notes>
+                <para>
+                  This is a temporary workaround for the inefficient
+                  <methodname>erase</methodname> method. Hopefully, in a future
+                  version the signature of <methodname>erase</methodname> will
+                  be changed and this will be deprecated.
+                </para>
+              </notes>
+            </method>
             <method name="clear">
               <type>void</type>
               <description>

include/boost/unordered/detail/fwd.hpp

@@ -532,7 +532,8 @@ namespace boost { namespace unordered_detail {
 
         void clear();
         std::size_t erase_key(key_type const& k);
-        iterator_base erase(iterator_base r);
+        iterator_base erase_return_iterator(iterator_base r);
+        void erase(iterator_base r);
         std::size_t erase_group(node_ptr* it, bucket_ptr bucket);
         iterator_base erase_range(iterator_base r1, iterator_base r2);
 

include/boost/unordered/detail/table.hpp

@@ -652,10 +652,20 @@ namespace boost { namespace unordered_detail {
         return *it ? this->erase_group(it, bucket) : 0;
     }
 
+    template <class T>
+    void hash_table<T>::erase(iterator_base r)
+    {
+        BOOST_ASSERT(r.node_);
+        --this->size_;
+        node::unlink_node(*r.bucket_, r.node_);
+        this->delete_node(r.node_);
+        // r has been invalidated but its bucket is still valid
+        this->recompute_begin_bucket(r.bucket_);
+    }
 
     template <class T>
     BOOST_DEDUCED_TYPENAME T::iterator_base
-        hash_table<T>::erase(iterator_base r)
+        hash_table<T>::erase_return_iterator(iterator_base r)
     {
         BOOST_ASSERT(r.node_);
         iterator_base next = r;

include/boost/unordered/unordered_map.hpp

@@ -355,7 +355,7 @@ namespace boost
 
         iterator erase(const_iterator position)
         {
-            return iterator(table_.erase(get(position)));
+            return iterator(table_.erase_return_iterator(get(position)));
         }
 
         size_type erase(const key_type& k)
@@ -368,6 +368,11 @@ namespace boost
             return iterator(table_.erase_range(get(first), get(last)));
         }
 
+        void erase_return_void(const_iterator position)
+        {
+            table_.erase(get(position));
+        }
+
         void clear()
         {
             table_.clear();
@@ -868,7 +873,7 @@ namespace boost
 
         iterator erase(const_iterator position)
         {
-            return iterator(table_.erase(get(position)));
+            return iterator(table_.erase_return_iterator(get(position)));
         }
 
         size_type erase(const key_type& k)
@@ -881,6 +886,11 @@ namespace boost
             return iterator(table_.erase_range(get(first), get(last)));
         }
 
+        void erase_return_void(const_iterator position)
+        {
+            table_.erase(get(position));
+        }
+
         void clear()
         {
             table_.clear();

include/boost/unordered/unordered_set.hpp

@@ -348,7 +348,7 @@ namespace boost
 
         iterator erase(const_iterator position)
         {
-            return iterator(table_.erase(get(position)));
+            return iterator(table_.erase_return_iterator(get(position)));
         }
 
         size_type erase(const key_type& k)
@@ -361,6 +361,11 @@ namespace boost
             return iterator(table_.erase_range(get(first), get(last)));
         }
 
+        void erase_return_void(const_iterator position)
+        {
+            table_.erase(get(position));
+        }
+
         void clear()
         {
             table_.clear();
@@ -822,7 +827,7 @@ namespace boost
 
         iterator erase(const_iterator position)
         {
-            return iterator(table_.erase(get(position)));
+            return iterator(table_.erase_return_iterator(get(position)));
         }
 
         size_type erase(const key_type& k)
@@ -835,6 +840,11 @@ namespace boost
             return iterator(table_.erase_range(get(first), get(last)));
         }
 
+        void erase_return_void(const_iterator position)
+        {
+            table_.erase(get(position));
+        }
+
         void clear()
         {
             table_.clear();

test/unordered/erase_tests.cpp

@@ -112,6 +112,55 @@ void erase_tests1(Container*, test::random_generator generator = test::default_g
         BOOST_TEST(x.erase(x.begin(), x.end()) == x.begin());
     }
 
+    std::cerr<<"erase_return_void(begin()).\n";
+    {
+        test::random_values<Container> v(1000, generator);
+        Container x(v.begin(), v.end());
+        std::size_t size = x.size();
+        while(size > 0 && !x.empty())
+        {
+            BOOST_DEDUCED_TYPENAME Container::key_type key = test::get_key<Container>(*x.begin());
+            std::size_t count = x.count(key);
+            x.erase_return_void(x.begin());
+            --size;
+            BOOST_TEST(x.count(key) == count - 1);
+            BOOST_TEST(x.size() == size);
+        }
+        BOOST_TEST(x.empty());
+    }
+
+    std::cerr<<"erase_return_void(random position).\n";
+    {
+        test::random_values<Container> v(1000, generator);
+        Container x(v.begin(), v.end());
+        std::size_t size = x.size();
+        while(size > 0 && !x.empty())
+        {
+            using namespace std;
+            int index = rand() % (int) x.size();
+            BOOST_DEDUCED_TYPENAME Container::const_iterator prev, pos, next;
+            if(index == 0) {
+                prev = pos = x.begin();
+            }
+            else {
+                prev = boost::next(x.begin(), index - 1);
+                pos = boost::next(prev);
+            }
+            next = boost::next(pos);
+            BOOST_DEDUCED_TYPENAME Container::key_type key = test::get_key<Container>(*pos);
+            std::size_t count = x.count(key);
+            x.erase_return_void(pos);
+            --size;
+            if(size > 0)
+                BOOST_TEST(index == 0 ? next == x.begin() :
+                        next == boost::next(prev));
+            BOOST_TEST(x.count(key) == count - 1);
+            BOOST_TEST(x.size() == size);
+        }
+        BOOST_TEST(x.empty());
+    }
+
+
     std::cerr<<"clear().\n";
     {
         test::random_values<Container> v(500, generator);