[libc++][ranges] Add implicit conversion to bool test for ranges::find{, if, if_not}

[ldionne]

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

225–227

WDYT? IMO this clarifies exactly what we're testing.

libcxx/test/support/immobile_bool.h

1 ↗

(On Diff #416515)

How about BooleanTestable, since this is roughly an archetype for being boolean-testable?

[ldionne]

libcxx/test/support/immobile_bool.h

1 ↗

(On Diff #416515)

Actually, here's what I came up with after discussing with you offline and playing around with the patch:

//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef LIBCXX_TEST_SUPPORT_BOOLEAN_TESTABLE_H
#define LIBCXX_TEST_SUPPORT_BOOLEAN_TESTABLE_H

struct BooleanTestable {
  constexpr operator bool() {
    return value_;
  }

  constexpr BooleanTestable(bool value) : value_{value} {}
  BooleanTestable(const BooleanTestable&) = delete;
  BooleanTestable(BooleanTestable&&) = delete;

private:
  bool value_;
};

template <typename T>
struct StrictComparable {
  constexpr StrictComparable(T value) : value_(value) { }

  friend constexpr BooleanTestable operator==(StrictComparable const& a, StrictComparable const& b) {
    return BooleanTestable(a.value_ == b.value_);
  }
  friend constexpr BooleanTestable operator!=(StrictComparable const& a, StrictComparable const& b) {
    return BooleanTestable(a.value_ != b.value_);
  }

private:
  T value_;
};

#endif // LIBCXX_TEST_SUPPORT_BOOLEAN_TESTABLE_H

Then you can use it as:

{
    StrictComparable<int> a[] = {1, 2, 3, 4};
    auto ret = std::ranges::find(a, a + 4, StrictComparable<int>{4});
    assert(ret == a + 3);
}
{
    StrictComparable<int> a[] = {1, 2, 3, 4};
    auto ret = std::ranges::find(a, StrictComparable<int>{4});
    assert(ret == a + 3);
}

I'm not a big fan of the name StrictComparable, but I like that we're not mixing up the type in the range and the boolean testable archetype.

[var-const]

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

232

Can you add an explicit return type to make it clear that we're also testing that the algorithm supports predicates that return something convertible to bool, not just bool? (assuming I'm understanding the intention correctly) Applies to find_if_not as well.

[philnik]

• Address comments

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

232

I think this comment doesn't apply anymore. LMK if I'm wrong.

[Mordante]

libcxx/test/support/boolean_testable.h

15

This header needs to be guarded with the proper C++ version.

[var-const]

libcxx/test/support/boolean_testable.h

15

Nit: can this function be const?

23

Do we need this class to work pre-C++20?

27

I presume this constructor intentionally allows implicit conversions, can you please mention that in a comment (assuming it's true)? Applies to StrictComparable as well.

32

Optional: = false?

[philnik]

• Address comments

libcxx/test/support/boolean_testable.h

23

No, but the operator!= only gets implicitly generated if the return value of operator== is bool. Things only compilers know :D.

32

Why? This class only has a single constructor which always sets the value. In this case I think it's more confusing than helpful.

[var-const]

libcxx/test/support/boolean_testable.h

23

Interesting -- might be worth adding a comment, actually (but very optional).

32

Personally, I find it easier because a) I don't need to read through the constructors to make sure this primitive is always initialized, and b) it can prevent bugs (which are easy to fix but still take some unnecessary time) when adding a default constructor later. But if you prefer the current state, please keep as is.

45

Nit: s/initlaizer/initialize/ (or perhaps it should be something like use as an initializer?).

[ldionne]

LGTM but please address Costa's comments.

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

 //     ranges::find(R&& r, const T& value, Proj proj = {});
 
 #include <algorithm>
 #include <array>
 #include <cassert>
 #include <ranges>
 
 #include "almost_satisfies_types.h"
+#include "boolean_testable.h"
 #include "test_iterators.h"
 
 struct NotEqualityComparable {};
 
 template <class It, class Sent = It>
 concept HasFindIt = requires(It it, Sent sent) { std::ranges::find(it, sent, *it); };
 static_assert(HasFindIt<int*>);
 static_assert(!HasFindIt<NotEqualityComparable*>);
test_iterators<contiguous_iterator<int*>>();
     }
     {
       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;
 }

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

 //     ranges::find_if(R&& r, Pred pred, Proj proj = {});
 
 #include <algorithm>
 #include <array>
 #include <cassert>
 #include <ranges>
 
 #include "almost_satisfies_types.h"
