[libcxx-commits] [PATCH] D129414: [libc++][ranges][NFC] Consolidate range algorithm tests with non-boolean predicates.

Konstantin Varlamov via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sun Jul 10 20:06:52 PDT 2022


var-const created this revision.
Herald added a project: All.
var-const updated this revision to Diff 443404.
var-const added a comment.
var-const updated this revision to Diff 443544.
var-const marked 3 inline comments as done.
var-const published this revision for review.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

Add comment.


philnik added a comment.

All in all I think it's a good idea. I don't really see a way to fuck it up other than with malicious intent if it compiles. You'd have to do something like `if (!is_same_v<decltype(pred()), bool>) { /*do stuff*/ })`. If you do `bool b = pred()` or `if (pred())` it would be implicitly converted to `bool`. if you do `auto x = pred(); auto y = x; if (x) {}` it would error at compile-time. IIUC even if you'd use `invoke_r<bool, ...>(pred)` it should get implicitly converted.


var-const added a comment.

Add tests for omitting `std::invoke`.



================
Comment at: libcxx/test/std/algorithms/ranges_robust_against_nonbool_predicates.pass.cpp:19
+#include <initializer_list>
+#include "boolean_testable.h"
+
----------------
Nit: Add a newline between `<>` and `""` includes


================
Comment at: libcxx/test/std/algorithms/ranges_robust_against_nonbool_predicates.pass.cpp:77
+void test_all() {
+  // TODO(ranges): uncomment test cases as range algorithms are implemented.
+
----------------
I don't think a TODO makes a lot of sense here. We are quite good at adding the tests as we go by now I think.


================
Comment at: libcxx/test/std/algorithms/ranges_robust_against_nonbool_predicates.pass.cpp:169-173
+int main(int, char**) {
+  test_all();
+
+  return 0;
+}
----------------
Maybe just make it a `.compile.pass.cpp`? It's not like we learn anything from running them. They probably work properly if they compile.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129414

Files:
  libcxx/docs/Status/RangesAlgorithms.csv
  libcxx/test/std/algorithms/ranges_robust_against_nonbool_predicates.compile.pass.cpp
  libcxx/test/std/algorithms/ranges_robust_against_omitting_invoke.compile.pass.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129414.443544.patch
Type: text/x-patch
Size: 19836 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220711/2d892963/attachment-0001.bin>


More information about the libcxx-commits mailing list