Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C API] Remove _PyErr_OCCURRED() macro #87436

Closed
vstinner opened this issue Feb 19, 2021 · 4 comments
Closed

[C API] Remove _PyErr_OCCURRED() macro #87436

vstinner opened this issue Feb 19, 2021 · 4 comments
Labels
3.10 only security fixes topic-C-API

Comments

@vstinner
Copy link
Member

BPO 43270
Nosy @vstinner
PRs
  • bpo-43270: Remove private _PyErr_OCCURRED() macro #24579
  • 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 = None
    closed_at = <Date 2021-02-19.14:09:11.893>
    created_at = <Date 2021-02-19.13:25:53.212>
    labels = ['expert-C-API', '3.10']
    title = '[C API] Remove _PyErr_OCCURRED() macro'
    updated_at = <Date 2021-02-19.14:09:11.892>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2021-02-19.14:09:11.892>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-02-19.14:09:11.893>
    closer = 'vstinner'
    components = ['C API']
    creation = <Date 2021-02-19.13:25:53.212>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 43270
    keywords = ['patch']
    message_count = 4.0
    messages = ['387318', '387319', '387320', '387322']
    nosy_count = 1.0
    nosy_names = ['vstinner']
    pr_nums = ['24579']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue43270'
    versions = ['Python 3.10']

    @vstinner
    Copy link
    Member Author

    The private _PyErr_OCCURRED() function was introduced to optimize Objects/setobject.c:

    commit 5ba0cbe
    Author: Raymond Hettinger <python@rcn.com>
    Date: Sat Aug 6 18:31:24 2005 +0000

    * set_new() doesn't need to zero the structure a second time after tp_alloc
      has already done the job.
    * Use a macro form of PyErr_Occurred() inside the set_lookkey() function.
    

    But the usage of the macro was removed one month later:

    commit 9bda1d6
    Author: Raymond Hettinger <python@rcn.com>
    Date: Fri Sep 16 07:14:21 2005 +0000

    No longer ignore exceptions raised by comparisons during key lookup.
    Inspired by Armin Rigo's suggestion to do the same with dictionaries.
    

    The macro is currently defined as:

    #if defined(Py_DEBUG) || defined(Py_LIMITED_API)
    #define _PyErr_OCCURRED() PyErr_Occurred()
    #else
    #define _PyErr_OCCURRED() (PyThreadState_GET()->curexc_type)
    #endif

    IMO the new _PyErr_Occurred(tstate) internal function is a more reliable way (don't depend on Py_DEBUG and Py_LIMITED_API) to ensure that the code uses the most efficient way to check if an exception was raised.

    I cannot find "_PyErr_OCCURRED" in the PyPI top 4000 projects, I checked with INADA-san's tool:
    https://github.com/methane/notes/tree/master/2020/wchar-cache
    (But I found many C extensiosn using "PyErr_Occurred" which is fine, this one stays ;-) I just wanted to check that my search was working.)

    So removing _PyErr_OCCURRED() is unlikely to break PyPI top 4000 projects. If it breaks a third party project: well, we don't provide any backward compatibility warranty on the *private* C API.

    @vstinner vstinner added 3.10 only security fixes topic-C-API labels Feb 19, 2021
    @vstinner
    Copy link
    Member Author

    #define _PyErr_OCCURRED() (PyThreadState_GET()->curexc_type)

    But this way, this macro access directly the PyThreadState.curexc_type member which goes against the bpo-39947 "[C API] Make the PyThreadState structure opaque (move it to the internal C API)" issue.

    @vstinner
    Copy link
    Member Author

    _PyErr_Occurred() is defined as:

    static inline PyObject* _PyErr_Occurred(PyThreadState *tstate)
    {
        assert(tstate != NULL);
        return tstate->curexc_type;
    }

    @vstinner
    Copy link
    Member Author

    New changeset a486054 by Victor Stinner in branch 'master':
    bpo-43270: Remove private _PyErr_OCCURRED() macro (GH-24579)
    a486054

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes topic-C-API
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant