Merge pull request #193 from boostorg/feature/proxy-erase

Feature/proxy erase

doc/unordered/changes.adoc

@@ -10,6 +10,10 @@
 
 * Added `boost::concurrent_flat_map`, a fast, thread-safe hashmap based on open addressing.
 * Sped up iteration of open-addressing containers.
+* In open-addressing containers, `erase(iterator)`, which previously returned nothing, now
+returns a proxy object convertible to an iterator to the next element.
+This enables the typical `it = c.erase(it)` idiom without incurring any performance penalty
+when the returned proxy is not used.
 
 == Release 1.82.0 - Major update
 

doc/unordered/compliance.adoc

@@ -135,7 +135,9 @@ The main differences with C++ unordered associative containers are:
 
 * In general:
   ** `begin()` is not constant-time.
-  ** `erase(iterator)` returns `void` instead of an iterator to the following element.
+  ** `erase(iterator)` does not return an iterator to the following element, but
+     a proxy object that converts to that iterator if requested; this avoids
+     a potentially costly iterator increment operation when not needed.
   ** There is no API for bucket handling (except `bucket_count`).
   ** The maximum load factor of the container is managed internally and can't be set by the user. The maximum load,
      exposed through the public function `max_load`, may decrease on erasure under high-load conditions.

doc/unordered/unordered_flat_map.adoc

@@ -17,7 +17,6 @@ a number of aspects from that of `boost::unordered_flat_map`/`std::unordered_fla
   - `value_type` must be move-constructible. 
   - Pointer stability is not kept under rehashing.
   - `begin()` is not constant-time.
-  - `erase(iterator)` returns `void`.
   - There is no API for bucket handling (except `bucket_count`) or node extraction/insertion.
   - The maximum load factor of the container is managed internally and can't be set by the user.
 
@@ -155,9 +154,9 @@ namespace boost {
     template<class K, class M>
       iterator xref:#unordered_flat_map_insert_or_assign_with_hint[insert_or_assign](const_iterator hint, K&& k, M&& obj);
 
-    void      xref:#unordered_flat_map_erase_by_position[erase](iterator position);
-    void      xref:#unordered_flat_map_erase_by_position[erase](const_iterator position);
-    size_type xref:#unordered_flat_map_erase_by_key[erase](const key_type& k);
+    _convertible-to-iterator_     xref:#unordered_flat_map_erase_by_position[erase](iterator position);
+    _convertible-to-iterator_     xref:#unordered_flat_map_erase_by_position[erase](const_iterator position);
+    size_type                   xref:#unordered_flat_map_erase_by_key[erase](const key_type& k);
     template<class K> size_type xref:#unordered_flat_map_erase_by_key[erase](K&& k);
     iterator  xref:#unordered_flat_map_erase_range[erase](const_iterator first, const_iterator last);
     void      xref:#unordered_flat_map_swap[swap](unordered_flat_map& other)
@@ -1035,15 +1034,19 @@ The `template<class K, class M>` only participates in overload resolution if `Ha
 
 ==== Erase by Position
 
-```c++
-void erase(iterator position);
-void erase(const_iterator position);
-```
+[source,c++,subs=+quotes]
+----
+_convertible-to-iterator_ erase(iterator position);
+_convertible-to-iterator_ erase(const_iterator position);
+----
 
 Erase the element pointed to by `position`.
 
 [horizontal]
+Returns:;; An opaque object implicitly convertible to the `iterator` or `const_iterator`
+immediately following `position` prior to the erasure.
 Throws:;; Nothing.
+Notes:;; The opaque object returned must only be discarded or immediately converted to `iterator` or `const_iterator`.
 
 ---
 

doc/unordered/unordered_flat_set.adoc

