Skip to content

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

Closed
vstinner wants to merge 1 commit intopython:masterfrom
vstinner:intmax
Closed

[WIP] bpo-17870: Add support for C intmax_t type#857
vstinner wants to merge 1 commit intopython: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 thread Doc/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 thread Modules/_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 thread Modules/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 thread Modules/_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 thread Modules/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 thread Modules/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?

@brettcannon brettcannon added the type-feature A feature request or enhancement label Mar 28, 2017
@vstinner
Copy link
Copy Markdown
Member Author

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

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

Successfully merging this pull request may close these issues.

4 participants