[libcxx-commits] [PATCH] D107396: [libcxx][ranges] Add `ranges::iota_view`.

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/include/__ranges/iota_view.h:257-260
+          if (__y.__value_ > __x.__value_) {
+            return difference_type(-difference_type(__y.__value_ - __x.__value_));
+          }
+          return difference_type(__x.__value_ - __y.__value_);
----------------



================
Comment at: libcxx/include/__ranges/iota_view.h:355-361
+        if (__value_ < 0) {
+          if (__bound_ < 0) {
+            return _VSTD::__to_unsigned_like(-__value_) - _VSTD::__to_unsigned_like(-__bound_);
+          }
+          return _VSTD::__to_unsigned_like(__bound_) + _VSTD::__to_unsigned_like(-__value_);
+        }
+        return _VSTD::__to_unsigned_like(__bound_) - _VSTD::__to_unsigned_like(__value_);
----------------
Yes, I inverted the first condition and moved it to the top.


================
Comment at: libcxx/include/__ranges/iota_view.h:318-320
+      if constexpr ((integral<_Start> || is_pointer_v<_Start>) &&
+                    (integral<_Bound> || is_pointer_v<_Bound>)) {
+        _LIBCPP_ASSERT(ranges::less_equal()(__value_, __bound_),
----------------
ldionne wrote:
> That would match the spec better.
This suggestion is too general: we're deliberately restricting this to integers and pointers because general reachability is not detectable.


================
Comment at: libcxx/test/std/ranges/range.factories/range.iota.view/begin.pass.cpp:45-46
+  testType<SomeInt>();
+  testType<signed long>();
+  testType<unsigned long>();
+  testType<int>();
----------------
zoecarver wrote:
> Quuxplusone wrote:
> > I notice a conspicuous lack of `long long` and `unsigned long long` here.
> > Notice that `iota_view<long long>::difference_type` needs to be `__int128_t` or some other integer-like class type suitable for holding the signed difference between `LLONG_MIN` and `LLONG_MAX`.
> > Notice that iota_view<long long>::difference_type needs to be __int128_t or some other integer-like class type suitable for holding the signed difference between LLONG_MIN and LLONG_MAX.
> 
> Is that standard? Are we supposed to support types larger than long? That is why I dropped it from this test. (I should have made a comment, though). 
> Are we supposed to support types larger than long?

Yes, this is //a// reason why //`integer-like`// exists.


================
Comment at: libcxx/test/std/ranges/range.factories/range.iota.view/iterator/compare.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
zoecarver wrote:
> cjdb wrote:
> > Please add commented-out tests for `<=>`, or a FIXME, or both. We also still need to test `!=` is `not (x == y)`.
> This is not testing `==` or `!=`. That's in eq.pass.cpp. 
I count four assertions for an `==`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107396