<complex>: Fix exp and polar on NaN/infinity input #1584
Conversation
statementreply
changed the title
Most of them (except `polar(+Inf, finite)`) are UB on paper. This commit
implements:
- `polar(-rho, theta)` = `-polar(rho, theta)`
- `polar(rho, theta)` ~= `exp(complex{log(rho), theta})`, when
`signbit(rho)` is `false`
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Thanks for fixing this long-standing misbehavior! Please let me know if you'd like me to investigate how to make the code compatible with /clr:pure - I think that avoiding C++20 usage and dealing with the float_control warning should be sufficient, but I can verify that if you want.
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
|
I've pushed a merge with |
|
I'm mirroring this to an MSVC-internal PR. It's totally fine to push changes for code review feedback, but please notify me in that case. |
|
Had to push one more change to fix the internal build - the |
There was a problem hiding this comment.
This does appear to implement advertised behavior, although I mostly checked only that it did not do anything non-conforming (mess with the polar(0,x) case). I observe that the changes to polar are not currently covered by tests, since even after this change the polar llvm tests can not be enabled. We need to make sure to remember to turn those on.
|
Thanks for fixing more of these complex bugs! 😹 🐞 🎉 |
Works towards #887.
exp.Also fix the imaginary part of
exp(complex{re, -0.0}), which should be negative zero. The current implementation returns positive zero.polar.Most of those input cases (except
polar(+Inf, finite)) are UB on paper. This PR implements:polar(-rho, theta)=-polar(rho, theta)polar(rho, theta)≈exp(complex{log(rho), theta}), whensignbit(rho)isfalse