EricWF added a comment.
Alright, so this is looking really good. Once we land this would you be
interested in applying the same optimization to `emplace` calls?
================
Comment at: include/__hash_table:863
@@ -862,2 +862,3 @@
_LIBCPP_INLINE_VISIBILITY
- pair<iterator, bool> __insert_unique_value(_ValueTp&& __x);
+ pair<iterator, bool> __insert_unique_value(_ValueTp&& __x)
+ {return __insert_unique_key_value(__x, std::forward<_ValueTp>(__x));}
----------------
Woops, I know this was my suggestion, but I don't see any reason to have this
function.
Please remove `__insert_unique_value`. Callers can call
`__insert_unique_key_value` directly.
================
Comment at: include/__hash_table:875
@@ -867,1 +874,3 @@
+ _LIBCPP_INLINE_VISIBILITY
+ pair<iterator, bool> __insert_unique_key_value(const _ValueTp& __key,
const _ValueTp& __x);
#endif
----------------
Please use the following signature for this overload:
```
template <class _KeyLike, class _ValueTp>
pair<iterator, bool> __insert_unique_key_value(const _KeyTp& __key, const
_ValueTp& __x);
```
Using `_ValueTp` twice in C++03, but `_KeyTp` in C++11 is confusing.
I understand that in C++03 `_KeyTp` should always be `_ValueTp` but it took me
10 minutes to realize that.
================
Comment at: include/unordered_map:401
@@ -400,1 +400,3 @@
{return static_cast<const _Hash&>(*this)(__x.__cc.first);}
+#if defined(_LIBCPP_HAS_NO_RVALUE_REFERENCES)
+ _LIBCPP_INLINE_VISIBILITY
----------------
Is there a reason your hiding these overloads in C++11 and beyond?
I would prefer as few conditional compilation blocks as possible.
================
Comment at: include/unordered_map:1004
@@ +1003,3 @@
+ _LIBCPP_INLINE_VISIBILITY
+ pair<iterator, bool> __insert_extract_key_if_pair(pair<_FirstTp,
_SecondTp>&& __x)
+ {return __insert_extract_key<_FirstTp>(_VSTD::move(__x));}
----------------
Overall the dispatch looks good, but it could be a lot simpler. Instead of
multi-level dispatch I would write a single trait to computer if "_Pp" is a
keylike pair. Then `insert(_Pp&&)` just use that to call
`__insert_extract_key_if_referenceable` directly using that trait.
For example:
```
template <class _Key, class _Pair, class _RawPair = typename
__uncvref<_RawPair>::type>
struct __is_keylike_pair : false_type {};
template <class _Key, class _Pair, class _First, class _Second>
struct __is_keylike_pair<_Key, _Pair, pair<_First, _Second> > :
is_same<typename remove_const<_First>::type, _Key> {};
```
We also should handle `pair<...>&` which you previously didn't have.
http://reviews.llvm.org/D16360
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
