Repository navigation
Try to reuse stdint.h types like int32_t #62084
Description
| BPO | 17884 |
|---|---|
| Nosy | @mdickinson, @abalkin, @vstinner, @tiran, @benjaminp, @ssbr, @skrah, @serhiy-storchaka |
| Files |
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']
Activity
vstinner commentedon Apr 30, 2013
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 commentedon Apr 30, 2013
See also issue bpo-17870 related to intmax_t and uintmax_t.
mdickinson commentedon May 1, 2013
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 commentedon 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 commentedon 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 commentedon May 1, 2013
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 commentedon May 1, 2013
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 commentedon May 1, 2013
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 commentedon 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 commentedon May 1, 2013
Mark Dickinson <[email protected]> wrote:
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:
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 commentedon May 1, 2013
Mark Dickinson <[email protected]> wrote:
That could be a problem. It's tempting though, since we'd finally have
a uniform behavior for the PyLong_FromXXX functions (bpo-12965).
Definitely.
mdickinson commentedon May 6, 2013
[Victor]
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 commentedon May 6, 2013
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 commentedon May 6, 2013
2013/5/6 Mark Dickinson <[email protected]>:
So in all these 3 cases, it is possible to use "uint32_t" in the code.
Windows issues can be fixed in PC/pyconfig.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.
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 commentedon May 6, 2013
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.
7 remaining items
mdickinson commentedon Sep 6, 2016
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 commentedon 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 commentedon Sep 6, 2016
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 commentedon 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 commentedon 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 commentedon Sep 6, 2016
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 commentedon 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 commentedon 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);
^
vstinner commentedon Sep 12, 2016
Why do you think that it's related to stdint.h changes?
SilentGhost commentedon 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 commentedon Sep 12, 2016
Probably this one: bpo-23545