[libcxx-commits] [PATCH] D87171: Implement P0919R3

Fri Oct 9 14:38:33 PDT 2020

zoecarver added inline comments.

================
Comment at: libcxx/include/unordered_map:473
+#if _LIBCPP_STD_VER > 17
+    template <typename _K2>
+    _LIBCPP_INLINE_VISIBILITY
----------------
rarutyun wrote:
> ldionne wrote:
> > Can you use this pattern instead?
> > 
> > ```
> > template <typename _K2, typename = enable_if_t<__is_transparent<_Hash, _K2>::value && __is_transparent<_Pred, _K2>::value>>
> > size_t ...
> > ```
> > 
> > It usually leads to easier-to-read code since the return type isn't hidden at the end of `enable_if_t`.
> > 
> Personally I don't like the approach with dummy argument since it adds possibility to use interface in the wrong way. Furthermore, It doesn't work in all the cases. I definitely can do that for those internal interfaces and I don't see the problems with that. But if you allow me I would prefer to have the `enable_if_t` as the return type for public interfaces (like `find`, `contains`, etc.) to do not have such a dummy parameter on the user level. If prevents incorrect usage and makes code completion hints in IDEs clearer.
Where possible, I suggest using `_EnableIf`. We can optimize that internally to improve compile times. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87171