[libcxx-commits] [PATCH] D129796: [libc++][ranges] implement `std::ranges::equal_range`

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__algorithm/equal_range.h:50
+      return pair<_Iter, _Iter>(
+          std::__lower_bound_impl<_AlgPolicy>(__first, __mid, __value, __comp, __proj),
+          std::__upper_bound<_AlgPolicy, _Compare>(++__mp1, __end, __value, __comp, __proj));
----------------
var-const wrote:
> philnik wrote:
> > var-const wrote:
> > > var-const wrote:
> > > > Does this need to be forwarded, or not?
> > > You probably need to pass `_Compare` explicitly as well, I'm not sure why it wasn't done before (`_Compare` is normally a reference, so type deduction would decay it to a value).
> > You don't have to (and can't really) pass `_Compare` explicitly. `__lower_bound_impl` always takes the comparator by reference instead of by value. I don't know why this hasn't been done this way before, but that makes a lot more sense that explicitly passing the template parameter around everywhere.
> I think it was done that way to support `_Comp_ref` resolving to either a reference type (in a regular build) or a non-reference type (in a debug build). Does that not apply here?
That does apply, but why wouldn't you just pass `_Comp_ref` by reference? The only thing may be that passing it by value is more efficient, but I would expect the functions to be inlined anyways, so it shouldn't matter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129796