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

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/include/__ranges/empty.h:38
+  template<class _Tp>
+  concept __can_invoke_size = requires(_Tp&& __t) { ranges::size(__t); };
+
----------------
ldionne wrote:
> tcanens wrote:
> > I'd expect this one and the next to require `!__member_empty`?
> I think the expression we check should be `ranges::size(_VSTD::forward<_Tp>(__t)) == 0` instead, according to the spec.
> 
> I'm actually not sure why the spec mandates checking `== 0` since `ranges::size(E)` is always supposed to be integer-like, but oh well.
@tcanens you're right. @ldionne I don't think we need the `== 0` because the return type is `integral`. But you might be right about forwarding the expression. I know `__t` is required to be an lvalue, so that's why I didn't have it originally. I guess it comes down to whether or not a copy is going to happen when we call `size`, and I don't think we can guarantee that won't happen, so I think maybe we should forward it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101193