Skip to content

Get rid of PyCFunction_New macro #59627

Closed

Description

@asvetlov
BPO 15422
Nosy @loewis, @birkenfeld, @pitrou, @asvetlov
Files
  • issue15422.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/asvetlov'
    closed_at = <Date 2012-12-26.20:55:22.871>
    created_at = <Date 2012-07-22.14:33:02.910>
    labels = ['interpreter-core', 'type-feature']
    title = 'Get rid of PyCFunction_New macro'
    updated_at = <Date 2014-04-26.13:00:47.782>
    user = 'https://github.com/asvetlov'

    bugs.python.org fields:

    activity = <Date 2014-04-26.13:00:47.782>
    actor = 'pitrou'
    assignee = 'asvetlov'
    closed = True
    closed_date = <Date 2012-12-26.20:55:22.871>
    closer = 'asvetlov'
    components = ['Interpreter Core']
    creation = <Date 2012-07-22.14:33:02.910>
    creator = 'asvetlov'
    dependencies = []
    files = ['27453']
    hgrepos = []
    issue_num = 15422
    keywords = ['patch']
    message_count = 14.0
    messages = ['166138', '172196', '178116', '178117', '178132', '178134', '178139', '178157', '178158', '178252', '178254', '178256', '202646', '217193']
    nosy_count = 5.0
    nosy_names = ['loewis', 'georg.brandl', 'pitrou', 'asvetlov', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue15422'
    versions = ['Python 3.4']

    Activity

    1. asvetlov commented on Jul 22, 2012

      @asvetlov
      ContributorAuthor

      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:

    2. self-assigned this
      on Jul 22, 2012
    3. asvetlov commented on Oct 6, 2012

      @asvetlov
      ContributorAuthor

      Attached patch for the issue.
      BTW PyCFunction_New/PyCFunction_NewEx are part of Stable ABI but never mentioned in the documentation.

    4. asvetlov commented on Dec 25, 2012

      @asvetlov
      ContributorAuthor

      bpo-16776 created for documenting PyCFunction_New/PyCFunction_NewEx

    5. python-dev commented on Dec 25, 2012

      python-devmannequin
      Mannequin

      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

    6. birkenfeld commented on Dec 25, 2012

      @birkenfeld
      Member

      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?

    7. birkenfeld commented on Dec 25, 2012

      @birkenfeld
      Member

      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.

    8. asvetlov commented on Dec 25, 2012

      @asvetlov
      ContributorAuthor
      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.
    9. birkenfeld commented on Dec 25, 2012

      @birkenfeld
      Member

      There is no silent acceptance. No comment means that nobody reviewed it, which is no surprise given the number of open issues :)

    10. birkenfeld commented on Dec 25, 2012

      @birkenfeld
      Member

      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.

    11. python-dev commented on Dec 26, 2012

      python-devmannequin
      Mannequin

      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

    12. asvetlov commented on Dec 26, 2012

      @asvetlov
      ContributorAuthor

      Georg, I've followed your instructions.
      Close the issue again.
      Thanks for mentorship.

    13. python-dev commented on Dec 26, 2012

      python-devmannequin
      Mannequin

      New changeset 70ea05f762a1 by Andrew Svetlov in branch 'default':
      Fix compilation error for bpo-15422
      http://hg.python.org/cpython/rev/70ea05f762a1

    14. python-dev commented on Nov 11, 2013

      python-devmannequin
      Mannequin

      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

    15. pitrou commented on Apr 26, 2014

      @pitrou
      Member

      Regression in issue bpo-21354.

    16. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    interpreter-core(Objects, Python, Grammar, and Parser dirs)type-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions