◐ Shell
reader mode source ↗
Skip to content

bpo-35059: Convert _Py_Dealloc() to static inline function#10223

Merged
vstinner merged 2 commits into
python:masterfrom
vstinner:py_dealloc
Oct 30, 2018
Merged

bpo-35059: Convert _Py_Dealloc() to static inline function#10223
vstinner merged 2 commits into
python:masterfrom
vstinner:py_dealloc

Conversation

@vstinner

@vstinner vstinner commented Oct 29, 2018

Copy link
Copy Markdown
Member

Convert _Py_Dealloc() macro into a static inline function. Moreover,
it is now also defined as a static inline function if Py_TRACE_REFS
is defined.

https://bugs.python.org/issue35059

Convert _Py_Dealloc() macro into a static inline function. Moreover,
it is now also defined as a static inline function if Py_TRACE_REFS
is defined.
@vstinner

Copy link
Copy Markdown
Member Author

@methane: Py_INCREF and Py_DECREF have been converted to static inline functions. So I came back to _Py_Dealloc() as you asked me, and it seems like I found a solution to handle properly this one:

  • " PyAPI_FUNC(void) _Py_Dealloc(PyObject *); " is now always declared the same way with any C define and it is now always implemented in object.c at the same place (previously, there were two implementations, depending if Py_TRACE_REFS)
  • Name the inlined flavor "_Py_Dealloc_inline" and use #define _Py_Dealloc(op) _Py_Dealloc_inline(op), so the name doesn't conflict depending if Py_LIMITED_API is defined or not

@vstinner vstinner requested a review from benjaminp October 29, 2018 19:07
@vstinner vstinner merged commit 3c09dca into python:master Oct 30, 2018
@vstinner vstinner deleted the py_dealloc branch October 30, 2018 13:48
@vstinner

Copy link
Copy Markdown
Member Author

@serhiy-storchaka: You asked interesting questions, but I merged my change anyway.

For the coding style, if you want to change it: please write a separated PR to reformat all "static inline" at once, not only these ones.

For _Py_Dealloc() race condition: if you consider that it's a bug, please open a new issue. My commit doesn't change the code, it only moves the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants