[libcxx-commits] [PATCH] D115272: [libc++] [test] Refactor range.prim/empty.pass.cpp

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

FWIW, it's usually easier to instead create three differential revisions and link them in the UI as parent/children.



================
Comment at: libcxx/test/std/ranges/range.access/range.prim/empty.pass.cpp:101
 struct BeginEndNotSizedSentinel {
-  friend constexpr forward_iterator<int*> begin(BeginEndNotSizedSentinel) { return {}; }
-  friend constexpr sentinel end(BeginEndNotSizedSentinel) { return {}; }
----------------
Was `forward_iterator` not relevant for test coverage? I guess we were only relying on the fact that it was not a `sized_range`?


================
Comment at: libcxx/test/std/ranges/range.access/range.prim/empty.pass.cpp:106-114
-struct InvalidMinusBeginEnd {
-  friend constexpr random_access_iterator<int*> begin(InvalidMinusBeginEnd) { return {}; }
-  friend constexpr sentinel end(InvalidMinusBeginEnd) { return {}; }
-};
-
-// Int is integer-like, but it is not other_forward_iterator's difference_type.
-constexpr short operator-(sentinel, random_access_iterator<int*>) { return 2; }
----------------
I don't understand why this removal doesn't reduce test coverage.


================
Comment at: libcxx/test/std/ranges/range.access/range.prim/empty.pass.cpp:142-146
-struct BeginEndAndConstEmpty {
-  int* begin();
-  int* end();
-  constexpr bool empty() const { return true; }
 };
----------------
How does this not reduce coverage? Now we don't test with a `empty() const` method anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115272