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

Mark de Wever via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed May 5 10:45:44 PDT 2021

Mordante added a comment.

I like the `Implements [range.iter.op.advance]` part in the description, makes finding the documentation a lot easier!
Is the synopsis up to date?

Comment at: libcxx/include/__iterator/functionish.h:31
+// Since these are still standard library functions, we use `__functionish` to eliminate most of
+// the properties that function objects get by default (e.g. semiregularity, addressability), to
Can we use a clearer name, for example `__function_like`, `ish` sounds rather fishy to me. Or maybe something with ADL in the name, but I've no nice name for that to offer.

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;
Can't we return the unsigned version of `_Tp`? Then  we can return the largest negative value.

Comment at: libcxx/include/__iterator/primitives.h:56
+      return;
+    } else {
+      // Otherwise, if `n` is non-negative, increments `i` by `n`.
Can you remove the `else`?

Comment at: libcxx/include/__iterator/primitives.h:112
+      return 0;
+    } else {
+      // Otherwise, if `n` is non-negative, while `bool(i != bound)` is true, increments `i` but at
Can you remove the `else`?

Comment at: libcxx/include/__iterator/primitives.h:129
+    }
+  }
If we violate the preconditions we leave the function without returning a value, resulting in UB. Can you add `_LIBCPP_UNREACHABLE()` instead.

Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/advance.pass.cpp:15
+#include <iterator>
Please use
// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DEBUG=1                                  
// UNSUPPORTED: libcxx-no-debug-mode                                            
I wonder whether we still need to use this at all now we have a CI build with debug mode enabled. I will probably remove them from my format patches.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list