Uh oh!
There was an error while loading. Please reload this page.
gh-114329: Add PyList_GetItemRef function - #114504
Conversation
The new `PyList_GetItemRef` is similar to `PyList_GetItem`, but returns a strong reference instead of a borrowed reference. Additionally, if the passed "list" object is not a list, the function sets a `TypeError` instead of calling `PyErr_BadInternalCall()`.
vstinner
commented
Jan 30, 2024
When I proposed such change, @serhiy-storchaka asked me to stick to |
| :exc:`TypeError` exception. | ||
| This behaves like :c:func:`PyList_GetItem`, but returns a | ||
| :term:`strong reference` instead of a :term:`borrowed reference`. |
There was a problem hiding this comment.
I suggest to make this function the new reference, and document PyList_GetItem() as: "Similar to PyList_GetItemRef(), but return a borrowed reference".
| # CRASHES get_item(21, 2) | ||
| # CRASHES get_item(NULL, 1) | ||
| def test_list_get_item_ref(self): |
There was a problem hiding this comment.
Can you share most code between test_list_get_item() and test_list_get_item_ref()?
Uh oh!
There was an error while loading. Please reload this page.
| [function.PyList_GetItemRef] | ||
| added = '3.13' |
There was a problem hiding this comment.
Move it to the end of the file.
serhiy-storchaka
commented
Jan 30, 2024
Is it RuntimeError or SystemError? The only discussed difference was returning a strong reference instead of a weak reference. If we want to change also the exception type, it should be a separate discussion. Most concrete type C API raises SystemError if they are called with wrong type. It is considered a programming error, not a data error. |
vstinner
commented
Jan 30, 2024
I suggest to document that the function "parameter must be list" , without mentioning the exact exception type. |
colesbury
commented
Jan 30, 2024
The |
colesbury
commented
Jan 30, 2024
I've removed the sentence mentioning |
vstinner
left a comment
There was a problem hiding this comment.
LGTM.
The TypeError behavior is explicitly stated in the C API WG capi-workgroup/decisions#9 (comment). See also capi-workgroup/api-evolution#29 (comment) and PyWeakref_GetRef().
Oh ok, I didn't notice that TypeError was explicitly mentioned.
Uh oh!
There was an error while loading. Please reload this page.
corona10
left a comment
There was a problem hiding this comment.
LGTM too.
I will merge the PR if @serhiy-storchaka approve.
encukou
left a comment
There was a problem hiding this comment.
If the WG does discuss the exception type, it'll take some time.
Let's merge this, the type can be changed later
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
The new `PyList_GetItemRef` is similar to `PyList_GetItem`, but returns a strong reference instead of a borrowed reference. Additionally, if the passed "list" object is not a list, the function sets a `TypeError` instead of calling `PyErr_BadInternalCall()`.
The new `PyList_GetItemRef` is similar to `PyList_GetItem`, but returns a strong reference instead of a borrowed reference. Additionally, if the passed "list" object is not a list, the function sets a `TypeError` instead of calling `PyErr_BadInternalCall()`.
The new
PyList_GetItemRefis similar toPyList_GetItem, but returns a strong reference instead of a borrowed reference. Additionally, if the passed "list" object is not a list, the function sets aTypeErrorinstead of callingPyErr_BadInternalCall().PyList_GetItemRef, a variant ofPyList_GetItemthat returns a strong reference #114329📚 Documentation preview 📚: https://cpython-previews--114504.org.readthedocs.build/