[libcxx-commits] [PATCH] D100369: [libc++] [P0458] Add map::contains and set::contains for heterogenous lookup missed in a17b1aed.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

LGTM % whitespace nits.



================
Comment at: libcxx/include/map:199-200
+
+    bool           contains(const key_type& x) const; // C++20
+    template<class K> bool contains(const K& x) const; // C++20
+
----------------
Nit: add an extra space before `// C++20` on line 199 so that all the comments line up. Ditto line 418.


================
Comment at: libcxx/include/map:1419
+    _LIBCPP_INLINE_VISIBILITY
+        typename enable_if<__is_transparent<_Compare, _K2>::value, bool>::type
+        contains(const _K2& __k) const {
----------------
curdeius wrote:
> Hmmm, just looking at other files and wondering. Should I use `_EnableIf`?
FWIW I'd say yes, but throughout; so in a separate PR if at all. See for example 199d2ebeed8382071809983f016e482b1d013b62 where I went through and `_EnableIf`'ed all the deduction guides. We could do the same kind of NFC-commit here. Or we could just say "meh, nvm for now."


================
Comment at: libcxx/include/set:162-163
+
+    bool           contains(const key_type& x) const; // C++20
+    template<class K> bool contains(const K& x) const; // C++20
+
----------------
Line 162 is missing a space before `contains`, if you were trying to get the semicolons to line up. (I care about aligning the comments; I don't care about aligning the semicolons.) Ditto line 367.


================
Comment at: libcxx/include/set:809-814
+    template <typename _K2>
+    _LIBCPP_INLINE_VISIBILITY
+        typename enable_if<__is_transparent<_Compare, _K2>::value, bool>::type
+        contains(const _K2& __k) const {
+      return find(__k) != end();
+    }
----------------
Here and throughout, it would be more //consistent// with the surrounding style to do the function on one line:
```
    template <typename _K2>
    _LIBCPP_INLINE_VISIBILITY
    typename enable_if<__is_transparent<_Compare, _K2>::value,bool>::type
    contains(const _K2& __k) const                 {return find(__k) != end();}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100369