I looked at the x86-64 machine code of test_py_clear() built by gcc -O3. I don't see any PyObject** type anymore, GCC works directly on pointers (PyObject*): inefficient was removed by the compiler, as expected.

(gdb) disassemble test_py_clear
Dump of assembler code for function test_py_clear:
   0x00007fffea27ba90 <+0>:	sub    rsp,0x8

   # obj = PyList_New(0);
   0x00007fffea27ba94 <+4>:	xor    edi,edi
   0x00007fffea27ba96 <+6>:	call   0x7fffea279080 <PyList_New@plt>

   # if (!obj) return NULL;
   0x00007fffea27ba9b <+11>:	test   rax,rax
   0x00007fffea27ba9e <+14>:	je     0x7fffea27bab1 <test_py_clear+33>

   # Py_DECREF(obj);
   0x00007fffea27baa0 <+16>:	sub    QWORD PTR [rax],0x1
   0x00007fffea27baa4 <+20>:	je     0x7fffea27bac0 <test_py_clear+48>

   # Py_RETURN_NONE;
   0x00007fffea27baa6 <+22>:	mov    rax,QWORD PTR [rip+0x224fb]        # 0x7fffea29dfa8
   0x00007fffea27baad <+29>:	add    QWORD PTR [rax],0x1
   0x00007fffea27bab1 <+33>:	add    rsp,0x8
   0x00007fffea27bab5 <+37>:	ret    

   # _Py_Dealloc()
   0x00007fffea27bab6 <+38>:	cs nop WORD PTR [rax+rax*1+0x0]
   0x00007fffea27bac0 <+48>:	mov    rdi,rax
   0x00007fffea27bac3 <+51>:	call   0x7fffea279400 <_Py_Dealloc@plt>
   0x00007fffea27bac8 <+56>:	jmp    0x7fffea27baa6 <test_py_clear+22>
End of assembler dump.

I completed test_py_clear() to test calling Py_CLEAR() with a side effect: test based on #98724 (comment) idea.

I have some suggestion for the test: It should crash with the old Py_CLEAR version so you do not get any regressions.

I have some suggestion for the test: It should crash with the old Py_CLEAR version so you do not get any regressions.

It does crash with the old Py_CLEAR() macro:

$ ./python -m test -v test_capi  -m test_py_clear 
(...)
test_py_clear (test.test_capi.Test_testcapi.test_py_clear) ...
Fatal Python error: Segmentation fault
(...)
Erreur de segmentation (core dumped)

cc @serhiy-storchaka: there is now a unit test which shows that the old macro had a bug.

Py_SETREF?

Py_SETREF?

I don't understand your comment. This PR fix a bug in Py_CLEAR(). Can you please elaborate?

Py_SETREF and Py_XSETREF are similar to Py_CLEAR. If change the latter, why not change the formers?

I think all macros should be changed so they only reference their arguments once. This avoids the whole issue with side effects applied multiple times.

I think all macros should be changed so they only reference their arguments once. This avoids the whole issue with side effects applied multiple times.

That's one of the purpose of PEP 670. It's just that @erlend-aasland and me missed these macros (Py_CLEAR, Py_SETREF, Py_XSETREF).

Py_SETREF and Py_XSETREF are similar to Py_CLEAR. If change the latter, why not change the formers?

You're right. I completed my PR to fix also Py_SETREF() and Py_XSETREF() macros.

Py_SETREF() and Py_XSETREF() macros require an additional _PyObject_CAST() since the l-value type becomes a PyObject*, whereas previously it had the type of the op argument:

PyObject **_py_tmp_ptr = _Py_CAST(PyObject**, &(op));
...
(*_py_tmp_ptr) = _PyObject_CAST(op2);  // <==== HERE

I added unit tests.

I would document it as a change in 3.12. And older versions need documenting that the first argument can be evaluated more than once.

I would document it as a change in 3.12.

Ok, I documented the Py_CLEAR() change with a "versionchanged".

But Py_SETREF() and Py_XSETREF() are not documented. Maybe it's no purpose, so I didn't modify their (non existing) documentation.

I would consider adding a What's New item.

I would consider adding a What's New item.

Ok, I added a What's New in Python 3.12 entry. I copied the same text everywhere.

Would you mind to review the update documentation text?

Doc/c-api/refcounting.rst contains two times an error: If the argument has side effects, there are no longer duplicated. Further down it is correct with these are no longer duplicated.

Doc/c-api/refcounting.rst contains two times an error: If the argument has side effects, there are no longer duplicated. Further down it is correct with these are no longer duplicated.

Ah, I hesitated between both :-) I changed for "these are".

Py_SETREF() and Py_XSETREF() are intentionally not documented because they are not in the public API.

Unfortunately, anything that's included by Python.h is de facto part of the public C API, documented or not.

GH-99288 is a backport of this pull request to the 3.11 branch.

I created a 3.11 backport without documentation changes: #99288

I created a 3.11 backport without documentation changes: #99288

But that will invalidate the versionchanged in the docs...?

But that will invalidate the versionchanged in the docs...?

I fixed the bug silently, but I cannot document that it's the default behavior in Python 3.11.0 since the fix will only be done in later 3.11.x bugfix versions.

Hum. I read again the discussion and now I'm confused.

@serhiy-storchaka @erlend-aasland: Should I backport the fix to 3.10 and 3.11 or not?

