<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 16, 2016 at 1:31 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> On 2016-Mar-16, at 12:20, Eric Fiselier <<a href="mailto:eric@efcs.ca">eric@efcs.ca</a>> wrote:<br>
><br>
> EricWF added a comment.<br>
><br>
> Adding inline comments for the implementation. Comments on the tests to follow shortly.<br>
><br>
><br>
> ================<br>
> Comment at: include/__hash_table:103<br>
> @@ -102,1 +102,3 @@<br>
><br>
> +template <class _ValTy, class _Key><br>
> +struct __extract_key;<br>
> ----------------<br>
> 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?<br>
<br>
</span>I started with that, but it seemed to require many more<br>
explicit specializations of `__extract_key`.  It's simpler to<br>
handle all the possibilities via overloading.<br></blockquote><div><br></div><div>It shouldn't actually take any additional overloads. Heres the implementation I'm hinting at. </div><div><br></div><div>template <class _RawValTy, class _Key, class _ValTy = typename __unconstref<_RawValTy>::type></div><div>struct __extract_key;</div><div><br></div><div>template <class _RawValTy, class _Key></div><div>struct __extract_key<_RawValTy, _Key, _Key>; // specialization for set</div><div><br></div><div>template <class _RawValTy, class _Key, class _Second></div><div>struct __extract_key<_RawValTy, _Key, pair<_Key, _Second>>; // map specialization for non-const key.</div><div><br></div><div>template <class _RawValTy, class _Key, class _Second></div><div>struct __extract_key<_RawValTy, _Key, pair<const _Key, _Second>>; // map specialization for const key.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I can have another look and see if I can find something that<br>
seems more symmetric, but I don't think moving the __uncref<br>
inside __extract_key is the right choice.<br>
<span class=""><br>
> ================<br>
> Comment at: include/__hash_table:110<br>
> @@ +109,3 @@<br>
> +    : is_same<<br>
> +          typename remove_const<typename remove_reference<_ValTy>::type>::type,<br>
> +          _Key> {};<br>
> ----------------<br>
> Assuming we can't be passed volatile types (I'll double check that). Then we should just use `is_same<_RawValTy, _Key>`<br>
<br>
</span>I think we can be passed volatile types.  `emplace` forwards all<br>
arguments, and the underlying type may have constructors from<br>
volatile types.<br></blockquote><div><br></div><div>Yeah, I double checked and your right.</div><div>I'll check in a "__unconstref" template within the hour. We should use that instead of "__uncvref".</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> ================<br>
> Comment at: include/__hash_table:113<br>
> @@ +112,3 @@<br>
> +template <class _Key><br>
> +struct __extract_key<_Key, _Key> {<br>
> +  const _Key &operator()(const _Key &__key) { return __key; }<br>
> ----------------<br>
</span>> Please keep the `__can_extract_key` and `__extract_key`  specializations together. \<br>
<br>
I'm fine either way, but I thought it would scale better (as we<br>
add more of these) to have each `__can_extract_key` paired with<br>
its corresponding `__extract_key`.<br></blockquote><div><br></div><div>I disagree only because I had a hard time reading the fractured specializations while reviewing this.</div><div> </div></div><br></div></div>