classification
Title: Get rid of PyCFunction_New macro
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: asvetlov Nosy List: asvetlov, georg.brandl, loewis, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2012-07-22 14:33 by asvetlov, last changed 2014-04-26 13:00 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
issue15422.diff asvetlov, 2012-10-06 13:41 review
Messages (14)
msg166138 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2012-12-25 11:16
#16776 created for documenting PyCFunction_New/PyCFunction_NewEx
msg178117 - (view) Author: Roundup Robot (python-dev) (Python triager) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) (Python triager) 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) * (Python committer) 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) (Python triager) 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) (Python triager) 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) * (Python committer) Date: 2014-04-26 13:00
Regression in issue #21354.
History
Date User Action Args
2014-04-26 13:00:47pitrousetmessages: + msg217193
2013-11-11 19:50:25python-devsetmessages: + msg202646
2012-12-26 21:09:06python-devsetmessages: + msg178256
2012-12-26 20:55:22asvetlovsetstatus: open -> closed
resolution: fixed
messages: + msg178254

stage: resolved
2012-12-26 20:52:14python-devsetmessages: + msg178252
2012-12-25 17:29:17asvetlovsetstatus: closed -> open
resolution: fixed -> (no value)
stage: resolved -> (no value)
2012-12-25 17:15:48georg.brandlsetmessages: + msg178158
2012-12-25 17:13:46georg.brandlsetmessages: + msg178157
2012-12-25 15:17:54asvetlovsetmessages: + msg178139
2012-12-25 14:59:26georg.brandlsetmessages: + msg178134
2012-12-25 14:58:38georg.brandlsetnosy: + georg.brandl, pitrou
messages: + msg178132
2012-12-25 11:33:04asvetlovsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2012-12-25 11:32:45python-devsetnosy: + python-dev
messages: + msg178117
2012-12-25 11:16:30asvetlovsetmessages: + msg178116
2012-10-06 13:41:16asvetlovsetstage: needs patch -> patch review
2012-10-06 13:41:02asvetlovsetfiles: + issue15422.diff
keywords: + patch
messages: + msg172196
2012-07-22 14:33:02asvetlovcreate