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

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

> On 2016-Mar-16, at 12:20, Eric Fiselier <eric at efcs.ca> wrote:
> 
> EricWF added a comment.
> 
> Adding inline comments for the implementation. Comments on the tests to follow shortly.
> 
> 
> ================
> Comment at: include/__hash_table:103
> @@ -102,1 +102,3 @@
> 
> +template <class _ValTy, class _Key>
> +struct __extract_key;
> ----------------
> Could you make `__extract_key` behave the same way as `__can_extract_key` where we apply the `__uncvref<ValTy>` instead of expecting the user to? 

I started with that, but it seemed to require many more
explicit specializations of `__extract_key`.  It's simpler to
handle all the possibilities via overloading.

I can have another look and see if I can find something that
seems more symmetric, but I don't think moving the __uncref
inside __extract_key is the right choice.

> ================
> Comment at: include/__hash_table:110
> @@ +109,3 @@
> +    : is_same<
> +          typename remove_const<typename remove_reference<_ValTy>::type>::type,
> +          _Key> {};
> ----------------
> Assuming we can't be passed volatile types (I'll double check that). Then we should just use `is_same<_RawValTy, _Key>`

I think we can be passed volatile types.  `emplace` forwards all
arguments, and the underlying type may have constructors from
volatile types.

> ================
> Comment at: include/__hash_table:113
> @@ +112,3 @@
> +template <class _Key>
> +struct __extract_key<_Key, _Key> {
> +  const _Key &operator()(const _Key &__key) { return __key; }
> ----------------
> Please keep the `__can_extract_key` and `__extract_key`  specializations together. \

I'm fine either way, but I thought it would scale better (as we
add more of these) to have each `__can_extract_key` paired with
its corresponding `__extract_key`.