I just backported the change to 3.11 as: 1082890

I created backport to 3.10 but then I closed it before it got merged: #99292

My idea is to fix the bug in Python 3.10 and 3.11 with a changelog entry, but don't document the behavior in the C API documentation with versionchanged. Is it a bad thing to fix this bug in 3.10 and 3.11?

People who need to use Py_CLEAR(), Py_SETREF() or Py_XSETREF() with side effects on old Python versions can redefine the macros with a flavor which has the bugfix, no?

I created python/pythoncapi-compat#50 to fix the two macros in the pythoncapi-compat project.

I proposed to add them to the stable API. But there were objections.

https://mail.python.org/archives/list/python-dev@python.org/thread/HGBT2Y4VAZOOU5XI2RPSURDGFBMRJKPO/

If you want to reconsider this, please open a new discussion.

By the way, Python 3.12 contains an important change related to memory allocation of objects tracked by the GC:

The Garbage Collector now runs only on the eval breaker mechanism of the Python bytecode evaluation loop instead on object allocations. The GC can also run when PyErr_CheckSignals() is called so C extensions that need to run for a long time without executing any Python code also have a chance to execute the GC periodically. (Contributed by Pablo Galindo in gh-97922.)

See #97922 for details. The change was done in _PyObject_GC_New() and _PyObject_GC_NewVar(), not on Py_DECREF().

See also issue #89060 and @nascheme's PR #27738 "WIP: Integrate trashcan into _Py_Dealloc". But again, this WIP change is only for objects tracked by the GC.

See also issue #99300: "Replace Py_INCREF()/Py_XINCREF() usage with Py_NewRef()/Py_XNewRef()", but it's not directly related :-)

If you want to reconsider this, please open a new discussion.

Is adding Py_SETREF() to the limited C API (stable ABI) related to deciding if the Py_CLEAR() and Py_SETREF() bugfix should be backported to Python 3.11? I'm not sure that I get it.

Functions excluded from the limited C API are also documented.

Well, I'm open to add Py_SETREF() to the limited C API, but I suggest to discuss it separately.

I am in favor of adding these functions to the limited C API, but since this idea was rejected once, you cannot do this without a broader discussion. How do you document that they are not a part of the limited C API?

Adding these functions to the Limited API should be discussed in a separate issue. It is out of scope for gh-98724, and this PR is definitely not the correct place to discuss it.

Regarding the backports, which should be discussed here (or on the linked issue): if we are not to update the docs in the backports, I don't think the backports should be made. It is unfortunate if the docs are inaccurate because of the backports.

Regarding the backports, which should be discussed here (or on the linked issue): if we are not to update the docs in the backports, I don't think the backports should be made.

What if I put a "versionchanged" with Python 3.11.x and Python 3.10.x bugfix versions, rather than "3.11" and "3.10"?

What if I put a "versionchanged" with Python 3.11.x and Python 3.10.x bugfix versions, rather than "3.11" and "3.10"?

I'm not sure how to handle that correctly in Sphinx. In order to be accurate, the versionchanged in main should include both 3.10.x, 3.11.1, and 3.12, so the rendered version should read "Changed in version 3.10.x, 3.11.1, and 3.12". Is that even possible?

I just feel uncomfortable that these macros were documented without discussion. What is they official status?

I just feel uncomfortable that these macros were documented without discussion. What is they official status?

I don't understand your point about the fact that functions which are excluded from the limited C API should not be documented.

Most of the C API is excluded from the limited C API and it is documented. For example, https://docs.python.org/dev/c-api/frame.html documents many PyFrame functions, PyFrame_GetBack() is documented and excluded from the limited C API. Only some functions have the "Part of the Stable ABI" label (see this Frame C API doc).

By the way, many C API macros are documented.

What is they official status?

Py_CLEAR(), Py_SETREF(), Py_XSETREF() are part of the public C API, are tested and documented in Python 3.12. How is it an issue? Sorry, I still don't get your point.

I'm not sure how to handle that correctly in Sphinx. In order to be accurate, the versionchanged in main should include both 3.10.x, 3.11.1, and 3.12, so the rendered version should read "Changed in version 3.10.x, 3.11.1, and 3.12". Is that even possible?

That's problematic in practice if 3.10 or 3.11 get a special release just for a single security fix, not based on the current 3.10 and 3.11 branch. Well, I don't recall that it ever happened. But I would prefer to not attempt to which today what will be the 3.x.y release in 3.10, 3.11 and main branches which will include the change. I prefer to only put 3.10.x in 3.10 doc, 3.11.x in the 3.11 doc, and 3.12 in the main branch. We did that multiple times in the past for security fixes. See also:

• https://docs.python.org/dev/whatsnew/3.9.html#notable-changes-in-python-3-9-2
• https://docs.python.org/dev/whatsnew/3.8.html#notable-changes-in-python-3-8-1
• https://docs.python.org/dev/whatsnew/3.7.html#notable-changes-in-python-3-7-1
• https://docs.python.org/dev/whatsnew/3.6.html#notable-changes-in-python-3-6-4

I'm not suggesting to document Py_SETREF() change as a "notable change".

I would prefer to not even document it in 3.10 and 3.11 branches. I don't get why you guys consider that Py_SETREF() is special enough to require that the bugfix is documented. But I'm fine with document it if you prefer.