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

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius added inline comments.


================
Comment at: libcxx/test/std/numerics/complex.number/complex.ops/complex_divide_complex.clang.pass.cpp:11
+
+// ADDITIONAL_COMPILE_FLAGS: -fconstexpr-steps=3500000
+
----------------
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`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79555