[libcxx-commits] [PATCH] D122011: [libc++][ranges] Add implicit conversion to bool test for ranges::find{, if, if_not}

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/test/support/boolean_testable.h:14
+public:
+  constexpr operator bool() {
+    return value_;
----------------
Mordante wrote:
> This header needs to be guarded with the proper C++ version.
Nit: can this function be `const`?


================
Comment at: libcxx/test/support/boolean_testable.h:22
+
+  friend constexpr BooleanTestable operator!=(const BooleanTestable& lhs, const BooleanTestable& rhs) {
+    return !(lhs == rhs);
----------------
Do we need this class to work pre-C++20?


================
Comment at: libcxx/test/support/boolean_testable.h:26
+
+  constexpr BooleanTestable(bool value) : value_{value} {}
+  constexpr BooleanTestable(const BooleanTestable&) = delete;
----------------
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.


================
Comment at: libcxx/test/support/boolean_testable.h:31
+private:
+  bool value_;
+};
----------------
Optional: `= false`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122011