[issue30459] PyList_SET_ITEM could be safer

STINNER Victor added the comment: New changeset 51f8196d05f0e271358eee0f90fe044b20449fb5 by Victor Stinner in branch 'main': bpo-30459: Use (void) in macros setting variables (GH-28982) https://github.com/python/cpython/commit/51f8196d05f0e271358eee0f90fe044b20449fb5 --

[issue30459] PyList_SET_ITEM could be safer

Change by STINNER Victor : -- pull_requests: +27269 pull_request: https://github.com/python/cpython/pull/28982 ___ Python tracker ___

[issue30459] PyList_SET_ITEM could be safer

Christian Tismer added the comment: Congrats to that change! -- nosy: +Christian.Tismer ___ Python tracker ___ ___

[issue30459] PyList_SET_ITEM could be safer

STINNER Victor added the comment: > That's not true; at least ALSA's python bindings use PyTuple_SET_ITEM as a > lvalue as well. alsa-python used PyTuple_SET_ITEM(..., obj) to decide if it should call Py_INCREF(obj). This code looks suspicious. PyTuple_SET_ITEM() should not be used to set

[issue30459] PyList_SET_ITEM could be safer

Petr Viktorin added the comment: > The change respects the documentation which always documented the result type > as "void". Then, IMO, the documentation should be fixed. > My expectation is that apart py-qt4, no project abuse these 3 macros. That's not true; at least ALSA's python

[issue30459] PyList_SET_ITEM could be safer

STINNER Victor added the comment: I checked: commit 0ef96c2b2a291c9d2d9c0ba42bbc1900a21e65f3 is part of Python 3.10.0a3 (released 3 days ago). -- ___ Python tracker ___

[issue30459] PyList_SET_ITEM could be safer

STINNER Victor added the comment: > This change goes directly against PEP 387. The change respects the documentation which always documented the result type as "void". 3.10: https://docs.python.org/dev/c-api/tuple.html#c.PyTuple_SET_ITEM 3.5:

[issue30459] PyList_SET_ITEM could be safer

Petr Viktorin added the comment: This change goes directly against PEP 387. -- nosy: +petr.viktorin ___ Python tracker ___ ___

[issue30459] PyList_SET_ITEM could be safer

Change by STINNER Victor : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed versions: +Python 3.10 -Python 3.9 ___ Python tracker ___

[issue30459] PyList_SET_ITEM could be safer

STINNER Victor added the comment: Thanks Espie Marc for the bug report, it's now fixed in the master branch. IMO not only clang users will benefit of a better defined API. For example, it should help other Python implementation to implement such API, without the weird side effects of a

[issue30459] PyList_SET_ITEM could be safer

STINNER Victor added the comment: New changeset 0ef96c2b2a291c9d2d9c0ba42bbc1900a21e65f3 by Victor Stinner in branch 'master': bpo-30459: Cast the result of PyCell_SET to void (GH-23654) https://github.com/python/cpython/commit/0ef96c2b2a291c9d2d9c0ba42bbc1900a21e65f3 --

[issue30459] PyList_SET_ITEM could be safer

Change by STINNER Victor : -- pull_requests: +22523 pull_request: https://github.com/python/cpython/pull/23654 ___ Python tracker ___

[issue30459] PyList_SET_ITEM could be safer

STINNER Victor added the comment: New changeset 556d97f473fa538cef780f84bd29239ecf57d9c5 by Zackery Spytz in branch 'master': bpo-30459: Cast the result of PyList_SET_ITEM() to void (GH-19975) https://github.com/python/cpython/commit/556d97f473fa538cef780f84bd29239ecf57d9c5 --

[issue30459] PyList_SET_ITEM could be safer

Serhiy Storchaka added the comment: I prefer PR 19975 to PR 23645. It solves the initial issue and is much simpler. -- ___ Python tracker ___

[issue30459] PyList_SET_ITEM could be safer

Espie Marc added the comment: On Sat, Dec 05, 2020 at 01:28:33AM +, STINNER Victor wrote: > > STINNER Victor added the comment: > > I propose to merge my PR 23645 change right now. If it breaks too many C > extensions, we still have time before Python 3.10.0 final scheduled at >

[issue30459] PyList_SET_ITEM could be safer

STINNER Victor added the comment: I propose to merge my PR 23645 change right now. If it breaks too many C extensions, we still have time before Python 3.10.0 final scheduled at Monday, 2021-10-04, to revert the change. "if (obj == NULL || PyList_SET_ITEM (l, i, obj) < 0)" Well, this is

[issue30459] PyList_SET_ITEM could be safer

Change by STINNER Victor : -- pull_requests: +22513 pull_request: https://github.com/python/cpython/pull/23645 ___ Python tracker ___

[issue30459] PyList_SET_ITEM could be safer

Change by Zackery Spytz : -- versions: +Python 3.9 -Python 3.7 ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue30459] PyList_SET_ITEM could be safer

Change by Zackery Spytz : -- keywords: +patch nosy: +ZackerySpytz nosy_count: 5.0 -> 6.0 pull_requests: +19291 stage: -> patch review pull_request: https://github.com/python/cpython/pull/19975 ___ Python tracker

[issue30459] PyList_SET_ITEM could be safer

Espie Marc added the comment: it's still 100% safe as a macro since each parameter is not used more than once. only the return type is an issue. -- ___ Python tracker

[issue30459] PyList_SET_ITEM could be safer

Raymond Hettinger added the comment: I prefer to leave this as macro rather than assuming the compiler will heed the inline hint. -- ___ Python tracker

[issue30459] PyList_SET_ITEM could be safer

Stefan Krah added the comment: I guess since it's documented, it would be ok to change for 3.7. BTW, we also allow "static inline" now in headers, so perhaps we could make it a function. -- nosy: +skrah ___ Python tracker

[issue30459] PyList_SET_ITEM could be safer

Espie Marc added the comment: Note that the API is fully documented for returning void... not anything else. "No basis" right. We're taling 1 pieces of software. a lot of what is actually used in the world. I'm very surprised, considering python has routinely done "spring cleanup" by

[issue30459] PyList_SET_ITEM could be safer

Raymond Hettinger added the comment: > Well, there is not going to be a lot of breakage. This > problem is the only instance I've encountered in the > full OpenBSD ports tree. That is no basis for knowing what the entire rest of the world has done with done with this API (perhaps valid

[issue30459] PyList_SET_ITEM could be safer

Espie Marc added the comment: yep, casting to (void) would be safer indeed. didn't think of that one ;) -- ___ Python tracker ___

[issue30459] PyList_SET_ITEM could be safer

Serhiy Storchaka added the comment: I think it would be safer to just cast the result of the expression to void. This decreases the chance of breaking valid code, that for example uses PyList_SET_ITEM() in a comma expression. -- nosy: +haypo, serhiy.storchaka versions: +Python 3.7

[issue30459] PyList_SET_ITEM could be safer

Espie Marc added the comment: Well, there is not going to be a lot of breakage. This problem is the only instance I've encountered in the full OpenBSD ports tree. I thought python was supposed to be a clean language, and didn't shy away from removing stuff/tweaking stuff to achieve that goal

[issue30459] PyList_SET_ITEM could be safer

Raymond Hettinger added the comment: The docs do make the claim of returning void: https://docs.python.org/3/c-api/list.html#c.PyList_SET_ITEM However, I think we should change the docs rather than changing the macro. This macro is very old and very widely used. Changing it is likely to

[issue30459] PyList_SET_ITEM could be safer

New submission from Espie Marc: Documentation says PyList_SET_ITEM is void, but it lies. The macro is such that it yields the actual element being set. wrapping the macro content in a do {} while (0) makes sure PyList_SET_ITEM is really void, e.g.: #define PyList_SET_ITEM(op, i, v) do {