-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-98724: Fix the Py_CLEAR() macro in the limited C API #100121
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
Conversation
✅ Deploy Preview for python-cpython-preview canceled.
|
Oh, I forgot that Py_CLEAR() is already part of the limited C API and the limited C API no longer includes <string.h> since Python 3.11. I already proposed a few times to provide Py_SETREF() and Py_CLEAR() as function calls. Well, here is my attempt to introduce a function implementation of the Py_CLEAR() macro :-) If Py_SETREF() and Py_XSETREF() macros are added to the limited C API (PR #99575), they will need a similar function implementation. I would really prefer hiding the implementation details and avoiding to include <string.h> in the limited C API <Python.h>. |
In this PR, I chose not use the function implementation if the |
If _Py_TYPEOF() is not available, the limited C API now implements the Py_CLEAR() macro as a function call which hides implementation details. The function uses memcpy() of <string.h> which is not included by <Python.h>. * Add private _Py_Clear() function. * Add _Py_Clear() to Misc/stable_abi.toml.
I suggest to not fix what was not broken. Py_CLEAR and Py_SETREF were macros, and worked pretty well as macros. Problems started when you converted them to functions. |
I'm not sure what you are talking about, the issue #98724 described a bug that was fixed in Python 3.12.
Py_CLEAR() and Py_SETREF() are currently implemented as macro. What makes you think that they became functions? This change is specific to the limited API. |
I do not think that it is a bug. The documentation can be improved, if it confuses somebody. If Py_CLEAR is a macro, no need to introduce _Py_Clear. It is a prelimilary pessimization. |
Right now, calling Py_CLEAR() in a C extension using the limited C API fails because memcpy() is not defined. That's the problem I'm trying to solve.
About the issue #98724 : yes, it would be possible to document that Py_CLEAR() and Py_SETREF() have bugs and there are ways to work around them. I chose to fix the bug instead. I prefer to provide an API which is not error prone. I would like to promote Py_CLEAR() and Py_SETREF() "functions" (macros), they are preventing race conditions and so more code should use them. But I would not be comfortable to have to explain that they have bugs. Fixing that bugs makes me me comfortable to add these functions to the limited C API. |
I do not think there was a bug. By fixing a non-bug you introduced a bug which you are trying to fix now. |
If _Py_TYPEOF() is not available, the limited C API now implements the Py_CLEAR() macro as a function call which hides implementation details. The function uses memcpy() of <string.h> which is not included by <Python.h>.