msg166138 - (view) |
Author: Andrew Svetlov (asvetlov) *  |
Date: 2012-07-22 14:33 |
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: - remove PyCFunction_New macro from Include/methodobject.h - declare PyCFunction_New as function in Include/methodobject.h - replace all calls of PyCFunction_New to PyCFunction_NewEx in code (about 8 occurrences). |
|
|
msg172196 - (view) |
Author: Andrew Svetlov (asvetlov) *  |
Date: 2012-10-06 13:41 |
Attached patch for the issue. BTW PyCFunction_New/PyCFunction_NewEx are part of Stable ABI but never mentioned in the documentation. |
|
|
msg178116 - (view) |
Author: Andrew Svetlov (asvetlov) *  |
Date: 2012-12-25 11:16 |
#16776 created for documenting PyCFunction_New/PyCFunction_NewEx |
|
|
msg178117 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-12-25 11:32 |
New changeset 3a86a3f1d89a by Andrew Svetlov in branch 'default': Issue #15422: get rid of PyCFunction_New macro http://hg.python.org/cpython/rev/3a86a3f1d89a |
|
|
msg178132 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2012-12-25 14:58 |
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? |
|
|
msg178134 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2012-12-25 14:59 |
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. |
|
|
msg178139 - (view) |
Author: Andrew Svetlov (asvetlov) *  |
Date: 2012-12-25 15:17 |
1. Yes, you right. We use this idiom also for PyAST_CompileEx, PyErr_WarnEx and bunch of functions in ./Include/pythonrun.h 2. 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. 3. 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. |
|
|
msg178157 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2012-12-25 17:13 |
There is no silent acceptance. No comment means that nobody reviewed it, which is no surprise given the number of open issues :) |
|
|
msg178158 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2012-12-25 17:15 |
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. |
|
|
msg178252 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-12-26 20:52 |
New changeset 6a56eaa5e5fb by Andrew Svetlov in branch 'default': Revert back PyCFunction_New macro. Keep PyCFunction_NewEx usage in python core modules (#15422) http://hg.python.org/cpython/rev/6a56eaa5e5fb |
|
|
msg178254 - (view) |
Author: Andrew Svetlov (asvetlov) *  |
Date: 2012-12-26 20:55 |
Georg, I've followed your instructions. Close the issue again. Thanks for mentorship. |
|
|
msg178256 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-12-26 21:09 |
New changeset 70ea05f762a1 by Andrew Svetlov in branch 'default': Fix compilation error for #15422 http://hg.python.org/cpython/rev/70ea05f762a1 |
|
|
msg202646 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-11-11 19:50 |
New changeset 267ad2ed4138 by Andrew Kuchling in branch 'default': #15422: remove NEWS item for a change that was later reverted http://hg.python.org/cpython/rev/267ad2ed4138 |
|
|
msg217193 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2014-04-26 13:00 |
Regression in issue #21354. |
|
|