[libcxx-commits] [PATCH] D101193: [libcxx][ranges] Add ranges::empty CPO.

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added a subscriber: tcanens.
cjdb added inline comments.


================
Comment at: libcxx/include/__ranges/empty.h:50-53
+    [[nodiscard]] constexpr bool operator()(_Tp&& __t) const
+        noexcept(noexcept(_VSTD::forward<_Tp>(__t).empty())) {
+      return _VSTD::forward<_Tp>(__t).empty();
+    }
----------------
Quuxplusone wrote:
> As usual, replace `X` with `bool(X)` in the noexcept-spec and also in the return. Also, `__t` should be an lvalue according to "let t be an lvalue that denotes the reified object for E." So:
> ```
> [[nodiscard]] constexpr bool operator()(_Tp&& __t) const
>     noexcept(noexcept(bool(__t.empty()))) {
>   return __t.empty();
> }
> ```
> Optionally, figure out a way to detect the difference here and write a regression test for it. (I say "optionally" because IMHO it doesn't really matter.)
> 
> As above so on lines 34 and 57/58: `t` is an lvalue, lose the forwarding.
> and also in the return

This shouldn't be necessary thanks to //`boolean-testable`//. @tcanens I don't think the wording permit us to use it right now; did we miss one in the cleanup?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101193