[libcxx-commits] [PATCH] D150588: [libc++] Remove tests from ranges.pass.cpp which violate semantic requirements

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision as: ldionne.
ldionne added a comment.

@philnik Please document why those tests are violating constraints or redundant in your commit message, but otherwise this LGTM with the above justifications.

@EricWF Please let us know if you still see a problem with this patch after these explanations.



================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp:155
-    // check that ranges::dangling is returned
-    [[maybe_unused]] std::same_as<std::ranges::dangling> auto ret =
-      std::ranges::find(std::array{1, 2}, 3);
----------------
This one is already tested in `libcxx/test/std/algorithms/ranges_robust_against_dangling.pass.cpp`.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp:168
-  {
-    // check that std::invoke is used
-    struct S { int i; };
----------------
This one is `libcxx/test/std/algorithms/ranges_robust_against_omitting_invoke.pass.cpp`.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp:196
-  {
-    // check comparison order
-    {
----------------
This tried to check that we are calling `*it == value` exactly in the algorithm. However, `equality_comparable_with` and in particular `__WeaklyEqualityComparableWith` (https://en.cppreference.com/w/cpp/concepts/equality_comparable) require that both `t == u` and `u == t` be valid, which means that this is not technically required to work.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp:210
-  {
-    // check that the return type of `iter::operator*` doesn't change
-    {
----------------
My understanding is that this test was trying to check that we don't artificially add a const qualifier to the result of `*it` when we do `*it == value`. However, per https://en.cppreference.com/w/cpp/concepts/equality_comparable:

```
template<class T, class U>
concept __WeaklyEqualityComparableWith = // exposition only
  requires(const std::remove_reference_t<T>& t,
           const std::remove_reference_t<U>& u) {
    { t == u } -> boolean-testable;
    { t != u } -> boolean-testable;
    { u == t } -> boolean-testable;
    { u != t } -> boolean-testable;
  };
```

This always adds a const qualifier to `t` and `u`, so we're allowed to rely on that.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp:250
 
-  {
-    // check that the implicit conversion to bool works
----------------
This is `libcxx/test/std/algorithms/ranges_robust_against_nonbool_predicates.pass.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150588