GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-119613: use C99+ functions instead of Py_IS_NAN/INFINITY/FINITE by skirpichev · Pull Request #119619 · python/cpython Conversation
|
|
@tim-one is there an on-going reason to have these portability macros? Are NaN values guaranteed to be supported? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Since 194a952 these macros are just aliases for isnan, isinf, etc: Lines 30 to 40 in 3e8b609
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Simple text replacement.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Python 3.10 and older supported platforms without IEEE 754, without NaN, and without C99 #ifndef Py_IS_NAN #if defined HAVE_DECL_ISNAN && HAVE_DECL_ISNAN == 1 #define Py_IS_NAN(X) isnan(X) #else #define Py_IS_NAN(X) ((X) != (X)) #endif #endif Python 3.11 and newer requires IEEE 754, NaN and C99 #define Py_IS_NAN(X) isnan(X) Issues:
See also this LWN article: CPython, C standards, and IEEE 754. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@mdickinson: Are you ok with this change? You wrote a comment about these macros there: #119457 (comment) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Yes, absolutely. I'm fairly sure the macros were born out of (now somewhat ancient) portability needs that no longer apply, now that C99 (and later) adoption is sufficiently widespread. I'd suggest running all buildbots on this PR, just to be on the safe side. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
🤖 New build scheduled with the buildbot fleet by @mdickinson for commit 13dcd12 🤖 If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
There are tests failures, but hardly it's related to the pr. Something expected, given there are not build bots for non-IEEE-754 platforms, right? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Agreed - the failures look unrelated. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Merged, thanks. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.