[libcxx] r260514 - Teach __hash_table how to handle unordered_map's __hash_value_type.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 11 04:25:27 PST 2016


Author: ericwf
Date: Thu Feb 11 06:25:27 2016
New Revision: 260514

URL: http://llvm.org/viewvc/llvm-project?rev=260514&view=rev
Log:
Teach __hash_table how to handle unordered_map's __hash_value_type.

This patch is fairly large and contains a number of changes. The main change
is teaching '__hash_table' how to handle '__hash_value_type'. Unfortunately
this change is a rampant layering violation, but it's required to make
unordered_map conforming without re-writing all of __hash_table.
After this change 'unordered_map' can delegate to '__hash_table' in almost all cases.

The major changes found in this patch are:

  * Teach __hash_table to differentiate between the true container value type
    and the node value type by introducing the "__container_value_type" and
    "__node_value_type" typedefs. In the case of unordered_map '__container_value_type'
    is 'pair<const Key, Value>' and '__node_value_type' is '__hash_value_type'.
    
  * Switch almost all overloads in '__hash_table' previously taking 'value_type'
    (AKA '__node_value_type) to take  '__container_value_type' instead. Previously
    'pair<K, V>' would be implicitly converted to '__hash_value_type<K, V>' because
    of the function signature.
    
  * Add '__get_key', '__get_value', '__get_ptr', and '__move' static functions to
    '__key_value_types'. These functions allow '__hash_table' to unwrap
    '__node_value_type' objects into '__container_value_type' and its sub-parts.

  * Pass  '__hash_value_type::__value_'  to 'a.construct(p, ...)' instead of
    '__hash_value_type' itself. The C++14 standard requires that 'a.construct()'
    and 'a.destroy()' are only ever instantiated for the containers value type.

  * Remove '__hash_value_type's constructors and destructors. We should never
    construct an instance of this type.
    (TODO this is UB but we already do it in plenty of places).
  
  * Add a generic "try-emplace" function to '__hash_table' called
    '__emplace_unique_key_args(Key const&, Args...)'.

  
The following changes were done as cleanup:

  * Introduce the '_LIBCPP_CXX03_LANG' macro to be used in place of
    '_LIBCPP_HAS_NO_VARIADICS' or '_LIBCPP_HAS_NO_RVALUE_REFERENCE'.
    
  * Cleanup C++11 only overloads that assume an incomplete C++11 implementation.
    For example this patch removes the __construct_node overloads that do
    manual pack expansion.
    
  * Forward 'unordered_map::emplace' to '__hash_table' and remove dead code
    resulting from the change. This includes almost all
    'unordered_map::__construct_node' overloads.


The following changes are planed for future revisions:

  * Fix LWG issue #2469 by delegating 'unordered_map::operator[]' to use
    '__emplace_unique_key_args'.
    
  * Rewrite 'unordered_map::try_emplace' in terms of '__emplace_unique_key_args'.
  
  * Optimize '__emplace_unique' to call '__emplace_unique_key_args' when possible.
    This prevent unneeded allocations when inserting duplicate entries.


The additional follow up work needed after this patch:

  * Respect the lifetime rules for '__hash_value_type' by actually constructing it.
  * Make '__insert_multi' act similar to '__insert_unique' for objects of type
    'T&' and 'T const &&' with 'T = __container_value_type'.
  
  

Modified:
    libcxx/trunk/include/unordered_map
    libcxx/trunk/test/std/containers/unord/unord.map/unord.map.elem/index.pass.cpp
    libcxx/trunk/test/std/containers/unord/unord.map/unord.map.elem/index_tuple.pass.cpp
    libcxx/trunk/test/support/container_test_types.h

Modified: libcxx/trunk/include/unordered_map
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/unordered_map?rev=260514&r1=260513&r2=260514&view=diff
==============================================================================
--- libcxx/trunk/include/unordered_map (original)
+++ libcxx/trunk/include/unordered_map Thu Feb 11 06:25:27 2016
@@ -369,6 +369,7 @@ template <class Key, class T, class Hash
 #include <__hash_table>
 #include <functional>
 #include <stdexcept>
+#include <tuple>
 
 #include <__debug>
 
@@ -1128,7 +1129,7 @@ public:
         {return __table_.__equal_range_unique(__k);}
 
     mapped_type& operator[](const key_type& __k);
-#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
+#ifndef _LIBCPP_CXX03_LANG
     mapped_type& operator[](key_type&& __k);
 #endif
 
@@ -1184,10 +1185,10 @@ public:
 #endif  // _LIBCPP_DEBUG_LEVEL >= 2
 
 private:
-#ifndef _LIBCPP_CXX03_LANG
-    __node_holder __construct_node_with_key(key_type&& __k);
-#endif  // _LIBCPP_CXX03_LANG
+
+#ifdef _LIBCPP_CXX03_LANG
     __node_holder __construct_node_with_key(const key_type& __k);
+#endif
 };
 
 template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
