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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

LGTM % comments and buildkite failures. (Do you need to use `_LIBCPP_CONSTEXPR` and `_NOEXCEPT` on the definition? is that what buildkite is complaining about? I haven't looked)



================
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));
----------------
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.)


================
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);
 }
----------------
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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102332