From ba41d7e1ff83f1aff8ec58815b4147c97c7ced97 Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Thu, 11 Feb 2016 12:25:27 +0000 Subject: [PATCH] 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' 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' would be implicitly converted to '__hash_value_type' 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'. git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@260514 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/unordered_map | 51 +++++++---------- .../unord.map/unord.map.elem/index.pass.cpp | 55 +++++++++++++++++-- .../unord.map.elem/index_tuple.pass.cpp | 9 +-- test/support/container_test_types.h | 5 ++ 4 files changed, 78 insertions(+), 42 deletions(-) diff --git a/include/unordered_map b/include/unordered_map index 7e84f74d7..372868a0d 100644 --- a/include/unordered_map +++ b/include/unordered_map @@ -369,6 +369,7 @@ template #include <__hash_table> #include #include +#include #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 @@ -1394,23 +1395,7 @@ unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::operator=( #endif // _LIBCPP_HAS_NO_GENERALIZED_INITIALIZERS -#ifndef _LIBCPP_CXX03_LANG - -template -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 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, _Alloc>::__construct_node_with_key(const __h.get_deleter().__second_constructed = true; return _LIBCPP_EXPLICIT_MOVE(__h); // explicitly moved for C++03 } +#endif template template @@ -1435,6 +1421,7 @@ unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::insert(_InputIterator __first, __table_.__insert_unique(*__first); } +#ifdef _LIBCPP_CXX03_LANG template _Tp& unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::operator[](const key_type& __k) @@ -1447,23 +1434,27 @@ unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::operator[](const key_type& __k) __h.release(); return __r.first->second; } +#else -#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES +template +_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 _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 __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 _Tp& diff --git a/test/std/containers/unord/unord.map/unord.map.elem/index.pass.cpp b/test/std/containers/unord/unord.map/unord.map.elem/index.pass.cpp index c072248f8..4757c926f 100644 --- a/test/std/containers/unord/unord.map/unord.map.elem/index.pass.cpp +++ b/test/std/containers/unord/unord.map/unord.map.elem/index.pass.cpp @@ -19,8 +19,11 @@ #include #include +#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 C; typedef std::pair 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, std::equal_to, min_allocator>> 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, std::equal_to, min_allocator>> 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::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::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::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 } diff --git a/test/std/containers/unord/unord.map/unord.map.elem/index_tuple.pass.cpp b/test/std/containers/unord/unord.map/unord.map.elem/index_tuple.pass.cpp index c319b5c30..f2c694e86 100644 --- a/test/std/containers/unord/unord.map/unord.map.elem/index_tuple.pass.cpp +++ b/test/std/containers/unord/unord.map/unord.map.elem/index_tuple.pass.cpp @@ -7,6 +7,8 @@ // //===----------------------------------------------------------------------===// +// UNSUPPORTED: c++98, c++03 + // // template , class Pred = equal_to, @@ -18,9 +20,6 @@ // http://llvm.org/bugs/show_bug.cgi?id=16542 #include - -#ifndef _LIBCPP_HAS_NO_VARIADICS - #include using namespace std; @@ -30,12 +29,8 @@ struct my_hash size_t operator()(const tuple&) const {return 0;} }; -#endif - int main() { -#ifndef _LIBCPP_HAS_NO_VARIADICS unordered_map, size_t, my_hash> m; m[make_tuple(2,3)]=7; -#endif } diff --git a/test/support/container_test_types.h b/test/support/container_test_types.h index 7f6544e9c..54657eb17 100644 --- a/test/support/container_test_types.h +++ b/test/support/container_test_types.h @@ -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) {