[libcxx-commits] [PATCH] D114133: [libc++] Minor fixups in the new introsort code.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

@nilayvaish wrote:

> Can you explain when do we need to use _Comp_ref? Why is adding _Compare sufficient?

See D93562 <https://reviews.llvm.org/D93562>, where I wrote:

> Basically, libc++ algorithms have an invariant that "`_Compare` is either an lvalue reference type or `__debug_less<T&>` (which works like `reference_wrapper` and is cheap to copy)." It would actually be very nice if we could just make the invariant that "`_Compare` is a value type that is cheap to copy (e.g. `reference_wrapper<T>`)," and then we'd never need to specify `<_Compare>` on any helper ever again...

In short, when you have a `_Compare` that was passed to you //by the user//, you need to `_Comp_ref` it before passing it further down. If you're //already// further down, like inside `__introsort`, then `_Compare` was passed to you by a higher-level libc++ function and therefore is safe to use without further `_Comp_ref`'ing.

In D93562 <https://reviews.llvm.org/D93562> I also wrote:

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

I guess I've volunteered now. ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114133