[libcxx-commits] [PATCH] D93562: [libc++] Add a missing `<_Compare>` template argument.
Marek Kurdej via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Dec 21 01:00:18 PST 2020
curdeius accepted this revision.
curdeius added a comment.
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...
> Other way around. (And I'm sure it's not your ignorance, just a brain fart. ;))
Indeed, I somehow forcingly made myself think that the rule "3) If P is a reference type, the type referred to by P is used for deduction." for argument deduction is non existent.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits