[libcxx-commits] [PATCH] D101922: [libcxx][iterator] adds `std::ranges::advance`

Mark de Wever via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu May 6 12:00:38 PDT 2021

Mordante accepted this revision as: Mordante.
Mordante added a comment.

LGTM modulo some small nits.

Comment at: libcxx/include/__function_like.h:10
Please also rename the include guard.

Comment at: libcxx/include/__iterator/primitives.h:41
+    auto const __unsigned_n = static_cast<make_unsigned_t<_Tp> >(__n);
+    return __n < 0 ? -__unsigned_n : __unsigned_n;
+  }
I'm not too fond of negating unsigned values. How do you feel about method/approach used in `<charconv>`?
Here the code I'm referring to, mainly `__complement`, but some added context.
template <typename _Tp>
__complement(_Tp __x)
    static_assert(is_unsigned<_Tp>::value, "cast to unsigned first");
    return _Tp(~__x + 1);

template <typename _Tp>
inline _LIBCPP_INLINE_VISIBILITY typename make_unsigned<_Tp>::type
__to_unsigned(_Tp __x)
    return static_cast<typename make_unsigned<_Tp>::type>(__x);

template <typename _Tp>
inline _LIBCPP_INLINE_VISIBILITY to_chars_result
__to_chars_itoa(char* __first, char* __last, _Tp __value, true_type)
    auto __x = __to_unsigned(__value);
    if (__value < 0 && __first != __last)
        *__first++ = '-';
        __x = __complement(__x);

    return __to_chars_itoa(__first, __last, __x, false_type());

Comment at: libcxx/include/__iterator/primitives.h:39
+  [[nodiscard]] static constexpr auto __abs(_Tp const __n) noexcept {
+    _LIBCPP_ASSERT(__n != numeric_limits<_Tp>::min(), "`-numeric_limits<I>::min()` is undefined");
+    return __n < 0 ? -__n : __n;
cjdb wrote:
> Mordante wrote:
> > Can't we return the unsigned version of `_Tp`? Then  we can return the largest negative value.
> I suppose we could. I don't like treating unsigned ints as "non-negative whole numbers", but I don't think we've got a choice here.
In general I also dislike this way to treat unsigned values. But I think it's better than disallowing one value.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list