[libcxx-commits] [PATCH] D128021: [libc++] Don't call key_eq in unordered_map/set rehashing routine

[Joe Loser via Phabricator via libcxx-commits]

jloser added inline comments.


================
Comment at: libcxx/include/__hash_table:864
+// Since both unordered_set/map and unordered_multiset/multimap 
+// use the same implementation of hash_table we might do reduntand work 
+// for unique keys containers (namely calling key_eq in rehashing).
----------------
Nit: s/reduntand/redundant


================
Comment at: libcxx/include/__hash_table:2284
                         __next_pointer __np = __cp;
-                        for (; __np->__next_ != nullptr &&
-                               key_eq()(__cp->__upcast()->__value_,
-                                        __np->__next_->__upcast()->__value_);
-                                                           __np = __np->__next_)
-                            ;
+			if (!_UniqueKeys::value) {
+                            for (; __np->__next_ != nullptr &&
----------------
Question: what's the libc++ stance for something like this which could be `if constexpr` in C++17 or later? Is it worth writing

```
#if _LIBCPP_STD_VER >= 17
if constexpr
#else
if
#endif
(!UniqueKeys::Value) {
```

I suspect the answer is just writing `if` for all standards modes and just letting the compiler realize it's always a constant value now up-front.


================
Comment at: libcxx/include/__hash_table:868
+
+template <class _Tp, class _Hash, class _Equal, class _Alloc, class _Traits>
+class __hash_table {
----------------
Mordante wrote:
> I'm not fond of name `_Traits` it very generic. How about `_UniqueKeys`?
FWIW, I personally preferred the traits class name holding a (member or type) to denote the one trait of interest: unique_keys. I don't feel too strongly, but I feel it leads to a more open design generally.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128021/new/

https://reviews.llvm.org/D128021