@@ -17,7 +17,6 @@ a number of aspects from that of `boost::unordered_flat_set`/`std::unordered_fla
   - `value_type` must be move-constructible. 
   - Pointer stability is not kept under rehashing.
   - `begin()` is not constant-time.
-  - `erase(iterator)` returns `void`.
   - There is no API for bucket handling (except `bucket_count`) or node extraction/insertion.
   - The maximum load factor of the container is managed internally and can't be set by the user.
 
@@ -123,9 +122,9 @@ namespace boost {
     template<class InputIterator> void xref:#unordered_flat_set_insert_iterator_range[insert](InputIterator first, InputIterator last);
     void xref:#unordered_flat_set_insert_initializer_list[insert](std::initializer_list<value_type>);
 
-    void      xref:#unordered_flat_set_erase_by_position[erase](iterator position);
-    void      xref:#unordered_flat_set_erase_by_position[erase](const_iterator position);
-    size_type xref:#unordered_flat_set_erase_by_key[erase](const key_type& k);
+    _convertible-to-iterator_     xref:#unordered_flat_set_erase_by_position[erase](iterator position);
+    _convertible-to-iterator_     xref:#unordered_flat_set_erase_by_position[erase](const_iterator position);
+    size_type                   xref:#unordered_flat_set_erase_by_key[erase](const key_type& k);
     template<class K> size_type xref:#unordered_flat_set_erase_by_key[erase](K&& k);
     iterator  xref:#unordered_flat_set_erase_range[erase](const_iterator first, const_iterator last);
     void      xref:#unordered_flat_set_swap[swap](unordered_flat_set& other)
@@ -846,15 +845,19 @@ Notes:;; Can invalidate iterators, pointers and references, but only if the inse
 
 ==== Erase by Position
 
-```c++
-void erase(iterator position);
-void erase(const_iterator position);
-```
+[source,c++,subs=+quotes]
+----
+_convertible-to-iterator_ erase(iterator position);
+_convertible-to-iterator_ erase(const_iterator position);
+----
 
 Erase the element pointed to by `position`.
 
 [horizontal]
+Returns:;; An opaque object implicitly convertible to the `iterator` or `const_iterator`
+immediately following `position` prior to the erasure.
 Throws:;; Nothing.
+Notes:;; The opaque object returned must only be discarded or immediately converted to `iterator` or `const_iterator`.
 
 ---
 

doc/unordered/unordered_node_map.adoc

@@ -13,7 +13,6 @@ As a result of its using open addressing, the interface of `boost::unordered_nod
 a number of aspects from that of `boost::unordered_map`/`std::unordered_map`: 
 
   - `begin()` is not constant-time.
-  - `erase(iterator)` returns `void`.
   - There is no API for bucket handling (except `bucket_count`).
   - The maximum load factor of the container is managed internally and can't be set by the user.
 
