Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the Py_NO_NAN macro: require NAN to build Python 3.11 #90814

Closed
mdickinson opened this issue Feb 6, 2022 · 13 comments
Closed

Remove the Py_NO_NAN macro: require NAN to build Python 3.11 #90814

mdickinson opened this issue Feb 6, 2022 · 13 comments
Labels
3.11 build The build process and cross-build interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@mdickinson
Copy link
Member

mdickinson commented Feb 6, 2022

BPO 46656
Nosy @mdickinson, @vstinner
PRs
  • bpo-46656: Remove Py_NO_NAN macro #31160
  • bpo-46656: Building Python now requires a C11 compiler #31557
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2022-02-25.00:45:52.191>
    created_at = <Date 2022-02-06.09:54:34.993>
    labels = ['interpreter-core', 'type-bug', 'build', '3.11']
    title = 'Remove the Py_NO_NAN macro: require NAN to build Python 3.11'
    updated_at = <Date 2022-03-10.16:52:54.326>
    user = 'https://github.com/mdickinson'

    bugs.python.org fields:

    activity = <Date 2022-03-10.16:52:54.326>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-02-25.00:45:52.191>
    closer = 'vstinner'
    components = ['Build', 'Interpreter Core']
    creation = <Date 2022-02-06.09:54:34.993>
    creator = 'mark.dickinson'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 46656
    keywords = ['patch']
    message_count = 13.0
    messages = ['412620', '412621', '412625', '412629', '412630', '412632', '412636', '412640', '412642', '412864', '413944', '413945', '414864']
    nosy_count = 2.0
    nosy_names = ['mark.dickinson', 'vstinner']
    pr_nums = ['31160', '31557']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue46656'
    versions = ['Python 3.11']

    @mdickinson
    Copy link
    Member Author

    mdickinson commented Feb 6, 2022

    The macro Py_NAN may or may not be defined: in particular, a platform that doesn't have NaNs is supposed to be able to define Py_NO_NAN in pyport.h to indicate that.

    But not all of our uses of Py_NAN are guarded by suitable #ifdef conditionals. As a result, compilation fails if Py_NAN is not defined.

    @mdickinson mdickinson added 3.9 3.10 3.11 type-bug An unexpected behavior, bug, or error labels Feb 6, 2022
    @mdickinson
    Copy link
    Member Author

    mdickinson commented Feb 6, 2022

    Here's the first point of failure on my machine. Fixing this shows up more failures.

    gcc -c -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -std=c99 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -I. -I./Include -DPy_BUILD_CORE -o Objects/complexobject.o Objects/complexobject.c
    Objects/complexobject.c:120:27: error: use of undeclared identifier 'Py_NAN'
    r.real = r.imag = Py_NAN;
    ^
    Objects/complexobject.c:206:16: error: use of undeclared identifier 'Py_NAN'
    return Py_NAN;
    ^
    2 errors generated.
    make: *** [Objects/complexobject.o] Error 1

    @vstinner
    Copy link
    Member

    vstinner commented Feb 6, 2022

    Python uses Py_NAN without "#ifdef Py_NAN" guard since 2008. Building Python with Py_NO_NAN never worked. Nobody reported such build failure in the last 14 years...

    IMO it's time to stop supporting platforms without NaN support.

    Objects/complexobject.c:120:27: error: use of undeclared identifier 'Py_NAN'

    I only try a few Python versions: I reproduce this issue in Python 3.11, 3.5, 3.2 and... even Python 2.7!

    The Py_NO_NAN macro was introduced in Python 2.6 (2007) by bpo-1635 "Float patch for inf and nan on Windows (and other platforms)":

    commit 0a8143f
    Author: Christian Heimes <christian@cheimes.de>
    Date: Tue Dec 18 23:22:54 2007 +0000

    Applied patch bpo-1635: Float patch for inf and nan on Windows (and other platforms).
    
    The patch unifies float("inf") and repr(float("inf")) on all platforms.
    

    (...)

    +#if !defined(Py_NAN) && !defined(Py_NO_NAN)
    +#define Py_NAN (Py_HUGE_VAL * 0.)
    +#endif
    ---

    The following change started to use Py_NAN in many C files:

    ---
    commit 6f34109 (HEAD)
    Author: Christian Heimes <christian@cheimes.de>
    Date: Fri Apr 18 23:13:07 2008 +0000

    I finally got the time to update and merge Mark's and my trunk-math branch. The patch is collaborated w
    

    ork of Mark Dickinson and me. It was mostly done a few months ago. The patch fixes a lot of loose ends and
    edge cases related to operations with NaN, INF, very small values and complex math.

    The patch also adds acosh, asinh, atanh, log1p and copysign to all platforms. Finally it fixes differen
    

    ces between platforms like different results or exceptions for edge cases. Have fun :)
    ---

    At this commit, floatobject.c and pymath.c use "#ifdef Py_NAN" carefully, whereas cmathmodule.c and complexobject.c use Py_NAN with no "#ifdef Py_NAN" guard.

    @vstinner
    Copy link
    Member

    vstinner commented Feb 6, 2022

    If someone suddenly requires platforms without NaN support, they can maintain a (downstream) patch, or we can revert the change in Python.

    @vstinner
    Copy link
    Member

    vstinner commented Feb 6, 2022

    See also bpo-46640 "Python can now use the C99 NAN constant or __builtin_nan()".

    @mdickinson
    Copy link
    Member Author

    mdickinson commented Feb 6, 2022

    Okay, the comments I made on bpo-46640 still apply (even though they didn't properly apply on that issue). I think this needs a python-dev discussion before it can be moved forward - requiring the existence of NaNs is very close to requiring IEEE 754 floating-point, and that's something we've been historically reluctant to do.

    @vstinner
    Copy link
    Member

    vstinner commented Feb 6, 2022

    Is the macro PY_NO_SHORT_FLOAT_REPR also related to platforms which don't support IEEE 754?

    In 2022, which platforms don't support IEEE 754?

    @mdickinson
    Copy link
    Member Author

    mdickinson commented Feb 6, 2022

    Is the macro PY_NO_SHORT_FLOAT_REPR also related to platforms which don't support IEEE 754?

    Yes, though it's a bit more than that: we also need the platform either not to have issues with double rounding for normal numbers, or we need to be able to control the x87 rounding mode in the case that double rounding might be an issue. See the explanations in the source.

    cpython/Include/pyport.h

    Lines 345 to 355 in 025cbe7

    /* If we can't guarantee 53-bit precision, don't use the code
    in Python/dtoa.c, but fall back to standard code. This
    means that repr of a float will be long (17 sig digits).
    Realistically, there are two things that could go wrong:
    (1) doubles aren't IEEE 754 doubles, or
    (2) we're on x86 with the rounding precision set to 64-bits
    (extended precision), and we don't know how to change
    the rounding precision.
    */

    In 2022, which platforms don't support IEEE 754?

    None that CPython might plausibly run on that I'm aware of.

    @mdickinson
    Copy link
    Member Author

    mdickinson commented Feb 6, 2022

    See the explanations in the source.

    Hmm. Those explanations made more sense before PR #73068. :-(

    @vstinner
    Copy link
    Member

    vstinner commented Feb 8, 2022

    Requiring IEEE 754 support is being discussed on python-dev: https://mail.python.org/archives/list/python-dev@python.org/thread/J5FSP6J4EITPY5C2UJI7HSL2GQCTCUWN/

    @vstinner
    Copy link
    Member

    vstinner commented Feb 25, 2022

    New changeset 5f8b5e2 by Victor Stinner in branch 'main':
    bpo-46656: Building Python now requires a C11 compiler (GH-31557)
    5f8b5e2

    @vstinner
    Copy link
    Member

    vstinner commented Feb 25, 2022

    New changeset 1b2611e by Victor Stinner in branch 'main':
    bpo-46656: Remove Py_NO_NAN macro (GH-31160)
    1b2611e

    @vstinner vstinner added build The build process and cross-build interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) and removed 3.9 3.10 labels Feb 25, 2022
    @vstinner vstinner changed the title Compile fails if Py_NO_NAN is defined Remove the Py_NO_NAN macro: require NAN to build Python 3.11 Feb 25, 2022
    @vstinner vstinner added build The build process and cross-build interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) and removed 3.9 3.10 labels Feb 25, 2022
    @vstinner vstinner changed the title Compile fails if Py_NO_NAN is defined Remove the Py_NO_NAN macro: require NAN to build Python 3.11 Feb 25, 2022
    @vstinner
    Copy link
    Member

    vstinner commented Mar 10, 2022

    bpo-46656: Remove Py_NO_NAN macro (GH-31160)

    Change documented by:

    New changeset 7854012 by Victor Stinner in branch 'main':
    bpo-46917: math.nan is now always available (GH-31793)
    7854012

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    encukou added a commit to encukou/cpython that referenced this issue Aug 26, 2022
    The current wording of this entry suggests that CPython
    won't work if optional compiler features are enabled.
    That's not the case. The change is that we require C11 rather
    than C89.
    
    Note that PEP 7 does say "Python 3.11 and newer versions use C11
    without optional features." It is correct there: that's
    not a guide for users who compile Python, but for CPython devs
    who must avoid the features.
    encukou added a commit that referenced this issue Aug 29, 2022
    The previous wording of this entry suggests that CPython
    won't work if optional compiler features are enabled.
    That's not the case. The change is that we require C11 rather
    than C89.
    
    Note that PEP 7 does say "Python 3.11 and newer versions use C11
    without optional features." It is correct there: that's
    not a guide for users who compile Python, but for CPython devs
    who must avoid the features.
    encukou added a commit to encukou/cpython that referenced this issue Aug 29, 2022
    …nGH-96309)
    
    The previous wording of this entry suggests that CPython
    won't work if optional compiler features are enabled.
    That's not the case. The change is that we require C11 rather
    than C89.
    
    Note that PEP 7 does say "Python 3.11 and newer versions use C11
    without optional features." It is correct there: that's
    not a guide for users who compile Python, but for CPython devs
    who must avoid the features.
    encukou added a commit that referenced this issue Aug 29, 2022
    …H-96384)
    
    The previous wording of this entry suggests that CPython
    won't work if optional compiler features are enabled.
    That's not the case. The change is that we require C11 rather
    than C89.
    
    Note that PEP 7 does say "Python 3.11 and newer versions use C11
    without optional features." It is correct there: that's
    not a guide for users who compile Python, but for CPython devs
    who must avoid the features.
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 build The build process and cross-build interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants