Skip to content

[WIP] bpo-17870: Add support for C intmax_t type - #857

Closed
vstinner wants to merge 1 commit into
python:masterfrom
vstinner:intmax
Closed

[WIP] bpo-17870: Add support for C intmax_t type#857
vstinner wants to merge 1 commit into
python:masterfrom
vstinner:intmax

Conversation

@vstinner

Copy link
Copy Markdown
Member
  • Add new conversions functions for PyLong:

    • PyLong_FromIntMax()
    • PyLong_FromUIntMax()
    • PyLong_AsIntMax()
    • PyLong_AsIntMaxAndOverflow()
    • PyLong_AsUIntMax()
  • getargs: add 'm' format

  • New _testcapi constants: INTMAX_MAX, INTMAX_MIN, UINTMAX_MAX, SIZEOF_INTMAX_T

  • Add _testcapi.getargs_m() and _testcapi.test_long_intmax_api()

  • PyLong_FromVoidPtr() uses PyLong_FromUIntMax()

  • Use intmax_t in various modules

array, struct, ctypes and memoryview are not modified yet to support
intmax_t.

* Add new conversions functions for PyLong:
- PyLong_FromIntMax()
- PyLong_FromUIntMax()
- PyLong_AsIntMax()
- PyLong_AsIntMaxAndOverflow()
- PyLong_AsUIntMax()
* getargs: add 'm' format
* New _testcapi constants: INTMAX_MAX, INTMAX_MIN, UINTMAX_MAX, SIZEOF_INTMAX_T
* Add _testcapi.getargs_m() and _testcapi.test_long_intmax_api()
* PyLong_FromVoidPtr() uses PyLong_FromUIntMax()
* Use intmax_t in various modules
array, struct, ctypes and memoryview are not modified yet to support
intmax_t.
Comment threadDoc/c-api/arg.rst
Convert a Python integer to a C :c:type:`Py_ssize_t`.

``m`` (:class:`int`) [intmax_t]
Convert a Python integer to a C :c:type:`intmax_t`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about uintmax_t?

Comment threadModules/_lzmamodule.c
would not work correctly on platforms with 32-bit longs. */
static int
module_add_int_constant(PyObject *m, const char *name, long long value)
module_add_int_constant(PyObject *m, const char *name, intmax_t value)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure 64 bit is enough for all LZMA constants.

Comment threadModules/mmapmodule.c
if (self->file_handle != INVALID_HANDLE_VALUE) {
DWORD low,high;
long long size;
intmax_t size;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

size is a 64-bit number. long long is enough.

Comment threadModules/_tkinter.c
if (Tcl_GetWideIntFromObj(Tkapp_Interp(tkapp), value, &wideValue) == TCL_OK) {
if (sizeof(wideValue) <= SIZEOF_LONG_LONG)
return PyLong_FromLongLong(wideValue);
if (sizeof(wideValue) <= SIZEOF_INTMAX_T) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this condition is always true. I think that it is always true even for long long. I added this check only because the support of long long was optional.

Comment threadModules/posixmodule.c
@@ -11414,11 +11387,7 @@ os_DirEntry_inode_impl(DirEntry *self)
Py_BUILD_ASSERT(sizeof(unsigned long long) >= sizeof(self->win32_file_index));
return PyLong_FromUnsignedLongLong(self->win32_file_index);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be used PyLong_FromUIntMax?

Comment threadModules/posixmodule.c
#else
return PyLong_FromLong((long)self->d_ino);
#endif
return PyLong_FromUIntMax((long)self->d_ino);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(long)?

Can d_ino be negative?

@brettcannonbrettcannon added the type-feature A feature request or enhancement label Mar 28, 2017
@vstinner

Copy link
Copy Markdown
MemberAuthor

Rejected for the reasons explained in the bpo: http://bugs.python.org/issue17870

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-featureA feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner@serhiy-storchaka@brettcannon@the-knights-who-say-ni