[libcxx-commits] [PATCH] D99141: [libcxx] adds `std::incrementable_traits` to <iterator>

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp:31-36
+template <class T, class Expected>
+[[nodiscard]] constexpr bool check_incrementable_traits() noexcept {
+  constexpr bool result = difference_type_matches<T, Expected>;
+  static_assert(difference_type_matches<T const, Expected> == result);
+  return result;
+}
----------------
Take the suggested edit.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp:222
+static_assert(!incrementable_traits_has_difference_type<
+              rvalue_ref_cv_subtraction const volatile>);
+
----------------
My usual complaints about hundreds of lines of tests, which test very little.
Please remove //a large percentage// of lines 94–222, and replace them with:

- One test that fails when line 458's `const _Tp& __x` is replaced with `const _Tp x`. (You currently lack any.)
- One test that fails when line 458's `const _Tp& __x` is replaced with `_Tp& x`. (You currently lack any.)
- One test that fails when line 458's `const _Tp& __x` is replaced with `_Tp x`. (You currently lack any.)


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp:43
+
+static_assert(check_incrementable_traits<int[], std::ptrdiff_t>());
+
----------------
cjdb wrote:
> Quuxplusone wrote:
> > This is surprising, but I guess it falls out naturally because `int[]` is subtractable?
> > Please add tests for `int[10]`, `int(*)()`, `int(&)()`, and `int()` as well.
> See below for `int(*)()` and friends.
Please add `int[10]`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99141