[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
Sun Dec 20 15:06:11 PST 2020

Quuxplusone added inline comments.

Comment at: libcxx/include/algorithm:4487
+        _VSTD::__half_inplace_merge<_Compare>(__buff, __p, __middle, __last, __first, __comp);
> Excuse my ignorance, but could you please explain why letting `_Compare` get deduced is wrong/suboptimal?
> IIUC the case you mention is when e.g. `__buffered_inplace_merge` has `_Compare` being some non-ref type `T`, so then calling `__half_inplace_merge` will deduce its `_Compare` to be an lvalue ref `T &`. So far so good. But why/when is it bad?

Other way around. (And I'm sure it's not your ignorance, just a brain fart. ;)) The trouble case is when `_Compare` is `T&`, and then passing `__comp` by value will make `__half_inplace_merge` deduce //its// `_Compare` as `T` and make a copy, when really we want `__half_inplace_merge`'s `_Compare` to also be `T&`.

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

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list