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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D102006#2755761 <https://reviews.llvm.org/D102006#2755761>, @zoecarver wrote:

>> 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?

AFAIK it's done for all inline functions. (I wouldn't be surprised if there are more functions not having the marker.) If you, for example, look at the `<bit>` header all functions have this marker. Both the public and internal functions.

Of course it would be nice to have a CI job to catch the issues ;-)



================
Comment at: libcxx/include/__ranges/concepts.h:72
+template<class _Tp, class _Up>
+concept __not_same_as = !same_as<remove_cvref_t<_Tp>, remove_cvref_t<_Up>>;
+
----------------
Why not in the namespace?


================
Comment at: libcxx/include/__ranges/subrange.h:63
+
+  template<class _Pair, class _Iter, class _Sent>
+  concept __pair_like_convertible_from =
----------------
zoecarver wrote:
> 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."
Thanks for the information.


================
Comment at: libcxx/include/__ranges/subrange.h:161
+
+    constexpr make_unsigned_t<iter_difference_t<_Iter>> size() const
+      requires (_Kind == subrange_kind::sized)
----------------
zoecarver wrote:
> Mordante wrote:
> > Can you rename this to `__make_unsigned_t`?
> I think this comment got moved. What do you mean?
I had another look and this time a `make-unsigned-like-t` doesn't mean exposition only type but `make_unsigned_t` depending on conditions. So it seems your code is correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102006