This is an archive of the discontinued LLVM Phabricator instance.

Implement P1209 - Adopt Consistent Container Erasure from Library Fundamentals 2 for C++20
ClosedPublic

Authored by mclow.lists on Dec 10 2018, 4:07 PM.

Details

Reviewers
EricWF
ldionne
Summary

Implement container-based erase and erase_if for all the containers.
https://wg21.link/P1209

Lots of similar changes, over and over.

Diff Detail

Event Timeline

mclow.lists created this revision.Dec 10 2018, 4:07 PM

Could you please split this change and the changes from class to struct into different patches? That would make it easier to review.

Could you please split this change and the changes from class to struct into different patches? That would make it easier to review.

Yes, sorry.

Removed the tuple_size changes - those are in D55466.

mclow.lists marked an inline comment as done.Dec 10 2018, 4:50 PM
mclow.lists added inline comments.
include/version
40

Tabs. I thought I got rid of them.

ldionne added inline comments.Dec 13 2018, 10:49 AM
include/deque
2942

I see that's what the specification says, but is the unqualified call intended? For a user-defined type, I think this could pick up a custom remove_if through ADL since our deque iterators have the _Tp as a template parameter. Either I'm wrong, or this seems like an undesirable thing.

include/unordered_set
948

There is a lot of duplication across associative containers -- would it make sense to factor this?

mclow.lists marked 2 inline comments as done.Dec 13 2018, 11:41 AM
mclow.lists added inline comments.
include/deque
2942

I agree that it could pick up a user-defined remove_if, (or actually, remove as well).

include/unordered_set
948

I agree - but they're in four different (unrelated) headers: set/map/unordered_set/unordered_map. I could cut them in half (down from eight to four), but getting down to one would require finding a common (unrelated) header file.

Hrm. They all include <functional>

  • Make a common routine for the associative and unordered associative containers.
  • Add _VSTD:: to prohibit ADL.
  • Add inline _LIBCPP_INLINE_VISIBILITY in a bunch of spots.
mclow.lists marked 2 inline comments as done.Dec 13 2018, 9:09 PM
ldionne accepted this revision.Dec 14 2018, 8:06 AM
This revision is now accepted and ready to land.Dec 14 2018, 8:06 AM
mclow.lists closed this revision.Dec 14 2018, 10:52 AM

Committed as revision 349178