[libcxx-commits] [PATCH] D155312: [libc++] Use _Complex for multiplication and division of complex floating point types

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

I think this is the right approach to implement floating-point complex. We should let the compiler do the work whenever we can. If the result is "not good enough", then that's something we should take to the compiler folks (probably compiler-rt) instead.

Also, this is how libstdc++ implements their `std::complex` operations, so it's not like we're doing something crazy here. The need to change tests makes me a tiny bit uncomfortable, but in reality I think the tests should have been written that way from the start -- comparing floating point values for equality is a well known anti pattern.

So this roughly looks good to me but I'd like to see again after updating with the comments and green CI.



================
Comment at: libcxx/test/std/numerics/complex.number/complex.member.ops/divide_equal_complex.pass.cpp:28
     c /= c2;
-    assert(c.real() == 1.5);
-    assert(c.imag() == 2.5);
+    assert(c.real() >= T(1.499999999999999));
+    assert(c.real() <= T(1.500000000000001));
----------------
Mordante wrote:
> I would like to test with an epsion and not `>=`.
> I think the values used in the test should give an exact result.
Yeah we should define a function to describe the fact that the value is "equal" (or "within the tolerance") instead of explicitly checking `>=` and `<=`.


================
Comment at: libcxx/test/std/numerics/complex.number/complex.ops/complex_divide_complex.pass.cpp:15
 
 // ADDITIONAL_COMPILE_FLAGS(has-fconstexpr-steps): -fconstexpr-steps=5000000
 
----------------
Is this still required? Applies here and in the other complex tests I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155312