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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D79555#2025500 <https://reviews.llvm.org/D79555#2025500>, @curdeius wrote:

> @ldionne, I'd appreciate if you (or somebody else) could tell me what you think about the way I test the constexpr. I wanted to avoid duplication but on a second thought, I think that run-time tests can actually go through compile-time (constexpr) branch and so leave the non-constexpr branch untested.


I think it's fine -- avoiding duplication is pretty important IMO. I don't think the compiler will try to evaluate this using the constexpr evaluator if it's not marked as `constexpr`. If we wanted to be really really sure, we could also make sure the values we use in the test are thought to be potentially-runtime by calling a function defined in a different TU. That would be somewhat more complicated, but imagine the following setup:

  // in support.cpp
  template <typename T>
  T identity(T t) { return t; }
  
  template int identity(int);
  template float identity(float);
  // etc...

Then, let's say you have a test that is potentially `constexpr` like this:

  template <class T>
  TEST_CONSTEXPR_CXX20 bool test(T zero) {
      T lhs(1.5 + zero);
      std::complex<T> rhs(3.5 + zero, 4.5);
      std::complex<T>   x(5.0 + zero, 4.5);
      test(lhs, rhs, x);
  }

You could now call it once at runtime and once at compile-time:

  int main(int argc, char** argv) {
      test<float>(identity<float>(0)); // runtime test
      static_assert(test<float>(0)); // compile-time test
  }

Since `identity` is defined in a different TU, the compiler would really have to generate code for the runtime tests. There's a few different ways to do this, another way I can think of is to piggy back on `argc` or just a (volatile?) global variable. But the idea is the same. If there's interest, we can look into setting this up more widely for amphibious (constexpr/not constexpr) tests.

> Another thing in which I'd need guidance is how to test the primary implementation (not specializations for float, double, long double). Is there another floating-point type I can use in tests? Maybe some sort of short float?

I'm seeing this on cppreference:

> The specializations `std::complex<float>`, `std::complex<double>`, and `std::complex<long double>` are `LiteralTypes` for representing and manipulating complex numbers. The effect of instantiating the template `complex` for any other type is unspecified.

I don't think we test other specializations in our other complex tests, do we? If not, I'd say it's OK to not test it... Maybe I'm not being imaginative enough?

> Oh, and should integral types be tested with `complex<T>`?

Like I said above, I don't think so. Please LMK if you disagree.

What *does* need to be tested however is the interaction between `std::complex<T>` and `U` where `T in {float, double, long double}` and `U` is an integral type. But you seem to be testing this correct (e.g. in `conj.pass.cpp`).



================
Comment at: libcxx/test/std/numerics/complex.number/complex.ops/scalar_plus_complex.pass.cpp:34
     test(lhs, rhs, x);
-    }
+  }
     {
----------------
Nit: this spacing seems to have changed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79555