dexonsmith updated this revision to Diff 50869.
dexonsmith added a comment.

Eric and I had a quick chat on IRC.

- The asymmetric usage of __extract_key and __can_extract_key was awkward, as 
Eric pointed out already.
- However, a symmetric __extract_key would have caused Clang (and other 
compilers) to IRGen extra copies of operator().
- Eric suggested using a custom tag type (instead of true_type/false_type) to 
avoid the problem entirely.

I implemented that in this updated patch, and it's way cleaner.  I'm using:

- __extract_key_fail_tag: cannot extract key.
- __extract_key_self_tag: use the argument itself as the key.
- __extract_key_first_tag: use ".first" on the argument to grab the key.


http://reviews.llvm.org/D16360

Files:
  include/__hash_table
  test/libcxx/containers/unord/unord.map/insert_dup_alloc.pass.cpp
  test/libcxx/containers/unord/unord.set/insert_dup_alloc.pass.cpp

Index: test/libcxx/containers/unord/unord.set/insert_dup_alloc.pass.cpp
===================================================================
--- test/libcxx/containers/unord/unord.set/insert_dup_alloc.pass.cpp
+++ test/libcxx/containers/unord/unord.set/insert_dup_alloc.pass.cpp
@@ -60,6 +60,21 @@
     const ValueTp v(3);
     assert(!c.insert(v).second);
   }
+
+  PRINT("Checking for mallocs in emplace(value_type&&)");
+  assert(!c.emplace(ValueTp(3)).second);
+
+  PRINT("Checking for mallocs in emplace(value_type&)");
+  {
+    ValueTp v(3);
+    assert(!c.emplace(v).second);
+  }
+
+  PRINT("Checking for mallocs in emplace(const value_type&)");
+  {
+    const ValueTp v(3);
+    assert(!c.emplace(v).second);
+  }
 }
 
 int main()
Index: test/libcxx/containers/unord/unord.map/insert_dup_alloc.pass.cpp
===================================================================
--- /dev/null
+++ test/libcxx/containers/unord/unord.map/insert_dup_alloc.pass.cpp
@@ -0,0 +1,103 @@
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// Check that we don't allocate when trying to insert a duplicate value into a
+// unordered_map.
+
+#include <cassert>
+#include <iostream>
+#include <unordered_map>
+
+#include "container_test_types.h"
+#include "count_new.hpp"
+#include "test_macros.h"
+
+#if TEST_STD_VER >= 11
+template <class Arg>
+void PrintInfo(int line, Arg&& arg)
+#else
+template <class Arg>
+void PrintInfo(int line, Arg arg)
+#endif
+{
+  std::cout << "In " << __FILE__ << ":" << line << ":\n    " << arg << "\n" << std::endl;
+}
+#define PRINT(...) PrintInfo(__LINE__, __VA_ARGS__)
+
+template <class Container>
+void testContainerInsert()
+{
+  typedef typename Container::value_type ValueTp;
+  typedef std::pair<typename ValueTp::first_type, typename ValueTp::second_type>
+      PairTp;
+  typedef Container C;
+  typedef std::pair<typename C::iterator, bool> R;
+  ConstructController* cc = getConstructController();
+  cc->reset();
+  Container c;
+  cc->expect<ValueTp&&>();
+
+  PRINT("Expect a malloc in initial insertion");
+  assert(c.insert(ValueTp(3, 4)).second);
+  assert(!cc->unchecked());
+  DisableAllocationGuard g;
+
+  PRINT("Checking for mallocs in insert(value_type&&)");
+  assert(!c.insert(ValueTp(3, 4)).second);
+  assert(!c.insert(PairTp(3, 4)).second);
+
+  PRINT("Checking for mallocs in insert(value_type&)");
+  {
+    ValueTp v(3, 4);
+    assert(!c.insert(v).second);
+  }
+  {
+    PairTp v(3, 4);
+    assert(!c.insert(v).second);
+  }
+
+  PRINT("Checking for mallocs in insert(const value_type&)");
+  {
+    const ValueTp v(3, 4);
+    assert(!c.insert(v).second);
+  }
+  {
+    const PairTp v(3, 4);
+    assert(!c.insert(v).second);
+  }
+
+  PRINT("Checking for mallocs in emplace(value_type&&)");
+  assert(!c.emplace(ValueTp(3, 4)).second);
+  assert(!c.emplace(PairTp(3, 4)).second);
+
+  PRINT("Checking for mallocs in emplace(value_type&)");
+  {
+    ValueTp v(3, 4);
+    assert(!c.emplace(v).second);
+  }
+  {
+    PairTp v(3, 4);
+    assert(!c.emplace(v).second);
+  }
+
+  PRINT("Checking for mallocs in emplace(const value_type&)");
+  {
+    const ValueTp v(3, 4);
+    assert(!c.emplace(v).second);
+  }
+  {
+    const PairTp v(3, 4);
+    assert(!c.emplace(v).second);
+  }
+}
+
+int main()
+{
+  testContainerInsert<TCT::unordered_map<> >();
+}
Index: include/__hash_table
===================================================================
--- include/__hash_table
+++ include/__hash_table
@@ -100,6 +100,21 @@
     return size_t(1) << (std::numeric_limits<size_t>::digits - __clz(__n-1));
 }
 
+struct __extract_key_fail_tag {};
+struct __extract_key_self_tag {};
+struct __extract_key_first_tag {};
+
+template <class _ValTy, class _Key,
+          class _RawValTy = typename __unconstref<_ValTy>::type>
+struct __can_extract_key
+    : conditional<is_same<_RawValTy, _Key>::value, __extract_key_self_tag,
+                  __extract_key_fail_tag>::type {};
+
+template <class _Pair, class _Key, class _First, class _Second>
+struct __can_extract_key<_Pair, _Key, pair<_First, _Second>>
+    : conditional<is_same<typename remove_const<_First>::type, _Key>::value,
+                  __extract_key_first_tag, __extract_key_fail_tag>::type {};
+
 template <class _Tp, class _Hash, class _Equal, class _Alloc> class __hash_table;
 
 template <class _NodePtr>      class _LIBCPP_TYPE_VIS_ONLY __hash_iterator;
@@ -903,6 +918,7 @@
 
     typedef typename _NodeTypes::__node_value_type           __node_value_type;
     typedef typename _NodeTypes::__container_value_type      __container_value_type;
+    typedef typename _NodeTypes::key_type                    key_type;
     typedef value_type&                              reference;
     typedef const value_type&                        const_reference;
     typedef typename __alloc_traits::pointer         pointer;
@@ -1041,13 +1057,49 @@
 
 #ifndef _LIBCPP_CXX03_LANG
     template <class _Key, class ..._Args>
+    _LIBCPP_INLINE_VISIBILITY
     pair<iterator, bool> __emplace_unique_key_args(_Key const& __k, _Args&&... __args);
 
     template <class... _Args>
-    pair<iterator, bool> __emplace_unique(_Args&&... __args);
+    _LIBCPP_INLINE_VISIBILITY
+    pair<iterator, bool> __emplace_unique_impl(_Args&&... __args);
+
+    template <class _Pp>
+    _LIBCPP_INLINE_VISIBILITY
+    pair<iterator, bool> __emplace_unique(_Pp&& __x) {
+      return __emplace_unique_extract_key(_VSTD::forward<_Pp>(__x),
+                                          __can_extract_key<_Pp, key_type>());
+    }
+    template <class... _Args>
+    _LIBCPP_INLINE_VISIBILITY
+    pair<iterator, bool> __emplace_unique(_Args&&... __args) {
+      return __emplace_unique_impl(_VSTD::forward<_Args>(__args)...);
+    }
+
+    template <class _Pp>
+    _LIBCPP_INLINE_VISIBILITY
+    pair<iterator, bool>
+    __emplace_unique_extract_key(_Pp&& __x, __extract_key_fail_tag) {
+      return __emplace_unique_impl(_VSTD::forward<_Pp>(__x));
+    }
+    template <class _Pp>
+    _LIBCPP_INLINE_VISIBILITY
+    pair<iterator, bool>
+    __emplace_unique_extract_key(_Pp&& __x, __extract_key_self_tag) {
+      return __emplace_unique_key_args(__x, _VSTD::forward<_Pp>(__x));
+    }
+    template <class _Pp>
+    _LIBCPP_INLINE_VISIBILITY
+    pair<iterator, bool>
+    __emplace_unique_extract_key(_Pp&& __x, __extract_key_first_tag) {
+      return __emplace_unique_key_args(__x.first, _VSTD::forward<_Pp>(__x));
+    }
+
     template <class... _Args>
+    _LIBCPP_INLINE_VISIBILITY
     iterator __emplace_multi(_Args&&... __args);
     template <class... _Args>
+    _LIBCPP_INLINE_VISIBILITY
     iterator __emplace_hint_multi(const_iterator __p, _Args&&... __args);
 
 
@@ -1989,7 +2041,7 @@
 template <class _Tp, class _Hash, class _Equal, class _Alloc>
 template <class... _Args>
 pair<typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator, bool>
-__hash_table<_Tp, _Hash, _Equal, _Alloc>::__emplace_unique(_Args&&... __args)
+__hash_table<_Tp, _Hash, _Equal, _Alloc>::__emplace_unique_impl(_Args&&... __args)
 {
     __node_holder __h = __construct_node(_VSTD::forward<_Args>(__args)...);
     pair<iterator, bool> __r = __node_insert_unique(__h.get());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to