[libcxx-commits] [PATCH] D100080: [libcxx][iterator] adds `std::weakly_incrementable` and `std::incrementable`

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

This LGTM generally speaking, but I'd like to understand the story for signed-integer-class types.



================
Comment at: libcxx/include/__iterator/concepts.h:62
+template<class _Tp>
+inline constexpr bool __is_signed_integer_like = signed_integral<_Tp>;
+
----------------
>From http://eel.is/c++draft/iterator.concept.winc#11:

> An integer-like type `I` is signed-integer-like if it models `signed_­integral<I>` or if it is a signed-integer-class type.

Here, we detect if it models `signed_­integral`, but not if it's a signed-integer-class type. Reading the description of what a signed-integer-class type is (starting at http://eel.is/c++draft/iterator.concept.winc#2), it's not clear to me how to even check for that. Do you know what's the intent of the standard here?


================
Comment at: libcxx/include/iterator:59
+
+// [iterator.concept.inc], concept incrementable
+template<class I>
----------------
I think those comments could be dropped since they don't add much IMO. Feel free to leave in this patch and we'll address that as a NFC on the whole file later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100080