Skip to content
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

C API: Consider adding a public PyList_Extend() function #111138

Closed
vstinner opened this issue Oct 20, 2023 · 8 comments
Closed

C API: Consider adding a public PyList_Extend() function #111138

vstinner opened this issue Oct 20, 2023 · 8 comments
Labels
topic-C-API type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

vstinner commented Oct 20, 2023

Feature or enhancement

The private _PyList_Extend() function has been removed in Python 3.13: see PR #108451.

@scoder asked what is the replacement for this removed function.

The obvious replacement is PyObject_CallMethod(list, "extend", "O", arg): call the list.extend() method. But it's slower, PyObject_CallMethod() has to get the method and decode the "O" format string.

Another replacement is PyList_SetSlice(L, PY_SSIZE_T_MAX, PY_SSIZE_T_MAX, arg) which is less straightforward, but it's efficient.

I propose adding a public PyList_Extend() function. The list type is commonly used in C extensions, it's a convenient API to create a collection when the length is not known is advance.

I don't think that performance is really the problem here since PyList_SetSlice(L, PY_SSIZE_T_MAX, PY_SSIZE_T_MAX, arg) is available. The problem is more to more the function easier to discover and make the API easier to use. Also, it should help people to migrate away from the removed function.

There is already a public PyDict_Update() API for the dict type, it is part of the limited C API.

If we add a function for the list type, should we also add PySet_Update() "for completeness"? The private _PySet_Update() was removed in Python 3.13.

cc @serhiy-storchaka @encukou


A code search on _PyList_Extend in PyPI top 5,000 projects (2023-07-04) found 4 projects using this function:

  • Cython (0.29.36)
  • catboost (1.2)
  • mypy (1.4.1)
  • pyrsistent (0.19.3)

Logs:

PYPI-2023-07-04/catboost-1.2.tar.gz: catboost-1.2/catboost_all_src/contrib/tools/cython/Cython/Compiler/Builtin.py: BuiltinMethod("extend",  "TO",   "r", "__Pyx_PyList_Extend",
PYPI-2023-07-04/catboost-1.2.tar.gz: catboost-1.2/catboost_all_src/contrib/tools/cython/Cython/Compiler/ExprNodes.py: extend_func = "__Pyx_PyList_Extend"
PYPI-2023-07-04/catboost-1.2.tar.gz: catboost-1.2/catboost_all_src/contrib/tools/cython/Cython/Utility/Optimize.c: static CYTHON_INLINE int __Pyx_PyList_Extend(PyObject* L, PyObject* v) {
PYPI-2023-07-04/catboost-1.2.tar.gz: catboost-1.2/catboost_all_src/contrib/tools/cython/Cython/Utility/Optimize.c: PyObject* none = _PyList_Extend((PyListObject*)L, v);

PYPI-2023-07-04/Cython-0.29.36.tar.gz: Cython-0.29.36/Cython/Compiler/Builtin.py: BuiltinMethod("extend",  "TO",   "r", "__Pyx_PyList_Extend",
PYPI-2023-07-04/Cython-0.29.36.tar.gz: Cython-0.29.36/Cython/Compiler/ExprNodes.py: extend_func = "__Pyx_PyList_Extend"
PYPI-2023-07-04/Cython-0.29.36.tar.gz: Cython-0.29.36/Cython/Utility/Optimize.c: static CYTHON_INLINE int __Pyx_PyList_Extend(PyObject* L, PyObject* v) {
PYPI-2023-07-04/Cython-0.29.36.tar.gz: Cython-0.29.36/Cython/Utility/Optimize.c: PyObject* none = _PyList_Extend((PyListObject*)L, v);

PYPI-2023-07-04/mypy-1.4.1.tar.gz: mypy-1.4.1/mypyc/lib-rt/dict_ops.c: PyObject *res = _PyList_Extend((PyListObject *)list, view);
PYPI-2023-07-04/mypy-1.4.1.tar.gz: mypy-1.4.1/mypyc/lib-rt/dict_ops.c: PyObject *res = _PyList_Extend((PyListObject *)list, view);
PYPI-2023-07-04/mypy-1.4.1.tar.gz: mypy-1.4.1/mypyc/lib-rt/dict_ops.c: PyObject *res = _PyList_Extend((PyListObject *)list, view);
PYPI-2023-07-04/mypy-1.4.1.tar.gz: mypy-1.4.1/mypyc/lib-rt/list_ops.c: return _PyList_Extend((PyListObject *)o1, o2);

PYPI-2023-07-04/pyrsistent-0.19.3.tar.gz: pyrsistent-0.19.3/pvectorcmodule.c: PyObject *retVal = _PyList_Extend((PyListObject *)self->appendList, args);

Cython uses the function to implement its own __Pyx_PyList_Extend() API:

static CYTHON_INLINE int __Pyx_PyList_Extend(PyObject* L, PyObject* v) {
#if CYTHON_COMPILING_IN_CPYTHON
    PyObject* none = _PyList_Extend((PyListObject*)L, v);
    if (unlikely(!none))
        return -1;
    Py_DECREF(none);
    return 0;
#else
    return PyList_SetSlice(L, PY_SSIZE_T_MAX, PY_SSIZE_T_MAX, v);
#endif
}

Linked PRs

@vstinner vstinner added type-feature A feature request or enhancement topic-C-API labels Oct 20, 2023
@vstinner vstinner changed the title C API: Consider adding a public PyList_Extend() function to replace removed private _PyList_Extend() C API: Consider adding a public PyList_Extend() function Oct 20, 2023
@serhiy-storchaka
Copy link
Member

PyDict_Update() is not trivial. PyList_Extend() is trivial, literally one-liner.

#define PyList_Extend(list, arg) PyList_SetSlice((list), PY_SSIZE_T_MAX, PY_SSIZE_T_MAX, (arg))

I don't mind adding it as a macro. Together with PyList_Clear().

#define PyList_Clear(list) PyList_SetSlice((list), 0, PY_SSIZE_T_MAX, NULL)

musicinmybrain added a commit to musicinmybrain/pyrsistent that referenced this issue Oct 26, 2023
This private function is no longer exported in Python 3.13.

It is possible that a PyList_Extend() function-like macro may be added
before Python 3.13 final, but using PyList_SetSlice() directly will
still work.

python/cpython#108451

python/cpython#111138
@musicinmybrain
Copy link

Just noting that the proposed PyList_Extend macro is not an exact drop-in replacement for the removed _PyList_Extend() function, since the necessary error-checking is a bit different: _PyList_Extend() returns PyObject *, but PyList_SetSlice() returns int.

musicinmybrain added a commit to musicinmybrain/pyrsistent that referenced this issue Oct 26, 2023
This private function is no longer exported in Python 3.13.

It is possible that a PyList_Extend() function-like macro may be added
before Python 3.13 final, but using PyList_SetSlice() directly will
still work.

python/cpython#108451

python/cpython#111138
@vstinner
Copy link
Member Author

#define PyList_Extend(list, arg) ...

I dislike macros: they cannot be used in programming languages other than C, and in projects which only load libpython symbols (ex: vim text editor). See for example PEP 670 – Convert macros to functions in the Python C API.

@vstinner
Copy link
Member Author

_PyList_Extend() returns PyObject *

For a method used in Python, it makes sense. For a C API, it looks inefficient to return None on success.

@musicinmybrain
Copy link

For a method used in Python, it makes sense. For a C API, it looks inefficient to return None on success.

I agree, and I think returning int is the correct signature for a C API.

It’s just worth pointing out that extension authors porting to Python 3.13 will need to be aware that they cannot simply search-and-replace _PyList_Extend with the proposed PyList_Extend, despite the similar names. tobgu/pyrsistent#284 is a good example.

vstinner added a commit to vstinner/cpython that referenced this issue Oct 31, 2023
Test the public PyList C API.
vstinner added a commit to vstinner/cpython that referenced this issue Nov 1, 2023
Test the public PyList C API.
vstinner added a commit to vstinner/cpython that referenced this issue Nov 1, 2023
Test the public PyList C API.
vstinner added a commit to vstinner/cpython that referenced this issue Nov 1, 2023
Add test_capi.test_list tests.
@vstinner
Copy link
Member Author

vstinner commented Nov 1, 2023

First step: I wrote PR #111582 to test the existing public PyList C API.

vstinner added a commit to vstinner/cpython that referenced this issue Nov 1, 2023
Add test_capi.test_list tests.
vstinner added a commit to vstinner/cpython that referenced this issue Nov 8, 2023
Add test_capi.test_list tests.
vstinner added a commit to vstinner/cpython that referenced this issue Nov 8, 2023
vstinner added a commit to vstinner/cpython that referenced this issue Nov 8, 2023
vstinner added a commit to vstinner/cpython that referenced this issue Nov 8, 2023
* Split list_extend() into two sub-functions: list_extend_fast() and
  list_extend_iter().
* list_inplace_concat() no longer has to call Py_DECREF() on the
  list_extend() result, since list_extend() now returns an int.
@vstinner
Copy link
Member Author

vstinner commented Nov 8, 2023

PR gh-111562 added tests on the PyList C API.

I wrote PR gh-111862 to add PyList_Extend() and PyList_Clear() to the public C API.

vstinner added a commit to vstinner/cpython that referenced this issue Nov 9, 2023
* Split list_extend() into two sub-functions: list_extend_fast() and
  list_extend_iter().
* list_inplace_concat() no longer has to call Py_DECREF() on the
  list_extend() result, since list_extend() now returns an int.
vstinner added a commit to vstinner/cpython that referenced this issue Nov 13, 2023
* Split list_extend() into two sub-functions: list_extend_fast() and
  list_extend_iter().
* list_inplace_concat() no longer has to call Py_DECREF() on the
  list_extend() result, since list_extend() now returns an int.
vstinner added a commit that referenced this issue Nov 13, 2023
* Split list_extend() into two sub-functions: list_extend_fast() and
  list_extend_iter().
* list_inplace_concat() no longer has to call Py_DECREF() on the
  list_extend() result, since list_extend() now returns an int.
@vstinner
Copy link
Member Author

I close the issue.

Commit babb787 adds PyList_Extend() and PyList_Clear(). I wrote python/pythoncapi-compat#80 to add these functions to pythoncapi-compat.

@serhiy-storchaka's macros can also be used on Python 3.12 and older:

#define PyList_Extend(list, arg) PyList_SetSlice((list), PY_SSIZE_T_MAX, PY_SSIZE_T_MAX, (arg))
#define PyList_Clear(list) PyList_SetSlice((list), 0, PY_SSIZE_T_MAX, NULL)

If we add a function for the list type, should we also add PySet_Update() "for completeness"? The private _PySet_Update() was removed in Python 3.13.

I didn't add this function.

aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
…thon#111862)

* Split list_extend() into two sub-functions: list_extend_fast() and
  list_extend_iter().
* list_inplace_concat() no longer has to call Py_DECREF() on the
  list_extend() result, since list_extend() now returns an int.
hauntsaninja pushed a commit to python/mypy that referenced this issue Jul 7, 2024
Replace `_PyList_Extend` with `PyList_Extend` from
`pythoncapi_compat.h`.

python/cpython#111138
https://docs.python.org/dev/c-api/list.html#c.PyList_Extend

Fixes
```cpp
  /home/runner/work/mypy/mypy/mypyc/lib-rt/list_ops.c: In function ‘CPyList_Extend’: (diff)
  /home/runner/work/mypy/mypy/mypyc/lib-rt/list_ops.c:259:12: error: implicit declaration of function ‘_PyList_Extend’; did you mean ‘CPyList_Extend’? [-Werror=implicit-function-declaration] (diff)
    259 |     return _PyList_Extend((PyListObject *)o1, o2); (diff)
        |            ^~~~~~~~~~~~~~ (diff)
        |            CPyList_Extend (diff)
  /home/runner/work/mypy/mypy/mypyc/lib-rt/list_ops.c:259:12: error: returning ‘int’ from a function with return type ‘PyObject *’ {aka ‘struct _object *’} makes pointer from integer without a cast [-Werror=int-conversion] (diff)
    259 |     return _PyList_Extend((PyListObject *)o1, o2); (diff)
        |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ (diff)
  /home/runner/work/mypy/mypy/mypyc/lib-rt/dict_ops.c: In function ‘CPyDict_Keys’: (diff)
  /home/runner/work/mypy/mypy/mypyc/lib-rt/dict_ops.c:233:21: error: initialization of ‘PyObject *’ {aka ‘struct _object *’} from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion] (diff)
    233 |     PyObject *res = _PyList_Extend((PyListObject *)list, view); (diff)
        |                     ^~~~~~~~~~~~~~ (diff)
  /home/runner/work/mypy/mypy/mypyc/lib-rt/dict_ops.c: In function ‘CPyDict_Values’: (diff)
  /home/runner/work/mypy/mypy/mypyc/lib-rt/dict_ops.c:253:21: error: initialization of ‘PyObject *’ {aka ‘struct _object *’} from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion] (diff)
    253 |     PyObject *res = _PyList_Extend((PyListObject *)list, view); (diff)
        |                     ^~~~~~~~~~~~~~ (diff)
  /home/runner/work/mypy/mypy/mypyc/lib-rt/dict_ops.c: In function ‘CPyDict_Items’: (diff)
  /home/runner/work/mypy/mypy/mypyc/lib-rt/dict_ops.c:273:21: error: initialization of ‘PyObject *’ {aka ‘struct _object *’} from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion] (diff)
    273 |     PyObject *res = _PyList_Extend((PyListObject *)list, view); (diff)
        |                     ^~~~~~~~~~~~~~ (diff)
```
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…thon#111862)

* Split list_extend() into two sub-functions: list_extend_fast() and
  list_extend_iter().
* list_inplace_concat() no longer has to call Py_DECREF() on the
  list_extend() result, since list_extend() now returns an int.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants