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

[vstinner]

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

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

[vstinner]

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))).

[malemburg]

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

[erlend-aasland]

+1!

See also bpo-43502

[erlend-aasland]

Previous discussion on Discourse:

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

[vstinner]

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

[erlend-aasland]

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

[vstinner]

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

[vstinner]

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

[vstinner]

Merged commits:

• commit 4e52c66: PEP 670: Convert unicodeobject.h macros to functions (gh-89653: PEP 670: Convert unicodeobject.h macros to functions #91705)
• commit 1a2b282: PEP 670: Convert PyWeakref_GET_OBJECT() to function (gh-89653: PEP 670: Convert PyWeakref_GET_OBJECT() to function #91785)
• commit 2a5f171: PEP 670: Convert tuple macros to functions (gh-89653: PEP 670: Convert tuple macros to functions #91786)
• commit 128d624: PEP 670: Convert unicodeobject.h macros to functions (gh-89653: PEP 670: Convert unicodeobject.h macros to functions #91773)
• commit c1474fa: PEP 670: Group deprecated API in unicodeobject.h (gh-89653: PEP 670: Group deprecated API in unicodeobject.h #91796)
• commit 636ad7b: PEP 670: Convert unicodeobject.h macros to functions (gh-89653: PEP 670: Convert unicodeobject.h macros to functions #91799)
• commit efe7fd4: Add assertions to unicodeobject.h functions (gh-89653: Add assertions to unicodeobject.h functions #91800)
• commit 61381d7: PEP 670: Functions don't cast pointers (gh-89653: PEP 670: Functions don't cast pointers #91697)
• commit 1218061: PEP 670: Amend docs (gh-89653: PEP 670: amend docs #91813)

See also #91768 which replaced const PyObject* with PyObject* and commit 9e146bb (gh-80527: Deprecate PEP 623 Unicode functions, #91801).

[vstinner]

Bugfixes in the 3.11 branch:

• commit 6f92872: PEP 670: Limited API doesn't cast arguments ([3.11] gh-89653: PEP 670: Limited API doesn't cast arguments (GH-92654) #92693)
• commit a1bef8c: PEP 670: Use PyObject* type for parameters ([3.11] gh-89653: PEP 670: Use PyObject* type for parameters (GH-92694) #92698)
• commit 6e1a214: PEP 670: unicodeobject.h uses _Py_CAST() ([3.11] gh-89653: PEP 670: unicodeobject.h uses _Py_CAST() (GH-92696) #92703)

[vstinner]

Merged commits:

• commit 61381d7: Functions don't cast pointers (gh-89653: PEP 670: Functions don't cast pointers #91697)
• commit 2eca5da: Convert PyBytes macros to functions (gh-89653: PEP 670: Convert PyBytes macros to functions #91990)
• commit 08b562a: Convert PyCFunction macros to functions (gh-89653: PEP 670: Convert PyCFunction macros to functions #92302)
• commit ffcc7cd: Convert pycore_gc.h macros to functions (gh-89653: PEP 670: Convert pycore_gc.h macros to functions #92649)
• commit 897f14d: Convert PyCell macros to functions (gh-89653: PEP 670: Convert PyCell macros to functions #92653)
• commit eb88f21: Convert unicodeobject.h macros to functions (gh-89653: PEP 670: Convert unicodeobject.h macros to functions #92648)
• commit 7d3b469: Limited API doesn't cast arguments (gh-89653: PEP 670: Limited API doesn't cast arguments #92654)
• commit 6de78ef: Use PyObject* type for parameters (gh-89653: PEP 670: Use PyObject* type for parameters #92694)
• commit f9b67ad: Convert PyDict_GET_SIZE() macro to function (gh-89653: PEP 670: Convert PyDict_GET_SIZE() macro to function #92695)
• commit 92f0ed1: Update C API unicode documentation (gh-89653: PEP 670: Update C API unicode documentation #92702)
• commit d0c9353: unicodeobject.h uses _Py_CAST() (gh-89653: PEP 670: unicodeobject.h uses _Py_CAST() #92696)

[erlend-aasland]

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

[vstinner]

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

It's common that I push a new branch into my vstinner/cpython: the commit is mentioned there. I prepare a PR, but I see a typo. I fix the typo and "git push --force": it adds a second mention. Sometimes, I do that 5 to 10 times for a single PR. Here I created many PRs, so there are tons of mentions.

[erlend-aasland]

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

[vstinner]

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 ;-)

[erlend-aasland]

Great job, Victor! 😃 🚀

[CAM-Gerlach]

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: