Begin cleanup of node_handle implementation

include/boost/unordered/detail/foa.hpp

@@ -92,32 +92,32 @@ struct insert_return_type
   NodeType node;
 };
 
-template <class NodeTypes,class Allocator>
+template <class TypePolicy,class Allocator>
 struct node_handle_base
 {
   protected:
-    using type_policy=NodeTypes;
+    using type_policy=TypePolicy;
+    using value_type=typename type_policy::value_type;
     using element_type=typename type_policy::element_type;
 
   public:
     using allocator_type = Allocator;
 
   private:
-    alignas(element_type) unsigned char x_[sizeof(element_type)];
+    value_type* p_=nullptr;
-    alignas(Allocator) unsigned char a_[sizeof(Allocator)];
+    alignas(Allocator) unsigned char a_[sizeof(Allocator)]={0};
-    bool empty_=true;
 
   protected:
-    element_type& element()noexcept
+    value_type& element()noexcept
     {
       BOOST_ASSERT(!empty());
-      return *reinterpret_cast<element_type*>(x_);
+      return *p_;
     }
 
-    element_type const& element()const noexcept
+    value_type const& element()const noexcept
     {
       BOOST_ASSERT(!empty());
-      return *reinterpret_cast<element_type const*>(x_);
+      return *p_;
     }
 
     Allocator& al()noexcept
@@ -132,13 +132,23 @@ struct node_handle_base
       return *reinterpret_cast<Allocator const*>(a_);
     }
 
-    void emplace(element_type x,Allocator a)
+    void emplace(value_type* p,Allocator a)
     {
       BOOST_ASSERT(empty());
-
+      p_=p;
-      new(x_)element_type(std::move(x));
       new(a_)Allocator(a);
-      empty_=false;
+    }
+
+    void emplace(element_type&& x,Allocator a)
+    {
+      emplace(x.p,a);
+      x.p=nullptr;
+    }
+
+    void clear()
+    {
+      al().~Allocator();
+      p_=nullptr;
     }
 
   public:
@@ -147,16 +157,8 @@ struct node_handle_base
     node_handle_base(node_handle_base&& nh) noexcept
     {
       if (!nh.empty()){
-        // neither of these move constructors are allowed to throw exceptions
+        emplace(nh.p_,nh.al());
-        // so we can get away with rote placement new
+        nh.clear();
-        //
-        new(a_)Allocator(std::move(nh.al()));
-        new(x_)element_type(std::move(nh.element()));
-        empty_=false;
-
-        reinterpret_cast<element_type*>(nh.x_)->~element_type();
-        reinterpret_cast<Allocator*>(nh.a_)->~Allocator();
-        nh.empty_ = true;
       }
     }
 
@@ -167,22 +169,19 @@ struct node_handle_base
           Allocator>::type::value;
 
       if(!empty()){
-        type_policy::destroy(al(),reinterpret_cast<element_type*>(x_));
+        type_policy::destroy(al(),p_);
-        reinterpret_cast<element_type*>(x_)->~element_type();
         if (pocma&&!nh.empty()){al()=std::move(nh.al());}
       }
 
       if(!nh.empty()){
-        new(x_)element_type(std::move(nh.element()));
         if(empty()){new(a_)Allocator(std::move(nh.al()));}
-        empty_=false;
+        p_=nh.p_;
 
-        reinterpret_cast<element_type*>(nh.x_)->~element_type();
+        nh.p_=nullptr;
         reinterpret_cast<Allocator*>(nh.a_)->~Allocator();
-        nh.empty_=true;
       }else if (!empty()){
         reinterpret_cast<Allocator*>(a_)->~Allocator();
-        empty_=true;
+        p_=nullptr;
       }
 
       return *this;
@@ -191,16 +190,14 @@ struct node_handle_base
     ~node_handle_base()
     {
       if(!empty()){
-        type_policy::destroy(al(),reinterpret_cast<element_type*>(x_));
+        type_policy::destroy(al(),p_);
-        reinterpret_cast<element_type*>(x_)->~element_type();
         reinterpret_cast<Allocator*>(a_)->~Allocator();
-        empty_=true;
       }
     }
 
     allocator_type get_allocator()const noexcept{return al();}
     explicit operator bool()const noexcept{ return !empty();}
-    BOOST_ATTRIBUTE_NODISCARD bool empty()const noexcept{return empty_;}
+    BOOST_ATTRIBUTE_NODISCARD bool empty()const noexcept{return p_==nullptr;}
 
     void swap(node_handle_base& nh) noexcept(
       boost::allocator_is_always_equal<Allocator>::type::value||
@@ -214,7 +211,9 @@ struct node_handle_base
       if (!empty()&&!nh.empty()){
         BOOST_ASSERT(pocs || al()==nh.al());
 
-        element().swap(nh.element());
+        value_type *p=p_;
+        p_=nh.p_;
+        nh.p_=p;
 
         if(pocs){
           swap(al(),nh.al());
@@ -226,21 +225,11 @@ struct node_handle_base
       if (empty()&&nh.empty()){return;}
 
       if (empty()){
-        new(x_)element_type(std::move(nh.element()));
+        emplace(nh.p_,nh.al());
-        new(a_)Allocator(nh.al());
+        nh.clear();
-        empty_=false;
-
-        reinterpret_cast<element_type*>(nh.x_)->~element_type();
-        reinterpret_cast<Allocator*>(nh.a_)->~Allocator();
-        nh.empty_=true;
       }else{
-        new(nh.x_)element_type(std::move(element()));
+        nh.emplace(p_,al());
-        new(nh.a_)Allocator(al());
+        clear();
-        nh.empty_=false;
-
-        reinterpret_cast<element_type*>(x_)->~element_type();
-        reinterpret_cast<Allocator*>(a_)->~Allocator();
-        empty_=true;
       }
     }
 
@@ -1668,13 +1657,12 @@ public:
     }
   }
 
