This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Simple contains for associative containers
AbandonedPublic

Authored by jloser on Apr 3 2019, 8:52 PM.

Details

Summary

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0458r2.html

This patch adds partial support for p0458, adding a contains member function
for associative containers. This feature allows users to query whether
a key type exists in an associative container.

Note:

  • This patch does not add support for heterogeneous key overloads for contains. A future patch will follow for support this. Currently, the unordered associative containers do not yet have a notion of transparent lookups.
  • This patch does not add support for contains with precomputed hashes. A future patch will follow for support for this.

Diff Detail

Repository
rCXX libc++

Event Timeline

jloser created this revision.Apr 3 2019, 8:52 PM

How does this compare to D59344 ?

jloser added a comment.Apr 4 2019, 7:13 AM

Sorry, I did not notice https://reviews.llvm.org/D59344 was in flight. That one looks like a superset of these changes, though I think we should only provide the declaration/definitions of contains in the case of #if _LIBCPP_STD_VER > 17. I will post a comment to that effect in https://reviews.llvm.org/D59344.

Going to abandon these changes since it's effectively a duplicate.

jloser abandoned this revision.Apr 4 2019, 7:14 AM

@jloser Sorry about the duplication of code here. As for custom hash support, I already have a partial implementation of that just FYI. If you already have written one though, just let me know and I will abandon mine.

jloser added a comment.Apr 5 2019, 5:06 PM

@zoecarver no worries; no I have not written one. Why do we need a custom hash implementation? We should be able to support contains with precomputed hashes without a custom hasher.

I was talking about P0920R2 (nothing really to do with contains). There is another patch I am waiting to be accepted before I put the finishing touches on it. I thought you mentioned it in the details, so I wanted to make sure we didn't make duplicated patches again :)

jloser added a comment.Apr 5 2019, 7:41 PM

Ah right. Yeah, the commit does mention how this patch didn't touch the precalculated hash lookups at all. Now your comment makes sense. :)