@@ -156,9 +155,9 @@ namespace boost {
     template<class K, class M>
       iterator xref:#unordered_node_map_insert_or_assign_with_hint[insert_or_assign](const_iterator hint, K&& k, M&& obj);
 
-    void      xref:#unordered_node_map_erase_by_position[erase](iterator position);
-    void      xref:#unordered_node_map_erase_by_position[erase](const_iterator position);
-    size_type xref:#unordered_node_map_erase_by_key[erase](const key_type& k);
+    _convertible-to-iterator_     xref:#unordered_node_map_erase_by_position[erase](iterator position);
+    _convertible-to-iterator_     xref:#unordered_node_map_erase_by_position[erase](const_iterator position);
+    size_type                   xref:#unordered_node_map_erase_by_key[erase](const key_type& k);
     template<class K> size_type xref:#unordered_node_map_erase_by_key[erase](K&& k);
     iterator  xref:#unordered_node_map_erase_range[erase](const_iterator first, const_iterator last);
     void      xref:#unordered_node_map_swap[swap](unordered_node_map& other)
@@ -1105,15 +1104,19 @@ The `template<class K, class M>` only participates in overload resolution if `Ha
 
 ==== Erase by Position
 
-```c++
-void erase(iterator position);
-void erase(const_iterator position);
-```
+[source,c++,subs=+quotes]
+----
+_convertible-to-iterator_ erase(iterator position);
+_convertible-to-iterator_ erase(const_iterator position);
+----
 
 Erase the element pointed to by `position`.
 
 [horizontal]
+Returns:;; An opaque object implicitly convertible to the `iterator` or `const_iterator`
+immediately following `position` prior to the erasure.
 Throws:;; Nothing.
+Notes:;; The opaque object returned must only be discarded or immediately converted to `iterator` or `const_iterator`.
 
 ---
 

doc/unordered/unordered_node_set.adoc

@@ -13,7 +13,6 @@ As a result of its using open addressing, the interface of `boost::unordered_nod
 a number of aspects from that of `boost::unordered_set`/`std::unordered_set`: 
 
   - `begin()` is not constant-time.
-  - `erase(iterator)` returns `void`.
   - There is no API for bucket handling (except `bucket_count`).
   - The maximum load factor of the container is managed internally and can't be set by the user.
 
@@ -124,9 +123,9 @@ namespace boost {
     insert_return_type xref:#unordered_node_set_insert_node[insert](node_type&& nh);
     iterator xref:#unordered_node_set_insert_node_with_hint[insert](const_iterator hint, node_type&& nh);
 
-    void      xref:#unordered_node_set_erase_by_position[erase](iterator position);
-    void      xref:#unordered_node_set_erase_by_position[erase](const_iterator position);
-    size_type xref:#unordered_node_set_erase_by_key[erase](const key_type& k);
+    _convertible-to-iterator_     xref:#unordered_node_set_erase_by_position[erase](iterator position);
+    _convertible-to-iterator_     xref:#unordered_node_set_erase_by_position[erase](const_iterator position);
+    size_type                   xref:#unordered_node_set_erase_by_key[erase](const key_type& k);
     template<class K> size_type xref:#unordered_node_set_erase_by_key[erase](K&& k);
     iterator  xref:#unordered_node_set_erase_range[erase](const_iterator first, const_iterator last);
     void      xref:#unordered_node_set_swap[swap](unordered_node_set& other)
@@ -919,15 +918,19 @@ Notes:;; Behavior is undefined if `nh` is not empty and the allocators of `nh` a
 
 ==== Erase by Position
 
-```c++
-void erase(iterator position);
-void erase(const_iterator position);
-```
+[source,c++,subs=+quotes]
+----
+_convertible-to-iterator_ erase(iterator position);
+_convertible-to-iterator_ erase(const_iterator position);
+----
 
 Erase the element pointed to by `position`.
 
 [horizontal]
+Returns:;; An opaque object implicitly convertible to the `iterator` or `const_iterator`
+immediately following `position` prior to the erasure.
 Throws:;; Nothing.
+Notes:;; The opaque object returned must only be discarded or immediately converted to `iterator` or `const_iterator`.
 
 ---
 

include/boost/unordered/detail/foa/table.hpp

@@ -123,6 +123,7 @@ public:
 
 private:
   template<typename,typename,bool> friend class table_iterator;
+  template<typename> friend class table_erase_return_type;
   template<typename,typename,typename,typename> friend class table;
 
   table_iterator(Group* pg,std::size_t n,const table_element_type* p_):
@@ -198,6 +199,45 @@ private:
   table_element_type *p=nullptr;
 };
 
+/* Returned by table::erase([const_]iterator) to avoid iterator increment
+ * if discarded.
+ */
+
+template<typename Iterator>
+class table_erase_return_type; 
+
+template<typename TypePolicy,typename Group,bool Const>
+class table_erase_return_type<table_iterator<TypePolicy,Group,Const>>
+{
+  using iterator=table_iterator<TypePolicy,Group,Const>;
+  using const_iterator=table_iterator<TypePolicy,Group,true>;
+
+public:
+  /* can't delete it because VS in pre-C++17 mode needs to see it for RVO */
+  table_erase_return_type(const table_erase_return_type&);
+
+  operator iterator()const noexcept
+  {
+    auto it=pos;
+    it.increment(); /* valid even if *it was erased */
+    return iterator(const_iterator_cast_tag{},it);
+  }
+
+  template<
+    bool dependent_value=false,
+    typename std::enable_if<!Const||dependent_value>::type* =nullptr
+  >
+  operator const_iterator()const noexcept{return this->operator iterator();}
+
+private:
+  template<typename,typename,typename,typename> friend class table;
+
+  table_erase_return_type(const_iterator pos_):pos{pos_}{}
+  table_erase_return_type& operator=(const table_erase_return_type&)=delete;
+
+  const_iterator pos;
+};
+
 /* foa::table interface departs in a number of ways from that of C++ unordered
  * associative containers because it's not for end-user consumption
  * (boost::unordered_(flat|node)_(map|set) wrappers complete it as
@@ -271,6 +311,7 @@ public:
     has_mutable_iterator,
     table_iterator<type_policy,group_type,false>,
     const_iterator>::type;
+  using erase_return_type=table_erase_return_type<iterator>;
 
   table(
     std::size_t n=default_bucket_count,const Hash& h_=Hash(),
@@ -353,12 +394,14 @@ public:
     typename std::enable_if<
       has_mutable_iterator||dependent_value>::type* =nullptr
   >
-  void erase(iterator pos)noexcept{return erase(const_iterator(pos));}
+  erase_return_type erase(iterator pos)noexcept
+  {return erase(const_iterator(pos));}
 
   BOOST_FORCEINLINE
-  void erase(const_iterator pos)noexcept
+  erase_return_type erase(const_iterator pos)noexcept
   {
     super::erase(pos.pc,pos.p);
+    return {pos};
   }
 
   template<typename Key>

include/boost/unordered/unordered_flat_map.hpp

@@ -381,11 +381,18 @@ namespace boost {
           .first;
       }
 
-      BOOST_FORCEINLINE void erase(iterator pos) { table_.erase(pos); }
-      BOOST_FORCEINLINE void erase(const_iterator pos)
+      BOOST_FORCEINLINE typename table_type::erase_return_type erase(
+        iterator pos)
       {
         return table_.erase(pos);
       }
+
+      BOOST_FORCEINLINE typename table_type::erase_return_type erase(
+        const_iterator pos)
+      {
+        return table_.erase(pos);
+      }
+
       iterator erase(const_iterator first, const_iterator last)
       {
         while (first != last) {

include/boost/unordered/unordered_flat_set.hpp

@@ -282,10 +282,12 @@ namespace boost {
         return table_.emplace(std::forward<Args>(args)...).first;
       }
 
-      BOOST_FORCEINLINE void erase(const_iterator pos)
+      BOOST_FORCEINLINE typename table_type::erase_return_type erase(
+        const_iterator pos)
       {
         return table_.erase(pos);
       }
+
       iterator erase(const_iterator first, const_iterator last)
       {
         while (first != last) {

include/boost/unordered/unordered_node_map.hpp

@@ -458,11 +458,18 @@ namespace boost {
           .first;
       }
 
-      BOOST_FORCEINLINE void erase(iterator pos) { table_.erase(pos); }
-      BOOST_FORCEINLINE void erase(const_iterator pos)
+      BOOST_FORCEINLINE typename table_type::erase_return_type erase(
+        iterator pos)
       {
         return table_.erase(pos);
       }
+
+      BOOST_FORCEINLINE typename table_type::erase_return_type erase(
+        const_iterator pos)
+      {
+        return table_.erase(pos);
+      }
+
       iterator erase(const_iterator first, const_iterator last)
       {
         while (first != last) {

include/boost/unordered/unordered_node_set.hpp

@@ -352,10 +352,12 @@ namespace boost {
         return table_.emplace(std::forward<Args>(args)...).first;
       }
 
-      BOOST_FORCEINLINE void erase(const_iterator pos)
+      BOOST_FORCEINLINE typename table_type::erase_return_type erase(
+        const_iterator pos)
       {
         return table_.erase(pos);
       }
+
       iterator erase(const_iterator first, const_iterator last)
       {
         while (first != last) {

test/unordered/compile_tests.hpp

@@ -844,7 +844,7 @@ void unordered_copyable_test(X& x, Key& k, T& t, Hash& hf, Pred& eq)
   a10.insert(t);
   q = a10.cbegin();
 #ifdef BOOST_UNORDERED_FOA_TESTS
-  BOOST_STATIC_ASSERT(std::is_same<void, decltype(a10.erase(q))>::value);
+  test::check_return_type<iterator>::convertible(a10.erase(q));
 #else
   test::check_return_type<iterator>::equals(a10.erase(q));
 #endif
@@ -937,7 +937,7 @@ void unordered_movable_test(X& x, Key& k, T& /* t */, Hash& hf, Pred& eq)
   a10.insert(boost::move(v5));
   q = a10.cbegin();
 #ifdef BOOST_UNORDERED_FOA_TESTS
