[libcxx-commits] [PATCH] D93562: [libc++] Add a missing `<_Compare>` template argument.

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Dec 23 08:59:04 PST 2020

Quuxplusone added inline comments.

Comment at: libcxx/include/algorithm:4500
@curdeius wrote:
> Hmm, just one thought, it could be tested using non-copyable comparators passed by (non-reduced) lvalue ref, nope? How did you find this "missed optimization"?

I believe comparators must be copyable by definition (or else it's library UB). I also believe it's unspecified how many times libc++ may copy them. //Traditionally// the STL copies comparators as liberally as it copies iterators or allocators; libc++'s reference dance is just for QoI. Arguably, any programmer who cares should be passing `std::ref(myComparator)` in the first place, instead of passing a heavyweight `myComparator` by value.

We could still add a QoI test for this in `libcxx/test/libcxx/`; leave the comparator copyable but have it count how many times it was copied, and then assert that the number of copies was 1. I'm not volunteering.

How did I find it? Just staring at the code. I've been staring at these helper-function calls a lot lately, because of constexpr algorithms and also because of ADL.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list