[libcxx-commits] [PATCH] D92255: [libc++] [P0202] constexpr set_union, set_difference, set_symmetric_difference, merge

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.merge/merge.pass.cpp:40
+                         std::end(ib), std::begin(results));
+    return std::equal(std::begin(results), it, std::begin(expected), std::end(expected)) &&
+           std::all_of(it, std::end(results), [](int a) { return a == 0; });
----------------
I think the previous version was a bit easier to read. Especially if we have a dozen assertions in some test function, we can't use this method, and in my opinion, it would be better to make these uniforms. 


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.set.operations/includes/includes_comp.pass.cpp:39
 void
 test()
 {
----------------
Can we make this test constexpr too? 


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.set.operations/set.difference/set_difference.pass.cpp:26
 
+#if TEST_STD_VER > 17
+constexpr bool test_constexpr() {
----------------
What I'd really like to do is rather than adding a `test_constexpr` function, simply modify the existing "test" function to be constexpr and `return true` at the end. If there's an assertion that fires while the constexpr is being evaluated, it will no longer be a valid constexpr and therefore will fail to compile. I think we agreed we should (as much as possible) try to have the same tests for constexpr and non-constexpr. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92255