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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone marked an inline comment as done.
Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.set.operations/includes/includes_comp.pass.cpp:57
 {
+  if (do_it) {
     test<input_iterator<const int*>, input_iterator<const int*> >();
----------------
zoecarver wrote:
> Why is this needed?
Suppose we remove the guard. And suppose we compile this file as C++14. Then, `do_tests` is marked `constexpr`, but it unconditionally calls `test<input_iterator<const int*>, input_iterator<const int*>>()`, which never returns a compile-time-constant result. My understanding of C++ formalities is that this would make it IFNDR.

Adding the `bool` parameter makes it probably not IFNDR, since now there's a compile-time-constant path through this function even in C++14.

Constexpr is a huge pain. :P


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.set.operations/includes/includes_comp.pass.cpp:39
 void
 test()
 {
----------------
zoecarver wrote:
> Quuxplusone wrote:
> > zoecarver wrote:
> > > Can we make this test constexpr too? 
> > It turns out that we can! So in the latest revision of this PR, I'm proposing that in the specific case of `std::includes`, we can eliminate `test_constexpr` and do everything in `test`.
> This is great! Let's do this for all the tests. 
Well, look at the existing "merge.pass.cpp" for example. It's extremely silly. The test for `std::includes` was really the one special case, where the existing test (was still very silly, but) happened to be constexpr-friendly already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92255