+#include "boolean_testable.h"
 #include "test_iterators.h"
 
 struct Predicate {
   bool operator()(int);
 };
 
 template <class It, class Sent = It>
 concept HasFindIfIt = requires(It it, Sent sent) { std::ranges::find_if(it, sent, Predicate{}); };
test_iterators<contiguous_iterator<int*>>();
     }
     {
       std::array<int, 0> a = {};
       auto ret = std::ranges::find_if(a, [](int) { return true; });
       assert(ret == a.begin());
     }
   }
 
+  {
+    // check that the implicit conversion to bool works
+    {
+      int a[] = {1, 2, 3, 4};
+      auto ret = std::ranges::find_if(a, a + 4, [](const int& i) { return BooleanTestable{i == 3}; });
+      assert(ret == a + 2);
+    }
+    {
+      int a[] = {1, 2, 3, 4};
+      auto ret = std::ranges::find_if(a, [](const int& b) { return BooleanTestable{b == 3}; });
+      assert(ret == a + 2);
+    }
+  }
+
   return true;
 }
 
 int main(int, char**) {
   test();
   static_assert(test());
 
   return 0;
 }

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

 //     ranges::find_if_not(R&& r, Pred pred, Proj proj = {});
 
 #include <algorithm>
 #include <array>
 #include <cassert>
 #include <ranges>
 
 #include "almost_satisfies_types.h"
+#include "boolean_testable.h"
 #include "test_iterators.h"
 
 struct Predicate {
   bool operator()(int);
 };
 
 template <class It, class Sent = It>
 concept HasFindIfNotIt = requires(It it, Sent sent) { std::ranges::find_if_not(it, sent, Predicate{}); };
test_iterators<contiguous_iterator<int*>>();
     }
     {
       std::array<int, 0> a = {};
       auto ret = std::ranges::find_if_not(a, [](int) { return true; });
       assert(ret == a.begin());
     }
   }
 
+  {
+    // check that the implicit conversion to bool works
+    {
+      int a[] = {1, 2, 3, 4};
+      auto ret = std::ranges::find_if_not(a, a + 4, [](const int& i) { return BooleanTestable{i != 3}; });
+      assert(ret == a + 2);
+    }
+    {
+      int a[] = {1, 2, 3, 4};
+      auto ret = std::ranges::find_if_not(a, [](const int& b) { return BooleanTestable{b != 3}; });
+      assert(ret == a + 2);
+    }
+  }
+
   return true;
 }
 
 int main(int, char**) {
   test();
   static_assert(test());
 
   return 0;
 }

libcxx/test/support/boolean_testable.h

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LIBCXX_TEST_SUPPORT_BOOLEAN_TESTABLE_H
+#define LIBCXX_TEST_SUPPORT_BOOLEAN_TESTABLE_H
+
+#if TEST_STD_VER > 17
+
+class BooleanTestable {
+public:
+  constexpr operator bool() const {
+    return value_;
+  }
+
+  friend constexpr BooleanTestable operator==(const BooleanTestable& lhs, const BooleanTestable& rhs) {
+    return lhs.value_ == rhs.value_;
+  }
+
+  friend constexpr BooleanTestable operator!=(const BooleanTestable& lhs, const BooleanTestable& rhs) {
+    return !(lhs == rhs);
+  }
+
+  constexpr BooleanTestable operator!() {
+    return BooleanTestable{!value_};
+  }
+
+  // this class should behave like a bool, so the constructor shouldn't be explicit
+  constexpr BooleanTestable(bool value) : value_{value} {}
+  constexpr BooleanTestable(const BooleanTestable&) = delete;
+  constexpr BooleanTestable(BooleanTestable&&) = delete;
+
+private:
+  bool value_;
+};
+
+template <class T>
+class StrictComparable {
+public:
+  // this shouldn't be explicit to make it easier to initlaize inside arrays (which it almost always is)
+  constexpr StrictComparable(T value) : value_{value} {}
+
+  friend constexpr BooleanTestable operator==(const StrictComparable& lhs, const StrictComparable& rhs) {
+    return (lhs.value_ == rhs.value_);
+  }
+
+  friend constexpr BooleanTestable operator!=(const StrictComparable& lhs, const StrictComparable& rhs) {
+    return !(lhs == rhs);
+  }
+
+private:
+  T value_;
+};
+
+#endif // TEST_STD_VER > 17
+
+#endif // LIBCXX_TEST_SUPPORT_BOOLEAN_TESTABLE_H