-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
gh-102471, PEP 757: Add PyLong import and export API #121339
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
Changes from 1 commit
f4fdbf2
c2e568e
f0d9525
b19764f
6f7fd11
080e079
1a7902f
b70a6dd
07552a7
0d0f942
762c33a
20be7a3
d92bf1e
b3b02a2
caca2d7
4221a49
37b1d49
d70a121
4aa25f6
90973d4
5d3e224
c7d7cb2
a3d601a
c049268
06b196b
3e8d296
86c68c2
a8fd669
a04f9d0
b2be94a
ca98ad1
167d75e
5e53a5b
c24789f
0422f9d
a529a48
3db44f3
1d2863e
d663511
816798d
033bd65
36b87d4
94d852e
a72ff83
53d584b
577598a
b08cd55
eaebef3
03248c7
0a26f97
88a62fe
45517ab
92007d1
6d3cb80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
* Rename functions and structure: * PyLong_AsDigitArray() => PyLong_Export() * PyLong_FreeDigitArray() => PyLong_FreeExport() * PyLong_DigitArray => PyLongExport * 'array' => 'export_long'
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -686,7 +686,7 @@ Export API | |||||||||||||||
Read-only array of unsigned digits. | ||||||||||||||||
|
||||||||||||||||
|
||||||||||||||||
.. c:function:: int PyLong_AsDigitArray(PyObject *obj, PyLong_DigitArray *array) | ||||||||||||||||
.. c:function:: int PyLong_Export(PyObject *obj, PyLong_DigitArray *array) | ||||||||||||||||
|
||||||||||||||||
Export a Python :class:`int` object as an array of digits. | ||||||||||||||||
|
||||||||||||||||
|
@@ -696,13 +696,13 @@ Export API | |||||||||||||||
This function always succeeds if *obj* is a Python :class:`int` object or a | ||||||||||||||||
subclass. | ||||||||||||||||
|
||||||||||||||||
:c:func:`PyLong_FreeDigitArray` must be called once done with using | ||||||||||||||||
:c:func:`PyLong_FreeExport` must be called once done with using | ||||||||||||||||
*array*. | ||||||||||||||||
|
||||||||||||||||
There was a problem hiding this comment.
8000
Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Lets see if we can restore this in a that way. It might be helpful for e.g. Sage, which doesn't support PyPy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer to not add this note. It was controversial during PEP 757 design. @serhiy-storchaka @encukou: What do you think? Would you be ok to declare that the PyLong_Export() function cannot fail if the argument is a Python int? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It was proposed unconditionally, not as CPython's implementation detail. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with it, as an implementation detail. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||||
|
||||||||||||||||
.. c:function:: void PyLong_FreeDigitArray(PyLong_DigitArray *array) | ||||||||||||||||
.. c:function:: void PyLong_FreeExport(PyLong_DigitArray *array) | ||||||||||||||||
|
||||||||||||||||
Release the export *array* created by :c:func:`PyLong_AsDigitArray`. | ||||||||||||||||
Release the export *array* created by :c:func:`PyLong_Export`. | ||||||||||||||||
|
||||||||||||||||
|
||||||||||||||||
PyLongWriter API | ||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -6791,31 +6791,43 @@ const PyLongLayout* PyLong_GetNativeLayout(void) | |||||||
|
||||||||
|
||||||||
int | ||||||||
PyLong_AsDigitArray(PyObject *obj, PyLong_DigitArray *array) | ||||||||
PyLong_Export(PyObject *obj, PyLongExport *export_long) | ||||||||
{ | ||||||||
if (!PyLong_Check(obj)) { | ||||||||
vstinner marked this conversation as resolved.
Show resolved
Hide resolved
vstinner marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
PyErr_Format(PyExc_TypeError, "expect int, got %T", obj); | ||||||||
return -1; | ||||||||
} | ||||||||
PyLongObject *self = (PyLongObject*)obj; | ||||||||
int64_t value; | ||||||||
if (PyLong_AsInt64(obj, &value) >= 0) { | ||||||||
export_long->value = value; | ||||||||
export_long->negative = 0; | ||||||||
export_long->ndigits = 0; | ||||||||
export_long->digits = 0; | ||||||||
vstinner marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
export_long->_reserved = 0; | ||||||||
} | ||||||||
else { | ||||||||
PyErr_Clear(); | ||||||||
|
||||||||
array->negative = _PyLong_IsNegative(self); | ||||||||
array->ndigits = _PyLong_DigitCount(self); | ||||||||
if (array->ndigits == 0) { | ||||||||
array->ndigits = 1; | ||||||||
PyLongObject *self = (PyLongObject*)obj; | ||||||||
export_long->value = 0; | ||||||||
export_long->negative = _PyLong_IsNegative(self); | ||||||||
export_long->ndigits = _PyLong_DigitCount(self); | ||||||||
if (export_long->ndigits == 0) { | ||||||||
export_long->ndigits = 1; | ||||||||
} | ||||||||
Comment on lines
+6829
to
+6831
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's to make the API easier to use: the consumer doesn't have to both with ndigits==0 special case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact it's assumed (see e.g. But I think it's safe just drop this check. This condition is unreachible here, as
Suggested change
|
||||||||
export_long->digits = self->long_value.ob_digit; | ||||||||
export_long->_reserved = (Py_uintptr_t)Py_NewRef(obj); | ||||||||
} | ||||||||
array->digits = self->long_value.ob_digit; | ||||||||
array->_reserved = (Py_uintptr_t)Py_NewRef(obj); | ||||||||
return 0; | ||||||||
} | ||||||||
|
||||||||
|
||||||||
void | ||||||||
PyLong_FreeDigitArray(PyLong_DigitArray *array) | ||||||||
PyLong_FreeExport(PyLongExport *export_long) | ||||||||
{ | ||||||||
PyObject *obj = (PyObject*)array->_reserved; | ||||||||
array->_reserved = 0; | ||||||||
Py_DECREF(obj); | ||||||||
PyObject *obj = (PyObject*)export_long->_reserved; | ||||||||
export_long->_reserved = 0; | ||||||||
Py_XDECREF(obj); | ||||||||
vstinner marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
} | ||||||||
|
||||||||
|
||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
?
Or in this way you are trying to emphasize that array is read-only?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to allocate
PyLong_DigitArray
on the stack memory and pass&array
. It avoids one memory allocation on the heap.Also, I like the return 0 for success and -1 for error: https://devguide.python.org/developer-workflow/c-api/index.html#guidelines-for-expanding-changing-the-public-api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try to benchmark this.
With a different signature you could do declaration & initialization of PyLong_DigitArray in one line.
I don't think this guide ban using NULL to signal error (which nobody will actually check;)).