@@ -1394,23 +1395,7 @@ unordered_map<_Key, _Tp, _Hash, _Pred, _
 
 #endif  // _LIBCPP_HAS_NO_GENERALIZED_INITIALIZERS
 
-#ifndef _LIBCPP_CXX03_LANG
-
-template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
-typename unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::__node_holder
-unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::__construct_node_with_key(key_type&& __k)
-{
-    __node_allocator& __na = __table_.__node_alloc();
-    __node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
-    __node_traits::construct(__na, _VSTD::addressof(__h->__value_.__cc.first), _VSTD::move(__k));
-    __h.get_deleter().__first_constructed = true;
-    __node_traits::construct(__na, _VSTD::addressof(__h->__value_.__cc.second));
-    __h.get_deleter().__second_constructed = true;
-    return __h;
-}
-
-#endif
-
+#ifdef _LIBCPP_CXX03_LANG
 template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
 typename unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::__node_holder
 unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::__construct_node_with_key(const key_type& __k)
@@ -1423,6 +1408,7 @@ unordered_map<_Key, _Tp, _Hash, _Pred, _
     __h.get_deleter().__second_constructed = true;
     return _LIBCPP_EXPLICIT_MOVE(__h);  // explicitly moved for C++03
 }
+#endif
 
 template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
 template <class _InputIterator>
@@ -1435,6 +1421,7 @@ unordered_map<_Key, _Tp, _Hash, _Pred, _
         __table_.__insert_unique(*__first);
 }
 
+#ifdef _LIBCPP_CXX03_LANG
 template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
 _Tp&
 unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::operator[](const key_type& __k)
@@ -1447,23 +1434,27 @@ unordered_map<_Key, _Tp, _Hash, _Pred, _
     __h.release();
     return __r.first->second;
 }
+#else
 
-#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
+template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
+_Tp&
+unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::operator[](const key_type& __k)
+{
+    return __table_.__emplace_unique_key_args(__k,
+        std::piecewise_construct, std::forward_as_tuple(__k),
+                                  std::forward_as_tuple()).first->__cc.second;
+}
 
 template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
 _Tp&
 unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::operator[](key_type&& __k)
 {
-    iterator __i = find(__k);
-    if (__i != end())
-        return __i->second;
-    __node_holder __h = __construct_node_with_key(_VSTD::move(__k));
-    pair<iterator, bool> __r = __table_.__node_insert_unique(__h.get());
-    __h.release();
-    return __r.first->second;
+    return __table_.__emplace_unique_key_args(__k,
+        std::piecewise_construct, std::forward_as_tuple(std::move(__k)),
+                                  std::forward_as_tuple()).first->__cc.second;
 }
 
-#endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
+#endif  // !_LIBCPP_CXX03_MODE
 
 template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
 _Tp&

Modified: libcxx/trunk/test/std/containers/unord/unord.map/unord.map.elem/index.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/unord/unord.map/unord.map.elem/index.pass.cpp?rev=260514&r1=260513&r2=260514&view=diff
==============================================================================
--- libcxx/trunk/test/std/containers/unord/unord.map/unord.map.elem/index.pass.cpp (original)
+++ libcxx/trunk/test/std/containers/unord/unord.map/unord.map.elem/index.pass.cpp Thu Feb 11 06:25:27 2016
@@ -19,8 +19,11 @@
 #include <string>
 #include <cassert>
 
+#include "test_macros.h"
 #include "MoveOnly.h"
 #include "min_allocator.h"
+#include "count_new.hpp"
+#include "container_test_types.h"
 
 int main()
 {
@@ -44,7 +47,7 @@ int main()
         assert(c.size() == 5);
         assert(c.at(11) == "eleven");
     }
-#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
+#if TEST_STD_VER >= 11
     {
         typedef std::unordered_map<MoveOnly, std::string> C;
         typedef std::pair<int, std::string> P;
@@ -65,8 +68,6 @@ int main()
         assert(c.size() == 5);
         assert(c.at(11) == "eleven");
     }
-#endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
-#if __cplusplus >= 201103L
     {
         typedef std::unordered_map<int, std::string, std::hash<int>, std::equal_to<int>,
                             min_allocator<std::pair<const int, std::string>>> C;
@@ -88,7 +89,7 @@ int main()
         assert(c.size() == 5);
         assert(c.at(11) == "eleven");
     }
-#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
+
     {
         typedef std::unordered_map<MoveOnly, std::string, std::hash<MoveOnly>, std::equal_to<MoveOnly>,
                             min_allocator<std::pair<const MoveOnly, std::string>>> C;
@@ -110,6 +111,50 @@ int main()
         assert(c.size() == 5);
         assert(c.at(11) == "eleven");
     }
-#endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
+    {
+        using Container = TCT::unordered_map<>;
+        using Key = Container::key_type;
+        using MappedType = Container::mapped_type;
+        using ValueTp = Container::value_type;
+        ConstructController* cc = getConstructController();
+        cc->reset();
+        {
+            Container c;
+            const Key k(1);
+            cc->expect<std::piecewise_construct_t const&, std::tuple<Key const&>&&, std::tuple<>&&>();
+            MappedType& mref = c[k];
+            assert(!cc->unchecked());
+            {
+                DisableAllocationGuard g;
+                MappedType& mref2 = c[k];
+                assert(&mref == &mref2);
+            }
+        }
+        {
+            Container c;
+            Key k(1);
+            cc->expect<std::piecewise_construct_t const&, std::tuple<Key const&>&&, std::tuple<>&&>();
+            MappedType& mref = c[k];
+            assert(!cc->unchecked());
+            {
+                DisableAllocationGuard g;
+                MappedType& mref2 = c[k];
+                assert(&mref == &mref2);
+            }
+        }
+        {
+            Container c;
+            Key k(1);
+            cc->expect<std::piecewise_construct_t const&, std::tuple<Key &&>&&, std::tuple<>&&>();
+            MappedType& mref = c[std::move(k)];
+            assert(!cc->unchecked());
+            {
+                Key k2(1);
+                DisableAllocationGuard g;
+                MappedType& mref2 = c[std::move(k2)];
+                assert(&mref == &mref2);
+            }
+        }
+    }
 #endif
 }

Modified: libcxx/trunk/test/std/containers/unord/unord.map/unord.map.elem/index_tuple.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/unord/unord.map/unord.map.elem/index_tuple.pass.cpp?rev=260514&r1=260513&r2=260514&view=diff
==============================================================================
--- libcxx/trunk/test/std/containers/unord/unord.map/unord.map.elem/index_tuple.pass.cpp (original)
+++ libcxx/trunk/test/std/containers/unord/unord.map/unord.map.elem/index_tuple.pass.cpp Thu Feb 11 06:25:27 2016
@@ -7,6 +7,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+// UNSUPPORTED: c++98, c++03
+
 // <unordered_map>
 
 // template <class Key, class T, class Hash = hash<Key>, class Pred = equal_to<Key>,
@@ -18,9 +20,6 @@
 // http://llvm.org/bugs/show_bug.cgi?id=16542
 
 #include <unordered_map>
-
-#ifndef _LIBCPP_HAS_NO_VARIADICS
-
 #include <tuple>
 
 using namespace std;
@@ -30,12 +29,8 @@ struct my_hash
     size_t operator()(const tuple<int,int>&) const {return 0;}
 };
 
-#endif
-
 int main()
 {
-#ifndef _LIBCPP_HAS_NO_VARIADICS
     unordered_map<tuple<int,int>, size_t, my_hash> m;
     m[make_tuple(2,3)]=7;
-#endif
 }

Modified: libcxx/trunk/test/support/container_test_types.h
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/support/container_test_types.h?rev=260514&r1=260513&r2=260514&view=diff
==============================================================================
--- libcxx/trunk/test/support/container_test_types.h (original)
+++ libcxx/trunk/test/support/container_test_types.h Thu Feb 11 06:25:27 2016
@@ -393,6 +393,11 @@ struct CopyInsertable {
     }
   }
 
+  CopyInsertable() : data(0), copied_once(false), constructed_under_allocator(true)
+  {
+    assert(getConstructController()->isInAllocatorConstruct());
+  }
+
   CopyInsertable(CopyInsertable const& other) : data(other.data),
                                                 copied_once(true),
                                                 constructed_under_allocator(true) {




More information about the cfe-commits mailing list