[libcxx-commits] [PATCH] D79555: [libc++] [C++20] [P0415] Constexpr for std::complex.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/docs/Cxx2aStatusPaperStatus.csv:8
 "`P0202R3 <https://wg21.link/P0202R3>`__","LWG","Add constexpr modifiers to functions in <algorithm> and <utility> Headers","Albuquerque","|Complete|","12.0"
-"`P0415R1 <https://wg21.link/P0415R1>`__","LWG","Constexpr for ``std::complex``\ ","Albuquerque","|In Progress|","7.0"
+"`P0415R1 <https://wg21.link/P0415R1>`__","LWG","Constexpr for ``std::complex``\ ","Albuquerque","|Complete|","13.0"
 "`P0439R0 <https://wg21.link/P0439R0>`__","LWG","Make ``std::memory_order``\  a scoped enumeration","Albuquerque","|Complete|",""
----------------



================
Comment at: libcxx/test/std/numerics/complex.number/complex.ops/complex_divide_complex.clang.pass.cpp:11
+
+// ADDITIONAL_COMPILE_FLAGS: -fconstexpr-steps=3500000
+
----------------
curdeius wrote:
> ldionne wrote:
> > I'm quite worried by the need to add this. It means that if we somehow went crazy in the implementation and used a huge number of steps to implement the functionality, we wouldn't catch it with the tests. I'd like to suggest something like this to remove the need to pass `-fconstexpr-steps`:
> > 
> > ```
> > // Helper to avoid blowing the number of steps in constexpr
> > template <typename F>
> > TEST_CONSTEXPR void chunk(std::size_t N, F f) {
> >   for (std::size_t i = (N/5) * 0; i != (N/5) * 1; ++i) f(i);
> >   for (std::size_t i = (N/5) * 1; i != (N/5) * 2; ++i) f(i);
> >   for (std::size_t i = (N/5) * 2; i != (N/5) * 3; ++i) f(i);
> >   for (std::size_t i = (N/5) * 3; i != (N/5) * 4; ++i) f(i);
> >   for (std::size_t i = (N/5) * 4; i != N;         ++i) f(i);
> > }
> > ```
> > 
> > This is just an idea, and it's untested code, but basically I think we could add a generic helper to help us chunk large ranges into several for loops instead of one giant loop. Then we'd use it something like
> > 
> > ```
> > TEST_CONSTEXPR_CXX20
> > bool test_edges() {
> >   const unsigned N = sizeof(testcases) / sizeof(testcases[0]);
> >   int classification[N];
> >   for (unsigned i=0; i < N; ++i)
> >     classification[i] = classify(testcases[i]);
> > 
> >   chunk(N, [](int i) {
> >     for (int j = 0; j != N; ++j) {
> >       std::complex<double> r = testcases[i] / testcases[j];
> >       switch (classification[i]) {
> >         ...
> >       };
> >     }
> >   });
> > }
> > ```
> I'm not sure I understand what you propose.
> The fact that we go over the max. constexpr steps, is because of `static_assert(test_edges());`.
> Which is a single constexpr expression to compute.
> And so, even splitting up testcases to 20 pieces still hits the limit on clang (obviously with the default constexpr-steps limit).
> It has as well a disadvantage of reducing the test coverage.
> 
> On the other hand, I agree that all this file splitting is a burden.
> Would it be doable to have sth like `// ADDITIONAL_COMPILE_FLAGS_IF_SUPPORTED: -fconstexpr-steps=3500000`?
Hmmmmm, yeah, I guess my suggestion was quite silly in retrospect. I didn't realize the constexpr step limit was on the overall constexpr evaluation, but that's the only thing that makes sense now that I think of it. Let's take this discussion to D106798.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79555