[libcxx-commits] [PATCH] D102332: [libc++][nfc] remove duplicated __to_unsigned.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 3 inline comments as done.
Mordante added a comment.

Thanks for the reviews! It turns out our `<type_traits>` header is available in C++98. So I'll guard against this version.



================
Comment at: libcxx/include/__ranges/size.h:92
         noexcept(noexcept(ranges::end(__t) - ranges::begin(__t))) {
-      return __to_unsigned_like<range_difference_t<remove_cvref_t<_Tp>>>(
+      return __to_unsigned<range_difference_t<remove_cvref_t<_Tp>>>(
           ranges::end(__t) - ranges::begin(__t));
----------------
Quuxplusone wrote:
> IMO, please `_VSTD::` this. (Yeah, its function parameter is a primitive type; but the fact that it //has// a function parameter is sufficient cause for me.)
I think it's indeed a bit pedantic, but we don't know the exact type of `_Tp`. In `<charconv>` I feel it's constrained so there it seems overkill.


================
Comment at: libcxx/include/type_traits:2314-2318
+template <class _Tp>
+_LIBCPP_NODISCARD_ATTRIBUTE _LIBCPP_INLINE_VISIBILITY constexpr
+typename make_unsigned<_Tp>::type __to_unsigned(_Tp __x) noexcept {
+    return static_cast<typename make_unsigned<_Tp>::type>(__x);
 }
----------------
cjdb wrote:
> Quuxplusone wrote:
> > Re naming: I approve of `__to_unsigned`. It simply converts to the `make_unsigned` version of the type, no more and no less.
> > @cjdb, if libc++ ever adds support for user-defined "unsigned-like but not unsigned" types, we can revisit the naming bikeshed (since we'll have to re-audit all the call-sites anyway, it'll actually be //good// to force the pull-requester to touch them all).
> If this were the case, then @ldionne wouldn't have asked for the concept `__integer_like`. At the very least, I think we should wait for Louis to chime in on this discussion.
I dislike the `_like` suffix. But it seems ranges uses this suffix a lot. So for that reason I think adding the `_like` suffix is a better idea. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102332