You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
For now (3.3 beta) PyCFunction_New defined as macro calling PyCFunction_NewEx.
To be compatible with PEP-384 (Defining a Stable ABI) Objects/methodobject.c has trampoline function named PyCFunction_New which calls PyCFunction_NewEx.
This is only single usage of this idiom in CPython code.
For sake of uniformity we need to:
Yes, you right. We use this idiom also for PyAST_CompileEx, PyErr_WarnEx and bunch of functions in ./Include/pythonrun.h
Patch is very simple and was available for review almost 3 months.
I assumed that developers looked on this and had no objections.
Sorry if I was wrong.
The change is not required. But, I think, it can be helpful to use direct function calls instead of macros, especially for functions which are part of Stable API.
Activity
asvetlov commentedon Jul 22, 2012
For now (3.3 beta) PyCFunction_New defined as macro calling PyCFunction_NewEx.
To be compatible with PEP-384 (Defining a Stable ABI) Objects/methodobject.c has trampoline function named PyCFunction_New which calls PyCFunction_NewEx.
This is only single usage of this idiom in CPython code.
For sake of uniformity we need to:
asvetlov commentedon Oct 6, 2012
Attached patch for the issue.
BTW PyCFunction_New/PyCFunction_NewEx are part of Stable ABI but never mentioned in the documentation.
asvetlov commentedon Dec 25, 2012
bpo-16776 created for documenting PyCFunction_New/PyCFunction_NewEx
python-dev commentedon Dec 25, 2012
New changeset 3a86a3f1d89a by Andrew Svetlov in branch 'default':
Issue bpo-15422: get rid of PyCFunction_New macro
http://hg.python.org/cpython/rev/3a86a3f1d89a
birkenfeld commentedon Dec 25, 2012
I don't think this is the only use of this particular idiom; I recall it is used every time we "amend" a function with an _Ex version.
Why was this change necessary?
birkenfeld commentedon Dec 25, 2012
BTW it would be good if you could have at least one other developer look at issues like this and get a "LGTM" vote before committing all by yourself.
asvetlov commentedon Dec 25, 2012
I assumed that developers looked on this and had no objections.
Sorry if I was wrong.
birkenfeld commentedon Dec 25, 2012
There is no silent acceptance. No comment means that nobody reviewed it, which is no surprise given the number of open issues :)
birkenfeld commentedon Dec 25, 2012
So given #1 and #3, I would recommend reverting the part of the patch that removes the macro. Changing caller sites in CPython sources is fine.
python-dev commentedon Dec 26, 2012
New changeset 6a56eaa5e5fb by Andrew Svetlov in branch 'default':
Revert back PyCFunction_New macro. Keep PyCFunction_NewEx usage in python core modules (bpo-15422)
http://hg.python.org/cpython/rev/6a56eaa5e5fb
asvetlov commentedon Dec 26, 2012
Georg, I've followed your instructions.
Close the issue again.
Thanks for mentorship.
python-dev commentedon Dec 26, 2012
New changeset 70ea05f762a1 by Andrew Svetlov in branch 'default':
Fix compilation error for bpo-15422
http://hg.python.org/cpython/rev/70ea05f762a1
python-dev commentedon Nov 11, 2013
New changeset 267ad2ed4138 by Andrew Kuchling in branch 'default':
bpo-15422: remove NEWS item for a change that was later reverted
http://hg.python.org/cpython/rev/267ad2ed4138
pitrou commentedon Apr 26, 2014
Regression in issue bpo-21354.