-  BOOST_STATIC_ASSERT(std::is_same<void, decltype(a10.erase(q))>::value);
+  test::check_return_type<iterator>::convertible(a10.erase(q));
 #else
   test::check_return_type<iterator>::equals(a10.erase(q));
 #endif

test/unordered/erase_tests.cpp

@@ -23,9 +23,7 @@ namespace erase_tests {
   template <class Container>
   void erase_tests1(Container*, test::random_generator generator)
   {
-#ifndef BOOST_UNORDERED_FOA_TESTS
     typedef typename Container::iterator iterator;
-#endif
     typedef typename Container::const_iterator c_iterator;
 
     BOOST_LIGHTWEIGHT_TEST_OSTREAM << "Erase by key.\n";
@@ -59,12 +57,8 @@ namespace erase_tests {
       while (size > 0 && !x.empty()) {
         typename Container::key_type key = test::get_key<Container>(*x.begin());
         std::size_t count = x.count(key);
-#ifdef BOOST_UNORDERED_FOA_TESTS
-        x.erase(x.begin());
-#else
         iterator pos = x.erase(x.begin());
         BOOST_TEST(pos == x.begin());
-#endif
         --size;
         BOOST_TEST(x.count(key) == count - 1);
         BOOST_TEST(x.size() == size);
@@ -95,15 +89,10 @@ namespace erase_tests {
         typename Container::key_type key = test::get_key<Container>(*pos);
         std::size_t count = x.count(key);
         BOOST_TEST(count > 0);
-#ifdef BOOST_UNORDERED_FOA_TESTS
-        x.erase(pos);
-        --size;
-#else
         BOOST_TEST(next == x.erase(pos));
         --size;
         if (size > 0)
           BOOST_TEST(index == 0 ? next == x.begin() : next == test::next(prev));
-#endif
         BOOST_TEST(x.count(key) == count - 1);
         if (x.count(key) != count - 1) {
           BOOST_LIGHTWEIGHT_TEST_OSTREAM << count << " => " << x.count(key)

test/unordered/reserve_tests.cpp

@@ -213,8 +213,8 @@ UNORDERED_AUTO_TEST (allocator_check) {
   typedef boost::allocator_rebind<A<int>, float>::type alloc_rebound;
   alloc_rebound b;
   A<int> a(b);
-  BOOST_ASSERT(alloc_rebound(a) == b);
-  BOOST_ASSERT(A<int>(b) == a);
+  BOOST_TEST(alloc_rebound(a) == b);
+  BOOST_TEST(A<int>(b) == a);
 }
 
 #ifdef BOOST_UNORDERED_FOA_TESTS

test/unordered/transparent_tests.cpp

@@ -1065,11 +1065,7 @@ typedef boost::unordered_map<int, int, transparent_hasher,
 // test that in the presence of the member function template `erase()`, we still
 // invoke the correct iterator overloads when the type is implicitly convertible
 //
-#ifdef BOOST_UNORDERED_FOA_TESTS
-void
-#else
 transparent_unordered_map::iterator
-#endif
 map_erase_overload_compile_test()
 {
   convertible_to_iterator<transparent_unordered_map> c;
@@ -1079,11 +1075,7 @@ map_erase_overload_compile_test()
   return map.erase(c);
 }
 
-#ifdef BOOST_UNORDERED_FOA_TESTS
-void
-#else
 transparent_unordered_map::const_iterator
-#endif
 map_erase_const_overload_compile_test()
 {
   convertible_to_const_iterator<transparent_unordered_map> c;
@@ -1226,11 +1218,7 @@ typedef boost::unordered_multiset<int, transparent_hasher,
   transparent_unordered_multiset;
 #endif
 
-#ifdef BOOST_UNORDERED_FOA_TESTS
-void
-#else
 transparent_unordered_set::iterator
-#endif
 set_erase_overload_compile_test()
 {
   convertible_to_iterator<transparent_unordered_set> c;
@@ -1240,11 +1228,7 @@ set_erase_overload_compile_test()
   return set.erase(c);
 }
 
-#ifdef BOOST_UNORDERED_FOA_TESTS
-void
-#else
 transparent_unordered_set::const_iterator
-#endif
 set_erase_const_overload_compile_test()
 {
   convertible_to_const_iterator<transparent_unordered_set> c;
@@ -1657,7 +1641,7 @@ template <class UnorderedMap> void test_map_transparent_subscript(UnorderedMap*)
   int key_count = key::count_;
 
   map[0] = 7331;
-  BOOST_ASSERT(BOOST_TEST_EQ(key::count_, key_count));
+  BOOST_TEST_EQ(key::count_, key_count);
 
   map[4] = 7331;
   BOOST_TEST_EQ(key::count_, key_count + 1);
@@ -1680,7 +1664,7 @@ void test_map_non_transparent_subscript(UnorderedMap*)
   int key_count = key::count_;
 
   map[0] = 7331;
-  BOOST_ASSERT(BOOST_TEST_EQ(key::count_, key_count + 1));
+  BOOST_TEST_EQ(key::count_, key_count + 1);
 
   key_count = key::count_;
   map[4] = 7331;