[libcxx-commits] [PATCH] D102789: [libcxx][iterator] adds `std::ranges::distance`

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__iterator/distance.h:40-45
+    auto __distance = iter_difference_t<_Ip>(0);
+    for (; __first != __last; ++__first) {
+      ++__distance;
+    }
+
+    return __distance;
----------------
Please put lines 40-45 in an `else` block, so that we don't instantiate them unnecessarily in the case that the `if` block is taken.


================
Comment at: libcxx/include/__iterator/distance.h:48-55
+  template <range _Rp>
+  [[nodiscard]] constexpr range_difference_t<_Rp> operator()(_Rp&& __r) const {
+    if constexpr (sized_range<_Rp>) {
+      return static_cast<range_difference_t<_Rp> >(ranges::size(__r));
+    } else {
+      return (*this)(ranges::begin(__r), ranges::end(__r));
+    }
----------------
I notice we're not doing the `noexcept` thing here. Is that because https://eel.is/c++draft/range.iter.op.distance#3 says merely "equivalent to" instead of the term of power "expression-equivalent to"?


================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.distance/distance.verify.cpp:17
+
+namespace std::ranges {
+class forward_iterator {
----------------
I don't think you're (that is, I don't think the programmer is) allowed to add your own types to `namespace std::ranges`.


================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.distance/distance.verify.cpp:39-46
+// The function templates defined in [range.iter.ops] are not found by argument-dependent name lookup ([basic.lookup.argdep]).
+void no_adl_participation() {
+  std::ranges::forward_iterator i;
+  distance(i, i); // expected-error {{use of undeclared identifier 'distance'}}
+
+  std::ranges::some_range r;
+  distance(r); // expected-error {{use of undeclared identifier 'distance'}}
----------------
We could do this in a .pass.cpp by SFINAE-testing the well-formedness of `distance(r)`, instead of relying on the exact wording of the compiler error and then having to skip the test for non-Clang compilers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102789