-  element_type extract(const_iterator pos)noexcept
+  element_type extract(const_iterator pos)
   {
     BOOST_ASSERT(pos!=end());
-    element_type x=std::move(*pos.p);
+    erase_on_exit e{*this,iterator{const_iterator_cast_tag{},pos}};
-    destroy_element(pos.p);
+    (void)e;
-    recover_slot(pos.pc);
+    return std::move(*pos.p);
-    return x;
   }
 
   // TODO: should we accept different allocator too?

include/boost/unordered/unordered_node_map.hpp

@@ -60,10 +60,6 @@ namespace boost {
             p = rhs.p;
             rhs.p = nullptr;
           }
-
-          void swap(element_type& rhs) noexcept {
-            std::swap(p, rhs.p);
-          }
         };
 
         static value_type& value_from(element_type const& x) { return *(x.p); }
@@ -118,14 +114,19 @@ namespace boost {
           BOOST_CATCH_END
         }
 
+        template <class A> static void destroy(A& al, value_type* p) noexcept
+        {
+          boost::allocator_destroy(al, p);
+          boost::allocator_deallocate(al,
+            boost::pointer_traits<
+              typename boost::allocator_pointer<A>::type>::pointer_to(*p),
+            1);
+        }
+
         template <class A> static void destroy(A& al, element_type* p) noexcept
         {
           if (p->p) {
-            boost::allocator_destroy(al, p->p);
+            destroy(al,p->p);
-            boost::allocator_deallocate(al,
-              boost::pointer_traits<
-                typename boost::allocator_pointer<A>::type>::pointer_to(*p->p),
-              1);
           }
         }
       };
@@ -159,14 +160,13 @@ namespace boost {
         key_type& key() const
         {
           BOOST_ASSERT(!empty());
-          return const_cast<key_type&>(type_policy::extract(element()));
+          return const_cast<key_type&>(element().first);
         }
 
         mapped_type& mapped() const
         {
           BOOST_ASSERT(!empty());
-          return const_cast<mapped_type&>(
+          return const_cast<mapped_type&>(element().second);
-            type_policy::value_from(element()).second);
         }
       };
     } // namespace detail
@@ -405,8 +405,12 @@ namespace boost {
 
         BOOST_ASSERT(get_allocator() == nh.get_allocator());
 
-        auto itp = table_.emplace_impl(map_types::move(nh.element()));
+        typename map_types::element_type x;
+        x.p=std::addressof(nh.element());
+
+        auto itp = table_.emplace_impl(std::move(x));
         if (itp.second) {
+          nh.clear();
           return {itp.first, true, node_type{}};
         } else {
           return {itp.first, false, std::move(nh)};

include/boost/unordered/unordered_node_set.hpp

@@ -56,10 +56,6 @@ namespace boost {
             p = rhs.p;
             rhs.p = nullptr;
           }
-
-          void swap(element_type& rhs) noexcept {
-            std::swap(p, rhs.p);
-          }
         };
 
         static value_type& value_from(element_type const& x) { return *x.p; }
@@ -100,14 +96,19 @@ namespace boost {
           BOOST_CATCH_END
         }
 
+        template <class A> static void destroy(A& al, value_type* p) noexcept
+        {
+          boost::allocator_destroy(al, p);
+          boost::allocator_deallocate(al,
+            boost::pointer_traits<
+              typename boost::allocator_pointer<A>::type>::pointer_to(*p),
+            1);
+        }
+
         template <class A> static void destroy(A& al, element_type* p) noexcept
         {
           if (p->p) {
-            boost::allocator_destroy(al, p->p);
+            destroy(al, p->p);
-            boost::allocator_deallocate(al,
-              boost::pointer_traits<
-                typename boost::allocator_pointer<A>::type>::pointer_to(*p->p),
-              1);
           }
         }
       };
@@ -139,7 +140,7 @@ namespace boost {
         value_type& value() const
         {
           BOOST_ASSERT(!empty());
-          return const_cast<value_type&>(type_policy::extract(element()));
+          return const_cast<value_type&>(element());
         }
       };
     } // namespace detail
@@ -392,8 +393,12 @@ namespace boost {
 
         BOOST_ASSERT(get_allocator() == nh.get_allocator());
 
-        auto itp = table_.emplace_impl(set_types::move(nh.element()));
+        typename set_types::element_type x;
+        x.p=std::addressof(nh.element());
+
+        auto itp = table_.emplace_impl(std::move(x));
         if (itp.second) {
+          nh.clear();
           return {itp.first, true, node_type{}};
         } else {
           return {itp.first, false, std::move(nh)};

test/unordered/node_handle_tests.cpp

@@ -119,7 +119,6 @@ static void failed_insertion_with_hint()
 
     typename Set<int>::node_type nh = src.extract(10);
 
-    std::cout << "performing relevant test now" << std::endl;
     BOOST_TEST(dst.insert(dst.find(10), boost::move(nh)) == dst.find(10));
     BOOST_TEST(nh);
     BOOST_TEST(!nh.empty());