gh-122681: merge m_atan2() and c_atan2() helper functions - #122682
Conversation
Sorry, something went wrong.
| is necessary to get consistent behaviour across platforms. | ||
| Windows, FreeBSD and alpha Tru64 are amongst platforms that don't | ||
| always follow C99. Windows screws up atan2 for inf and nan, and | ||
| alpha Tru64 5.1 doesn't follow C99 for atan2(0., 0.). |
There was a problem hiding this comment.
I'm curious if Windows and FreeBSD still need these workaround in 2024. But I don't ask you to test ;-)
Sorry, something went wrong.
There was a problem hiding this comment.
I'm curious if Windows and FreeBSD still need these workaround in 2024.
I doubt, especially FreeBSD.
As I said in the description, most functions in the cmath works with libm's atan2 function (e.g. acos). I think it's safe to remove these helpers. But this should be heavily tested. Here is a draft pr with this attempt: #122715
Sorry, something went wrong.
There was a problem hiding this comment.
I doubt, especially FreeBSD.
Right. When I added math.fma(), I found a bug on FreeBSD. It takes a while to get it fixed upstream, but it's being fixed: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=277783 :-)
Sorry, something went wrong.
vstinner
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
|
I merged the main branch into this PR to retrieve the Hypothesis workaround (pin attrs version). |
Sorry, something went wrong.
|
Merged. Thank you. |
Sorry, something went wrong.
For reviewers: as it mentioned in the issue, I think it worth to try removing these workarounds. Most cmath's functions are using libm's atan2() and this don't poses any problems. If that does make sense, here is a draft pr to test this: #122715.