This is an archive of the discontinued LLVM Phabricator instance.

[libc++] [P0458] Add map::contains and set::contains for heterogenous lookup missed in a17b1aed.
ClosedPublic

Authored by curdeius on Apr 13 2021, 1:39 AM.

Details

Summary

Commit rGa17b1aed added bool contains(const key_type& x) const; methods to associative containers, but didn't add template<class K> bool contains(const K& x) const; for heterogenous lookup.

Diff Detail

Event Timeline

curdeius requested review of this revision.Apr 13 2021, 1:39 AM
curdeius created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2021, 1:39 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
curdeius edited the summary of this revision. (Show Details)Apr 13 2021, 1:41 AM
curdeius edited the summary of this revision. (Show Details)
curdeius added inline comments.Apr 13 2021, 1:46 AM
libcxx/include/map
1419

Hmmm, just looking at other files and wondering. Should I use _EnableIf?

Quuxplusone added a subscriber: Quuxplusone.

LGTM % whitespace nits.

libcxx/include/map
199–200

Nit: add an extra space before // C++20 on line 199 so that all the comments line up. Ditto line 418.

1419

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."

libcxx/include/set
162–163

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.

809–814

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();}
curdeius marked an inline comment as done.Apr 13 2021, 6:51 AM

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

libcxx/include/map
1419

OK. Noted. I'll do it separately.

libcxx/include/set
162–163

The idea was just to put the same text as in the standard.

ldionne accepted this revision.Apr 13 2021, 7:28 AM
This revision is now accepted and ready to land.Apr 13 2021, 7:28 AM