Skip to content

[C API] PEP 670: Convert macros to functions in the Python C API #89653

Closed
Closed
[C API] PEP 670: Convert macros to functions in the Python C API#89653
Labels
3.11only security fixestopic-C-API

Description

@vstinner
vstinner
opened on Oct 15, 2021
BPO 45490
Nosy @malemburg, @vstinner, @erlend-aasland
PRs
  • [WIP] bpo-45490: Convert static inline to macros #29728
  • bpo-45490: Rename static inline functions #31217
  • bpo-45490: Convert unicodeobject.h macros to static inline functions #31221
    		•
    Files
  • macros-that-reuse-args.txt
    		•

    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 = None
created_at = <Date 2021-10-15.17:43:25.813>
labels = ['expert-C-API', '3.11']
title = '[C API]  PEP 670: Convert macros to functions in the Python C API'
updated_at = <Date 2022-02-11.16:01:26.957>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2022-02-11.16:01:26.957>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['C API']
creation = <Date 2021-10-15.17:43:25.813>
creator = 'vstinner'
dependencies = []
files = ['50616']
hgrepos = []
issue_num = 45490
keywords = ['patch']
message_count = 7.0
messages = ['404038', '404045', '404183', '404185', '412861', '412995', '413079']
nosy_count = 3.0
nosy_names = ['lemburg', 'vstinner', 'erlendaasland']
pr_nums = ['29728', '31217', '31221']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue45490'
versions = ['Python 3.11']

    Linked PRs

    Activity

    1. vstinner commented on Oct 15, 2021

      @vstinner
      vstinner
      on Oct 15, 2021
      MemberAuthor

      C macros are really cool and useful, but there are a bunch of pitfalls which are better to avoid:
      https://gcc.gnu.org/onlinedocs/cpp/Macro-Pitfalls.html

      Some macros of the Python C API have been converted to static inline functions over the last years. It went smoothly, I am not aware of any major issue with these conversions.

      This meta issue tracks other issues related to macros and static inline functions.

      === Return void ===

      One issue is that some macros are treated as an expression and can be reused, whereas it was not intended. For example PyList_SET_ITEM() was implemented as (simplified code):

        #define PyList_SET_ITEM(op, i, v) (op->ob_item[i] = v)

      This expression has a value! Two projects used this value by mistake, like:

      "if (obj == NULL || PyList_SET_ITEM (l, i, obj) < 0)"

      PyList_SET_ITEM() was fixed by casting the expression to void:

        #define PyList_SET_ITEM(op, i, v) ((void)(op->ob_item[i] = v))

      => bpo-30459

      === Abuse macros as an l-value ===

      The Py_TYPE() macro could be used to assign a value: "Py_TYPE(obj) = new_type".

      The macro was defined as:

        #define Py_TYPE(ob) (ob->ob_type)

      It was converted to a static inline function to disallow using it as an l-value and a new Py_SET_TYPE(op, new_type) function was added. These changes give more freedom to other Python implementations to implement "PyObject" and Py_SET_TYPE().

      => bpo-45476 "[C API] Disallow using PyFloat_AS_DOUBLE() as l-value"
      => bpo-39573 PyObject Py_TYPE/Py_SET_TYPE

      === C API: Macros and embedded Python ===

      Sadly, only symbols exported by libpython are accessible to other programming languages embedding Python. Macros of the Python C API are simply not available to them. Projects embedding Python have to hardcode constants and copy macros to their own language, with the risk of being outdated when Python macros are updated.

      Even some projects written in C cannot use macros, because they only use libpython symbols. The vim text editor embeds Python this way.

      Also, macros are simply excluded from the Python stable ABI (PEP-384).

      === Performance of static inline functions ===

      In bpo-45116, it seems like _PyEval_EvalFrameDefault() reached Visual Studio thresholds and some static inline functions are no longer inlined (Py_INCREF/Py_DECREF).

      I also noticed that when Python is built in debug mode in Visual Studio, static inline functions are not inlined. Well, the compiler is free to not inline in debug mode. I guess that GCC and clang also skip inlining using -Og and/or -O0 optimization levels. Using __forceinline and __attribute__((always_inline)) on static inline functions (Py_INCREF, Py_TYPE) for debug builds was discussed in bpo-45094, but the idea was rejected.

      On the other side, sometimes it's better to *disable* inlining on purpose to reduce the stack memory consumption, using the Py_NO_INLINE macro. See recent measurements of the stack memory usage:
      https://bugs.python.org/issue45439#msg403768

      In #73079, I noticed that converting a static inline function (PyObject_CallOneArg) to a regular function made it faster. I am not really sure, more benchmarks should be run to really what's going on.

      === Advantages of static inline functions ===

      • It's possible to put a breakpoint on a static inline functions.

      • Debuggers and profilers are able to get the static inline function names from the machine line, even with inline functions.

      • Parameters and the return value have well defined types.

      • Variables have a local scope.

      • There is no risk of evaluating an expression multiple times.

      • Regular C code. No need to use "\" character to multi-line statement. No need for "do { ... } while (0)" and other quicks to workaround preprocessor pitfalls. No abuse of (((parenthesis))).

    2. vstinner
      added
      3.11only security fixes
      topic-C-API
      on Oct 15, 2021

    3. malemburg commented on Oct 15, 2021

      @malemburg
      malemburg
      on Oct 15, 2021
      Member

      Meta comment :-) ... wouldn't it be better to enable the Github wiki feature for
      such collections ?

    4. erlend-aasland commented on Oct 18, 2021

      @erlend-aasland
      erlend-aasland
      on Oct 18, 2021
      Contributor

      +1!

      See also bpo-43502

    5. erlend-aasland commented on Oct 18, 2021

      @erlend-aasland
      erlend-aasland
      on Oct 18, 2021
      Contributor

      Previous discussion on Discourse:

      https://discuss.python.org/t/what-to-do-with-unsafe-macros/7771

    6. vstinner commented on Feb 8, 2022

      @vstinner
      vstinner
      on Feb 8, 2022
      MemberAuthor

      I will use this issue to track changes related to PEP-670.

    7. vstinner
      changed the title [-][meta][C API] Avoid C macro pitfalls and usage of static inline functions[/-] [+][C API] PEP 670: Convert macros to functions in the Python C API[/+] on Feb 8, 2022
    8. vstinner
      changed the title [-][meta][C API] Avoid C macro pitfalls and usage of static inline functions[/-] [+][C API] PEP 670: Convert macros to functions in the Python C API[/+] on Feb 8, 2022

    9. erlend-aasland commented on Feb 10, 2022

      @erlend-aasland
      erlend-aasland
      on Feb 10, 2022
      Contributor

      I made a list of macros that reuse their argument some time around February/March 2021. (Each macro is squashed into a single line for some reason I can't remember.) See attachment, or check out the gist version:

      https://gist.github.com/erlend-aasland/a7ca3cff95b136e272ff5b03447aff21

    10. vstinner commented on Feb 11, 2022

      @vstinner
      vstinner
      on Feb 11, 2022
      MemberAuthor

      New changeset e0bcfd0 by Victor Stinner in branch 'main':
      bpo-45490: Rename static inline functions (GH-31217)
      e0bcfd0

    11. ezio-melotti
      transferred this issue fromon Apr 10, 2022

    12. vstinner commented on Apr 19, 2022

      @vstinner
      vstinner
      on Apr 19, 2022
      MemberAuthor

      Update: the SC approved PEP 670: https://mail.python.org/archives/list/python-dev@python.org/thread/QQFCJ7LR36RUZSC3WI6WZZMQVQ3ZI4MS/

    13. 34 remaining items

    14. erlend-aasland commented on May 12, 2022

      @erlend-aasland
      erlend-aasland
      on May 12, 2022
      Contributor

      Yeah, but even simple issues get noisy pretty quickly. There should be a way to collapse these notifications with a keyboard shortcut.

    15. vstinner
      added 3 commits that reference this issue on May 13, 2022

      gh-89653: PEP 670: Convert PyUnicode_KIND() macro to function (#92705)

      db388df

      gh-89653: Use int type for Unicode kind (#92704)

      f62ad4f

      gh-89653: PEP 670: Fix PyUnicode_READ() cast (#92872)

      e6fd799
    16. miss-islington
      added a commit that references this issue on May 17, 2022

      pythongh-89653: PEP 670: Fix PyUnicode_READ() cast (pythonGH-92872)

      60e987f
    17. vstinner
      added a commit that references this issue on May 17, 2022

      gh-89653: Add assertions on PyUnicode_READ() index (#92883)

      1305832
    18. miss-islington
      added a commit that references this issue on May 17, 2022

      gh-89653: PEP 670: Fix PyUnicode_READ() cast (GH-92872)

      1df4550
    19. vstinner
      added 2 commits that reference this issue on Jun 13, 2022

      gh-89653: PEP 670: Convert PyFunction macros (#93765)

      272bec4

      gh-89653: PEP 670: Macros always cast arguments in cpython/ (#93766)

      df22eec

    20. vstinner commented on Jun 15, 2022

      @vstinner
      vstinner
      on Jun 15, 2022
      MemberAuthor

      Most important macros have been converted. I close the issue, I marked PEP 670 Status as Final. Maybe I will push a few more changes using this issue number, but the main part is done ;-)

    21. vstinner
      closed this as completedon Jun 15, 2022

    22. erlend-aasland commented on Jun 15, 2022

      @erlend-aasland
      erlend-aasland
      on Jun 15, 2022
      Contributor

      Great job, Victor! 😃 🚀

    23. serhiy-storchaka
      added a commit that references this issue on Jun 26, 2022

      Revert 30935 defer bpo45162 to 312 (#94285)

      1ffde3e

    24. CAM-Gerlach commented on Oct 16, 2022

      @CAM-Gerlach
      CAM-Gerlach
      on Oct 16, 2022
      Member

      OT: There should be an easy way of toggle hide/show commit references to issues. This gets pretty noisy after a while.

      A little late, but FYI there is with Refined GitHub:

      image

    25. vstinner
      added a commit that references this issue on Nov 28, 2022

      gh-89653: PEP 670: Convert macros to functions (#99843)

      02f72b8
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.11only security fixestopic-C-API

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions