[libcxx-commits] [PATCH] D102006: [libcxx][ranges] Add range.subrange.

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added a comment.

> inline function -> _LIBCPP_INLINE_VISIBILITY

I thought this was only for internal functions, because public functions have stable pre/post conditions, is that not correct?



================
Comment at: libcxx/include/__ranges/subrange.h:40
+  template<class _Range>
+  concept sized_range = range<_Range> && requires(_Range& t) {
+    ranges::size(t);
----------------
Mordante wrote:
> Please __uglify t.
> Minor nit, since you use `_Range` instead of `_Tp`, how do you feel about using `__r` instead of `__t`?
Good idea about using `__r`. I should have documented this more clearly, but I'm going to "properly" add these concepts in a future patch. I just didn't realize they were a prerequisite until too late here, so I just copied them in. 


================
Comment at: libcxx/include/__ranges/subrange.h:63
+
+  template<class _Pair, class _Iter, class _Sent>
+  concept __pair_like_convertible_from =
----------------
Mordante wrote:
> Just curious, why these names instead of `_Tp`, `_Up`, and `_Vp`?
I think it's a bit clearer, then you don't have to map `T` to the caller and try to figure out which argument it is, you can just say, "oh that's supposed to be pair-like."


================
Comment at: libcxx/include/__ranges/subrange.h:161
+
+    constexpr make_unsigned_t<iter_difference_t<_Iter>> size() const
+      requires (_Kind == subrange_kind::sized)
----------------
Mordante wrote:
> Can you rename this to `__make_unsigned_t`?
I think this comment got moved. What do you mean?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102006