gh-99574: Add Py_SETREF() to the limited C API

[vstinner]

Add Py_SETREF() and Py_XSETREF() macros to the limited C API.

• Issue: Add Py_SETREF() and Py_XSETREF() macros to the limited C API 3.12 #99574

[vstinner]

cc @encukou@serhiy-storchaka@erlend-aasland

[encukou]

This adds much more than Py_SETREF() and Py_XSETREF().
Could you add comments about all the added functions, especially those added for earlier versions? Preferably in a different issue/PR, so it's not misleading.

[vstinner]

This adds much more than Py_SETREF() and Py_XSETREF(). Could you add comments about all the added functions, especially those added for earlier versions? Preferably in a different issue/PR, so it's not misleading.

In terms of API, only Py_SETREF() and Py_XSETREF() are added to the limited C API. But while I was updating Misc/stable_abi.toml, I noticed that many macros were already part of the limited C API but not documented as being part of it.

[vstinner]

I updated the commit message (and PR description) to mention that the change also adds macros missing in Misc/stable_abi.toml but already part of the limited C API.

[encukou]

Py_LIMITED_API does not define what the limited API is.
Please allow more discussion on the retroactive additions. And add comments in the source.

[vstinner]

Py_LIMITED_API does not define what the limited API is.

I don't understand that. Is there a definition of what is the limited C API in this case? What is Py_LIMITED_API if it doesn't define the limited C API?

[encukou]

Misc/stable_abi.toml defines it. Py_LIMITED_API should follow Misc/stable_abi.toml, but that's not always possible (and there might be bugs).
We can decide the definition is wrong and change it retroactively, but that should be a deliberate change.

[encukou]

I really don't think we want to add these.

[vstinner]

Misc/stable_abi.toml defines it. Py_LIMITED_API should follow Misc/stable_abi.toml, but that's not always possible (and there might be bugs).

Ok, now I'm confused. Py_LIMITED_API is documented as:

Define this macro before including Python.h to opt in to only use the Limited API

When I define this macro, the Py_TYPE() macro is available. But Py_TYPE() is not part of Misc/stable_abi.toml. So something is wrong: either Py_TYPE() should be added to Misc/stable_abi.toml, to Py_TYPE() must not defined if Py_LIMITED_API is defined.

For example module Modules/xxlimited.c uses Py_TYPE():

static int Xxo_traverse(PyObject *self_obj, visitproc visit, void *arg){// Visit the type Py_VISIT(Py_TYPE(self_obj)); ... } 

This code is even required to define heap types:

Heap-allocated types are expected to visit Py_TYPE(self) in tp_traverse.

This is just one example. I'm trying to understand what is the "limited C API" "according to you" :-) (Sadly, it seems like different people have a different definition.)

[vstinner]

[macro.Py_REFCNT]
[macro.Py_SET_REFCNT]
I really don't think we want to add these.

PEP 384 explicitly gives access to object type and reference count, by giving access to PyObject.ob_refcnt and PyObject.ob_type. But accessing directly structure members is bad for future Python evolutions. The "nogil" project is one concrete example: see the nogil section of PEP 674.

IMO using an abstraction, Py_REFCNT() and Py_SET_REFCNT(), is better than giving access to the structure members. IMO these macros are already part of the limited C API, since they are accessible with the Py_LIMITED_API macro is defined, but it seems like we have a different view on what is the limited C API.

[serhiy-storchaka]

Please add

#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030C0000 

[encukou]

Ok, now I'm confused. Py_LIMITED_API is documented as:

Define this macro before including Python.h to opt in to only use the Limited API

That's the intention. But, there are always bugs -- wrong Py_LIMITED_API guards, or wrong Misc/stable_abi.toml. And there are technical issues, like exposing underscored names.

What makes Py_LIMITED_API work badly as the definition is that you can't easily check what it exposes on all the different builds/platforms.

When I define this macro, the Py_TYPE() macro is available. But Py_TYPE() is not part of Misc/stable_abi.toml. So something is wrong: either Py_TYPE() should be added to Misc/stable_abi.toml, to Py_TYPE() must not defined if Py_LIMITED_API is defined.

In this case, it looks like Misc/stable_abi.toml is wrong. But in that case, shouldn't the fix be backported to all the supported versions?
It would work better as a separate PR.

This is just one example. I'm trying to understand what is the "limited C API" "according to you" :-) (Sadly, it seems like different people have a different definition.)

See Misc/stable_abi.toml. The file exists to give a simple answer :)
Unfortunately, you can find bugs in Misc/stable_abi.toml, as in any other source.

IMO using an abstraction, Py_REFCNT() and Py_SET_REFCNT(), is better than giving access to the structure members. IMO these macros are already part of the limited C API, since they are accessible with the Py_LIMITED_API macro is defined, but it seems like we have a different view on what is the limited C API.

I see as Misc/stable_abi.toml is the definition. If it's buggy, we should fix it. (We need to be careful that the fix can't break existing code, but for macros that Py_LIMITED_API always allowed, and that work with stable ABI, that shouldn't be an issue.)
A bugfix should be backported.

[vstinner]

I rewrote this PR to restrict it to Py_SETREF() and Py_XSETREF(). I added #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030C0000 as asked by @serhiy-storchaka.

I now agree that adding other macros to Misc/stable_abi.toml should be discussed in separated issues/PRs.

[vstinner]

Once this PR will be merged, I will open separated issues/PRs for other discussed macros:

• Py_CLEAR()
• Py_DECREF(), Py_XDECREF()
• Py_INCREF(), Py_XINCREF()
• Py_REFCNT(), Py_SET_REFCNT()

[vstinner]

PR rebased on main to fix a conflict on Misc/stable_abi.toml.

[netlify]

✅ Deploy Preview for python-cpython-preview ready!

Name	Link
🔨 Latest commit	bf7dc14
🔍 Latest deploy log	https://app.netlify.com/sites/python-cpython-preview/deploys/6390a2a93409ff0008adde75
😎 Deploy Preview	https://deploy-preview-99575--python-cpython-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[vstinner]

I fixed a type punning bug which caused Python to be miscompiled: commit b11a384.

I rebased this PR on top of it.

[vstinner]

I fixed a type punning bug which caused Python to be miscompiled: commit b11a384.

Oh. The new Py_CLEAR() implementation uses memcpy() of <string.h>, whereas the limited C API <Python.h> no longer includes <string.h> (since Python 3.11). I proposed PR #100121 to fix Py_CLEAR().