gh-101233: Add part to explain Py_SETREF and Py_NewRef
#101234
Conversation
|
|
||
| if (last) { | ||
| Py_XSETREF(self->last, Py_NewRef(last)); | ||
| } |
There was a problem hiding this comment.
Why not suggesting to use these functions at the first place (in the code above)? I'm not sure that this tutorial is the best place to explain how to handle reference counting in length.
There was a problem hiding this comment.
I think there should be a part for Py_SETREF/Py_XSETREF and Py_NewRef/Py_XNewRef in the reference counting part earlier in the tutorial and Py_SETREF/Py_XSETREF in the reference counting section since it isn't documented anywhere else yet. That way, this tutorial could just use Py_XSETREF and Py_NewRef instead of the 4-line snippet with no further explanation needed. I'll probably do that later if you agree with it.
|
I added Py_NewRef() to Python 3.10. For me, it's easier to understand its semantics than Py_INCREF() which changes the reference count in-place. Recently, I worked on fixing the Py_SETREF() and Py_CLEAR() macros and I documented Py_SETREF(): https://docs.python.org/dev/c-api/refcounting.html#c.Py_SETREF I modified CPython source code to use Py_CLEAR() and Py_SETREF() where appropriate: see PRs related to the issue GH-99537. Problem: my Py_SETREF() and Py_CLEAR() fix is causing troubles and might get reverted: see issue GH-98724 and https://discuss.python.org/t/c-api-design-goal-hide-implementation-details-convert-macros-to-regular-functions-avoid-memset-or-errno-in-header-files-etc/25137 For now, I prefer to hold the work on these macros. But my plan is to advertize Py_SETREF() and Py_CLEAR() more widely since they prevent race conditions which are hard to notice when reading/auditing code, and hard to debug. |
|
Got it. |
|
The following commit authors need to sign the Contributor License Agreement: |
Add the part right below the "why do we have to do this?" part explaining the correct way of reassigning to an object/attribute.
Py_SETREFandPy_NewRefis not explained in extending new types using a C extension #101233