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

Try to reuse stdint.h types like int32_t #62084

Closed
vstinner opened this issue Apr 30, 2013 · 32 comments
Closed

Try to reuse stdint.h types like int32_t #62084

vstinner opened this issue Apr 30, 2013 · 32 comments
Labels
type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

BPO 17884
Nosy @mdickinson, @abalkin, @vstinner, @tiran, @benjaminp, @ssbr, @skrah, @serhiy-storchaka
Files
  • reuse_stdint_types.patch
  • 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 2016-09-12.17:06:15.555>
    created_at = <Date 2013-04-30.22:52:46.780>
    labels = ['type-feature']
    title = 'Try to reuse stdint.h types like int32_t'
    updated_at = <Date 2016-09-12.17:07:12.732>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2016-09-12.17:07:12.732>
    actor = 'SilentGhost'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-09-12.17:06:15.555>
    closer = 'skrah'
    components = []
    creation = <Date 2013-04-30.22:52:46.780>
    creator = 'vstinner'
    dependencies = []
    files = ['30083']
    hgrepos = []
    issue_num = 17884
    keywords = ['patch']
    message_count = 32.0
    messages = ['188192', '188193', '188210', '188233', '188237', '188238', '188239', '188240', '188243', '188244', '188246', '188497', '188498', '188518', '188520', '188527', '229458', '274579', '274601', '274604', '274607', '274609', '274613', '274617', '274618', '274619', '274624', '274652', '276065', '276067', '276068', '276069']
    nosy_count = 11.0
    nosy_names = ['mark.dickinson', 'belopolsky', 'vstinner', 'christian.heimes', 'benjamin.peterson', 'Devin Jeanpierre', 'SilentGhost', 'skrah', 'python-dev', 'serhiy.storchaka', 'guidod-2011']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue17884'
    versions = ['Python 3.6']

    @vstinner
    Copy link
    Member Author

    In different places, macros like SIZEOF_VOID_P or SIZEOF_LONG to choose a type, whereas the C language proposes standard types since ISO C99: intXX_t and uintXX_t (8, 16, 32, 64), intptr_t, etc.

    Python should rely on these new types instead of trying to reimplement this standard. autotools provides missing types if needed. On Windows, stdint.h is supported in Microsoft Visual Studio 2010.

    Attached patch reuses stdint.h types in different places. I only tested it on Linux Fedora 11.

    sha3 module should also be modified.

    I don't know if int64_t and uint64_t are always supported, nor if we can rely on them. Does Python support platform without 64-bit integer type?

    The md5 module uses PY_LONG_LONG to get a 64-bit type. Does anyone know a platform where the md5 module is not compiled?

    @vstinner
    Copy link
    Member Author

    See also issue bpo-17870 related to intmax_t and uintmax_t.

    @mdickinson
    Copy link
    Member

    Relying on things like int64_t or uint64_t is tricky, both in principle *and* in practice.

    C99 (7.18.1.1) specifies that the types are optional, but that if the implementation provides types with the appropriate characteristics then the typenames should exist. I think we could probably get away with assuming the existence of uint32_t and int32_t and smaller types, but uint64_t and int64_t may be a stretch, particularly on ARM-style platforms.

    In practice, we've had significant difficulties in the past simply finding and identifying exact-width types in our autoconf machinery: whether they're defined in <inttypes.h> or <stdint.h> seems to vary from platform to platform, as does whether they're defined as typedef's or preprocessor macros. I *think* that since the issue bpo-10052 fix, the current autoconf machinery is now fairly good at finding those types across platforms, but I wouldn't want to make any bets.

    I do agree that in principle it would be nice to define conversions for the fixed-width types and have everything else defer to those.

    There's also some cleanup to be done with respect to semantics; I think it's still the case that the various PyLong_FromXXX functions have different behaviours with respect to overflow, __int__, __index__ and the like. If we just blindly map the old functions to the fixed-width versions we're going to end up changing those semantics on some platforms.

    I'd be quite happy to see fixed-width conversion functions that *completely ignore* __int__ and __index__, and leave the magic stuff to general PyNumber_... functions.

    Adding skrah to the nosy, since this is something I think he's spent some time thinking about, too.

    @ssbr
    Copy link
    Mannequin

    ssbr mannequin commented May 1, 2013

    I don't know what context these types are being used in, but would int_least64_t suffice? C99 does require the existence of the [u]int_leastN_t types (for N in {8,16,32,64}), unlike [u]intN_t.

    @vstinner
    Copy link
    Member Author

    vstinner commented May 1, 2013

    "Relying on things like int64_t or uint64_t is tricky, both in
    principle *and* in practice. (...) uint64_t and int64_t may be a
    stretch, particularly on ARM-style platforms."

    I don't understand. Python is already using 64-bit types, in md5
    module for example (MD5_INT64). This module is not compiled on ARM?

    "In practice, we've had significant difficulties in the past simply
    finding and identifying exact-width types in our autoconf machinery:
    whether they're defined in <inttypes.h> or <stdint.h> seems to vary
    from platform to platform, as does whether they're defined as
    typedef's or preprocessor macros."

    I don't understand. AC_TYPE_UINT64_T is supposed to provide the
    uint64_t (unsigned integer of 64-bit).

    Autotool can also generate the stdint.h header if it is not available.

    @vstinner
    Copy link
    Member Author

    vstinner commented May 1, 2013

    I *think* that since the issue bpo-10052 fix, the current autoconf
    machinery is now fairly good at finding those types across platforms,
    but I wouldn't want to make any bets.

    The issue bpo-10052 was not that the int32_t was not present, but an #ifdef issue: "This error occurs when HAVE_UINT32_T or HAVE_INT32_T are not defined".

    I don't understand why "#ifdef HAVE_UINT32_T" is tested, since configure ensures that uint32_t is always defined. (If it is not, it is defined in pyconfig.h using a #define.)

    @mdickinson
    Copy link
    Member

    I don't understand why "#ifdef HAVE_UINT32_T" is tested, since configure ensures that uint32_t is always defined.

    Take a look at the explanations in the autoconf file and in pyport.h. No, configure does *not* always ensure that uint32_t is defined: it does that only if the platform *doesn't* provide uint32_t, but does provide a 32-bit exact-width unsigned integer type (two's complement, no padding bits, etc. etc.). Which is why we need to make a second check for the case that the platform *does* define uint32_t directly.

    @mdickinson
    Copy link
    Member

    I don't understand. Python is already using 64-bit types, in md5
    module for example (MD5_INT64). This module is not compiled on ARM?

    No idea. Do you have good evidence that 64-bit integer types *will* be supported on all platforms that we care about Python compiling on?

    @vstinner
    Copy link
    Member Author

    vstinner commented May 1, 2013

    Ok, I think I understood the issue :-) The problem is when the uint32_t type is present but is not exactly 32-bit width. When using uint32_t, *I* expect that an array of uint32_t items will takes 4 x n bytes. In which case is it interesting to use an uint32_t which may be bigger? If there is an use case (speed maybe?), we should define a Py_uint32_t which is always present and always exaclty 32 bits. If there is no use case (ex: int_fast32_t or int_least32_t can be used instead), it is maybe better to replace the available type using a #define to use the expect type.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented May 1, 2013

    Mark Dickinson <report@bugs.python.org> wrote:

    No idea. Do you have good evidence that 64-bit integer types *will* be
    supported on all platforms that we care about Python compiling on?

    I'm not sure how many people have tried to compile Python 3.3 on obscure
    platforms, but libmpdec currently requires manual intervention to switch
    on the "without uint64_t" mode:

    /* The following #error is just a warning. If the compiler indeed does
     * not have uint64_t, it is perfectly safe to comment out the #error. */
    #error "Warning: Compiler without uint64_t. Comment out this line."
    

    I did this because I'm almost certain that a failure to detect uint64_t is
    more likely a ./configure issue than an actual absence of the type.

    So far there have been no reports. libmpdec also uses stdint.h directly,
    and there haven't been any reports either.

    Some commercial Unix buildbots have loads of problems, but support stdint.h,
    static inline functions in headers and even the tricky extern inline
    C99 functions (See: http://www.greenend.org.uk/rjk/tech/inline.html).

    If other developers support it, I'd actually like to write a PEP that allows
    unrestricted direct use of stdint.h and static inline functions in header files
    for Python 3.4.

    My gut feeling is that if a platform doesn't have these features, it will
    likely be the least of their problems.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented May 1, 2013

    Mark Dickinson <report@bugs.python.org> wrote:

    There's also some cleanup to be done with respect to semantics; I think it's still the case that the various PyLong_FromXXX functions have different behaviours with respect to overflow, __int__, __index__ and the like. If we just blindly map the old functions to the fixed-width versions we're going to end up changing those semantics on some platforms.

    That could be a problem. It's tempting though, since we'd finally have
    a uniform behavior for the PyLong_FromXXX functions (bpo-12965).

    I'd be quite happy to see fixed-width conversion functions that *completely ignore* __int__ and __index__, and leave the magic stuff to general PyNumber_... functions.

    Definitely.

    @mdickinson
    Copy link
    Member

    [Victor]

    Ok, I think I understood the issue :-) The problem is when the uint32_t
    type is present but is not exactly 32-bit width.

    No, that's still not the issue :-). In any case, it would be a fairly serious violation of C99 to use uint32_t for something that *wasn't* exactly 32 bits in width. More generally, padding bits, trap representations, and non two's complement representations for signed integers seem to be a thing of the past; having integer types be exact width seems to be one of the few things that we *can* reasonably rely on.

    Concentrating on uint32_t for specificity, there are essentially three cases:

    (1) The platform (either in stdint.h or inttypes.h) #defines uint32_t.

    (2) The platform (either in stdint.h or inttypes.h) makes a typedef for uint32_t.

    (3) The platform doesn't do (1) *or* (2), but nevertheless, there's an unsigned integer type that has exact 32-bit width (and it's probably called "unsigned int").

    [Oh, and (4) Windows. Let's leave that out of this discussion, since there don't seem to be any Windows-specific problems in practice here, and we don't use autoconf.]

    We've encountered all three of these cases, and as far as I know in recent history we haven't encountered a platform that doesn't match at least one of these three cases.

    With respect to the autoconf and pyport machinery:

    In case (1): AC_TYPE_UINT32_T does nothing, because uint32_t is already available, while the
    AC_CHECK_TYPE(uint32_t) #defines HAVE_UINT32_T.

    In case (2): AC_TYPE_UINT32_T still does nothing, and again AC_CHECK_TYPE(uint32_t) #defines HAVE_UINT32_T.

    In case (3): AC_TYPE_UINT32_T #defines uint32_t to be the appropriate type, and AC_CHECK_TYPE(uint32_t) will fail.

    So cases (1) and (3) lead to uint32_t being #defined, while cases (1) and (2) lead to HAVE_UINT32_T being defined. We want to catch all 3 cases, so we have to check for *both* uint32_t and HAVE_UINT32_T in pyport.h.

    Note that using AC_TYPE_UINT32_T and checking whether uint32_t is defined is not enough on platforms that do (2): because uint32_t is a typedef rather than a #define, there's no easy way for pyport.h to find it. Prior to the issue bpo-10052 fix, we assumed that any platform doing (2) would, following C99, also define UINT32_MAX, but that turned out not to be true on some badly-behaved platforms.

    @mdickinson
    Copy link
    Member

    Stefan: sounds reasonable. I'd be happy to make availability of exact-width 32-bit and 64-bit, signed and unsigned integer types a requirement for building Python, provided that we run that by the python-dev mailing list first. I agree that this is what we seem to be doing in practice anyway, so we'd just be codifying existing practice.

    @vstinner
    Copy link
    Member Author

    vstinner commented May 6, 2013

    2013/5/6 Mark Dickinson <report@bugs.python.org>:

    Concentrating on uint32_t for specificity, there are essentially three cases:

    (1) The platform (either in stdint.h or inttypes.h) #defines uint32_t.

    (2) The platform (either in stdint.h or inttypes.h) makes a typedef for uint32_t.

    (3) The platform doesn't do (1) *or* (2), but nevertheless, there's an unsigned integer type that has exact 32-bit width (and it's probably called "unsigned int").

    So in all these 3 cases, it is possible to use "uint32_t" in the code.

    [Oh, and (4) Windows. Let's leave that out of this discussion, since there don't seem to be any Windows-specific problems in practice here, and we don't use autoconf.]

    Windows issues can be fixed in PC/pyconfig.h.

    So cases (1) and (3) lead to uint32_t being #defined, while cases (1) and (2) lead to HAVE_UINT32_T being defined. We want to catch all 3 cases, so we have to check for *both* uint32_t and HAVE_UINT32_T in pyport.h.

    Sorry I still don't understand why do you need HAVE_UINT32_T define.
    According to what you wrote above, uint32_t can always be used on any
    platform.

    Note that using AC_TYPE_UINT32_T and checking whether uint32_t is defined is not enough on platforms that do (2): because uint32_t is a typedef rather than a #define, there's no easy way for pyport.h to find it. Prior to the issue bpo-10052 fix, we assumed that any platform doing (2) would, following C99, also define UINT32_MAX, but that turned out not to be true on some badly-behaved platforms.

    Why should Python do extra checks in pyport.h (ex: check if UINT32_MAX
    is available)? Can't we rely on stdint.h?

    If stdint.h is not available, we can ask configure to build it for us
    using AX_CREATE_STDINT_H. Oh, it looks like this macro is not part of
    autotools directly. It's a third party module released under the GNU
    GPLv3+ license. Can we include a GPL script in our tool chain? I
    contacted the author by email to ask him if we can distribute it under
    a different license (compatible with the Python license).

    http://www.gnu.org/software/autoconf-archive/ax_create_stdint_h.html
    http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=history;f=m4/ax_create_stdint_h.m4

    @mdickinson
    Copy link
    Member

    According to what you wrote above, uint32_t can always be used on any
    platform.

    The issue is in *detecting* whether uint32_t exists or not. In cases (1) and (3), that can be done in the preprocessor with an "#ifdef uint32_t". In case (2), it can't: the check fails, because uint32_t isn't a preprocessor define---it's a typedef. *That's* why we need the HAVE_UINT32_T check.

    @guidod-2011
    Copy link
    Mannequin

    guidod-2011 mannequin commented May 6, 2013

    Relicensing to Python core from my Opensource-licensed material is always permitted, including ax_create_stdint_h.m4

    @pitrou
    Copy link
    Member

    pitrou commented Oct 15, 2014

    I'm in favour of trying this in 3.5. A platform not supporting those types would not be able to run a lot of contemporary software, I think.

    @pitrou pitrou added the type-feature A feature request or enhancement label Oct 15, 2014
    @benjaminp
    Copy link
    Contributor

    In 3.6, I just deprecated support for platforms without "long long". https://bugs.python.org/issue27961 "deprecated" is a strong word because I couldn't actually find a modern Python version that compiles without it. I'm informed that the MSVC we're since 3.5 will have stdint.h available. I think we should start requiring it.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 6, 2016

    New changeset ae03163b6378 by Benjamin Peterson in branch 'default':
    require standard int types to be defined (bpo-17884)
    https://hg.python.org/cpython/rev/ae03163b6378

    @mdickinson
    Copy link
    Member

    LVGTM, if the buildbots agree.

    @serhiy-storchaka
    Copy link
    Member

    Python/dtoa.c contains a number of "#ifdef ULLong". Since now ULLong is not a macro but a typedef, this condition is false and the compiler now compiles different (less efficient and perhaps less tested) branch of the code.

    @mdickinson
    Copy link
    Member

    Good point. I'd be happy to see the non-"#ifdef ULLong" branches simply disappear in dtoa.c. We're long past the point of trying to keep dtoa.c in sync with the upstream version.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 6, 2016

    New changeset 8b74e5528f35 by Benjamin Peterson in branch 'default':
    dtoa.c: remove code for platforms with 64-bit integers (bpo-17884)
    https://hg.python.org/cpython/rev/8b74e5528f35

    @serhiy-storchaka
    Copy link
    Member

    Include/pyport.h now includes both <inttypes.h> and <stdint.h>. But the <inttypes.h> header shall include the <stdint.h> header. [1]

    [1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/inttypes.h.html

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 6, 2016

    New changeset c092eb31db05 by Benjamin Peterson in branch 'default':
    only include inttypes.h (bpo-17884)
    https://hg.python.org/cpython/rev/c092eb31db05

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 6, 2016

    New changeset be8645213517 by Benjamin Peterson in branch 'default':
    replace Python aliases for standard integer types with the standard integer types (bpo-17884)
    https://hg.python.org/cpython/rev/be8645213517

    @benjaminp
    Copy link
    Contributor

    Thank you all for your advice. The buildbots seem to have survived the removal of conditional includes of <inttypes.h>. I've also replaced most of the Python compatibility aliases with the standard ones. Closing this issue.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 6, 2016

    New changeset 1d29d3a1e0c6 by Victor Stinner in branch 'default':
    Run Argument Clinic on posixmodule.c
    https://hg.python.org/cpython/rev/1d29d3a1e0c6

    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Sep 12, 2016

    I get the following compiler warning now:

    In file included from ./Include/Python.h:66:0,
    from /cpython/Modules/_ssl.c:19:
    /cpython/Modules/_ssl.c: In function ‘_setup_ssl_threads’:
    ./Include/pymem.h:136:18: warning: comparison is always false due to limited range of data type [-Wtype-limits]
    ( ((size_t)(n) > PY_SSIZE_T_MAX / sizeof(type)) ? NULL : \
    ^
    /cpython/Modules/_ssl.c:5076:22: note: in expansion of macro ‘PyMem_New’
    _ssl_locks = PyMem_New(PyThread_type_lock, _ssl_locks_count);
    ^

    @SilentGhost SilentGhost mannequin reopened this Sep 12, 2016
    @vstinner
    Copy link
    Member Author

    I get the following compiler warning now: (...)

    Why do you think that it's related to stdint.h changes?

    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Sep 12, 2016

    Probably, not for a particularly good reason, but none of the near-by code in _ssl.c seemed relevant on superficial inspection. The commit 02c8db9c255f which modified Include/pymem.h didn't have an issue associated with it.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Sep 12, 2016

    Probably this one: bpo-23545

    @skrah skrah mannequin closed this as completed Sep 12, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants