[libcxx-commits] [PATCH] D105795: [libcxx][algorithms] adds ranges::lower_bound and ranges::upper_bound

[Tim Song via Phabricator via libcxx-commits]

tcanens added inline comments.


================
Comment at: libcxx/include/__algorithm/lower_bound.h:95
+    _Ip operator()(_Ip __first, _Sp __last, const _Tp& __value, _Comp __comp = {}, _Proj __proj = {}) const {
+      auto __pred = [&__value, &__comp](const auto& __proj_elem){
+        return _VSTD::invoke(__comp, __proj_elem, __value);
----------------
cjdb wrote:
> tcanens wrote:
> > `indirect_strict_weak_order` doesn't guarantee that the projected element is comparable after it's been turned into a const lvalue.
> > 
> > // `boolean-testable` // doesn't guarantee that it can be copied or moved.
> > `indirect_strict_weak_order` doesn't guarantee that the projected element is comparable after it's been turned into a const lvalue.
> 
> If this were your only comment, I'd change it to this.
> ```
> auto __pred = [&__value, &__comp]<class _ProjElem>(_ProjElem&& __proj_elem) {
>   return _VSTD::invoke(__comp, _VSTD::forward<_ProjElem>(__proj_elem), __value);
> };
> ```
> 
> > // `boolean-testable` // doesn't guarantee that it can be copied or moved.
> 
> This gives me pause. Can I rely on perfect forwarding to do the right thing here?

> > // `boolean-testable` // doesn't guarantee that it can be copied or moved.
> 
> This gives me pause. Can I rely on perfect forwarding to do the right thing here?

You can just have `__pred` explicitly return `bool` and let the implicit conversion handle it. There's no real reason to forward the result all the way through.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105795