[PATCH] D16360: unordered_map: Avoid unnecessary mallocs when no insert occurs

[Duncan P. N. Exon Smith via cfe-commits]

Great, I should have time to clean this up tomorrow afternoon.

Regarding emplace, this patch as is has tests for emplace, but
they depend on r258575 being in tree.  You asked me to revert
that... I'll wait for your response over in that thread:
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160118/147795.html

I have a few other questions inline below.

> On 2016-Jan-24, at 19:25, Eric Fiselier <eric at efcs.ca> wrote:
> 
> 
> ================
> 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.

In C++11, this code is dead and untested.  Happy to drop the
conditionals though, it's tested in C++03 so it should be fine.

On second thought, I wonder if it would be possible to skip
this, and use the key directly like in C++11?  I'll take a
look.

> ================
> 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> {};
> ```

Above the unordered_map<> definition a good place for this, or
would it be better to drop it in type_traits or some such?

> We also should handle `pair<...>&` which you previously didn't have. 

I guess that got lost when I switched away from the lazy logic
using decay.  I'll add it back.  Strange that none of the tests
failed... I guess I'll need to find a way to cover that case.