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

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius marked an inline comment as done.
curdeius added a comment.

I'll take care of whitespace changes on the next update/landing.
Thanks for the review.



================
Comment at: libcxx/include/map:1419
+    _LIBCPP_INLINE_VISIBILITY
+        typename enable_if<__is_transparent<_Compare, _K2>::value, bool>::type
+        contains(const _K2& __k) const {
----------------
Quuxplusone wrote:
> 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."
OK. Noted. I'll do it separately.


================
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
+
----------------
Quuxplusone wrote:
> 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.
The idea was just to put the same text as in the standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100369