[libc++] Remove tests from ranges.pass.cpp which violate semantic requirements

[philnik]

Summary

This also removes some tests which we have grouped together into robust_from_*.pass.cpp tests.

Specifically, checking that

• ranges::dangling is returned is done in libcxx/test/std/algorithms/ranges_robust_against_dangling.pass.cpp
• std::invoke is used is done in libcxx/test/std/algorithms/ranges_robust_against_omitting_invoke.pass.cpp.
• implicit conversion to bool works is done in libcxx/test/std/algorithms/ranges_robust_against_nonbool_predicates.pass.cpp

Checking the comparison order is invalid because the operator== isn't symmetric.
Checking what the exact type of operator== is, is invalid because comparing the same object has to yield the same results if the objects are not modified.

[philnik]

Rebased

[EricWF]

What semantic requirements do these violate. Please add that information to the description.

[ldionne]

@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.

libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp

155

This one is already tested in libcxx/test/std/algorithms/ranges_robust_against_dangling.pass.cpp.

168

This one is libcxx/test/std/algorithms/ranges_robust_against_omitting_invoke.pass.cpp.

196

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.

210

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.

250

This is libcxx/test/std/algorithms/ranges_robust_against_nonbool_predicates.pass.cpp.

[ldionne]

@EricWF Gentle ping -- does this look good to you with the added justifications? @philnik, let's land this tomorrow unless @EricWF raises additional concerns, otherwise we can address them in a follow-up.

libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp

constexpr void test_iterators() {
     int a[] = {1, 2, 3, 4};
     auto range = std::ranges::subrange(It(a), Sent(It(a + 4)));
     std::same_as<It> auto ret = std::ranges::find(range, 4);
     assert(base(ret) == a + 3);
     assert(*ret == 4);
   }
 }
 
-struct OneWayComparable {
-  bool isLeft;
-  friend constexpr bool operator==(OneWayComparable l, OneWayComparable) { return l.isLeft; }
-};
-
-struct NonConstComparableLValue {
-  friend constexpr bool operator==(const NonConstComparableLValue&, const NonConstComparableLValue&) { return false; }
-  friend constexpr bool operator==(NonConstComparableLValue&, NonConstComparableLValue&) { return false; }
-  friend constexpr bool operator==(const NonConstComparableLValue&, NonConstComparableLValue&) { return false; }
-  friend constexpr bool operator==(NonConstComparableLValue&, const NonConstComparableLValue&) { return true; }
-};
-
-struct NonConstComparableRValue {
-  friend constexpr bool operator==(const NonConstComparableRValue&, const NonConstComparableRValue&) { return false; }
-  friend constexpr bool operator==(const NonConstComparableRValue&&, const NonConstComparableRValue&&) { return false; }
-  friend constexpr bool operator==(NonConstComparableRValue&&, NonConstComparableRValue&&) { return false; }
-  friend constexpr bool operator==(NonConstComparableRValue&&, const NonConstComparableRValue&) { return true; }
-};
-
 constexpr bool test() {
   test_iterators<int*>();
   test_iterators<const int*>();
   test_iterators<cpp20_input_iterator<int*>, sentinel_wrapper<cpp20_input_iterator<int*>>>();
   test_iterators<bidirectional_iterator<int*>>();
   test_iterators<forward_iterator<int*>>();
   test_iterators<random_access_iterator<int*>>();
   test_iterators<contiguous_iterator<int*>>();
test_iterators<contiguous_iterator<int*>>();
     {
       int a[] = {1, 1, 1};
       auto ret = std::ranges::find(a, 0);
       assert(ret == a + 3);
     }
   }
 
   {
-    // check that ranges::dangling is returned
-    [[maybe_unused]] std::same_as<std::ranges::dangling> auto ret =
-      std::ranges::find(std::array{1, 2}, 3);
-  }
-
-  {
     // check that an iterator is returned with a borrowing range
     int a[] = {1, 2, 3, 4};
     std::same_as<int*> auto ret = std::ranges::find(std::views::all(a), 1);
     assert(ret == a);
     assert(*ret == 1);
   }
 
   {
-    // check that std::invoke is used
-    struct S { int i; };
-    S a[] = { S{1}, S{3}, S{2} };
-    std::same_as<S*> auto ret = std::ranges::find(a, 4, &S::i);
-    assert(ret == a + 3);
-  }
-
-  {
     // count invocations of the projection
     {
       int a[] = {1, 2, 3, 4};
       int projection_count = 0;
       auto ret = std::ranges::find(a, a + 4, 2, [&](int i) { ++projection_count; return i; });
       assert(ret == a + 1);
       assert(*ret == 2);
       assert(projection_count == 2);
     }
     {
       int a[] = {1, 2, 3, 4};
       int projection_count = 0;
       auto ret = std::ranges::find(a, 2, [&](int i) { ++projection_count; return i; });
       assert(ret == a + 1);
       assert(*ret == 2);
       assert(projection_count == 2);
     }
   }
 
   {
-    // check comparison order
-    {
-      OneWayComparable a[] = { OneWayComparable{true} };
-      auto ret = std::ranges::find(a, a + 1, OneWayComparable{false});
-      assert(ret == a);
-    }
-    {
-      OneWayComparable a[] = { OneWayComparable{true} };
-      auto ret = std::ranges::find(a, OneWayComparable{false});
-      assert(ret == a);
-    }
-  }
-
-  {
-    // check that the return type of `iter::operator*` doesn't change
-    {
-      NonConstComparableLValue a[] = { NonConstComparableLValue{} };
-      auto ret = std::ranges::find(a, a + 1, NonConstComparableLValue{});
-      assert(ret == a);
-    }
-    {
-      using It = std::move_iterator<NonConstComparableRValue*>;
-      NonConstComparableRValue a[] = { NonConstComparableRValue{} };
-      auto ret = std::ranges::find(It(a), It(a + 1), NonConstComparableRValue{});
-      assert(ret.base() == a);
-    }
-    {
-      NonConstComparableLValue a[] = { NonConstComparableLValue{} };
-      auto ret = std::ranges::find(a, NonConstComparableLValue{});
-      assert(ret == a);
-    }
-    {
-      using It = std::move_iterator<NonConstComparableRValue*>;
-      NonConstComparableRValue a[] = { NonConstComparableRValue{} };
-      auto range = std::ranges::subrange(It(a), It(a + 1));
-      auto ret = std::ranges::find(range, NonConstComparableRValue{});
-      assert(ret.base() == a);
-    }
-  }
-
-  {
     // check that an empty range works
     {
       std::array<int ,0> a = {};
       auto ret = std::ranges::find(a.begin(), a.end(), 1);
       assert(ret == a.begin());
     }
     {
       std::array<int, 0> a = {};
       auto ret = std::ranges::find(a, 1);
       assert(ret == a.begin());
     }
   }
 
-  {
-    // check that the implicit conversion to bool works
-    {
-      StrictComparable<int> a[] = {1, 2, 3, 4};
-      auto ret = std::ranges::find(a, a + 4, StrictComparable<int>{2});
-      assert(ret == a + 1);
-    }
-    {
-      StrictComparable<int> a[] = {1, 2, 3, 4};
-      auto ret = std::ranges::find(a, StrictComparable<int>{2});
-      assert(ret == a + 1);
-    }
-  }
-
   return true;
 }
 
 int main(int, char**) {
   test();
   static_assert(test());
 
   return 0;
 }