Avoid macros and static inline functions

[encukou]

All functions should be exported as proper symbols, so they are usable without a C preprocessor/parser.

We expect a lot of exceptions, which should be added as alternatives to proper functions (as per #4).

Macros or static inline functions can override/shadow a function with the same name and behavior. The dance for that is:

Header file:

static inline returntype
_Py_Foo_impl(args)
{
    ...
}

PyAPI_FUNC(returntype) Py_Foo (args);

// speedup for C/C++
#define Py_Foo _Py_Foo_impl

.c file:

// at the end (after all calls to Py_Foo):
#undef Py_Foo

returntype
Py_Foo (args)
{
    return _Py_Foo_impl(args);
}

Let's have Argument Clinic generate the boilerplate?

---

Another exception are C-specific helper macros, like Py_VISIT or Py_MIN. IMO, the docs for these should document the internals clearly enough that people can rewrite them in other languages.
Unless they're not useful in other languages (like Py_UNREACHABLE or Py_UNUSED).

[vstinner]

See also issue #37: Declare constants as variables, instead of macros.

[pitrou]

I'm not sure why inline functions should be avoided. As long as you make sure there is a corresponding extern declaration somewhere in the compilation units, the symbol should be exposed.

According to https://en.cppreference.com/w/c/language/inline (emphasis mine):

If a function is declared inline in some translation units, it does not need to be declared inline everywhere: at most one translation unit may also provide a regular, non-inline non-static function, or a function declared extern inline. This one translation unit is said to provide the external definition.

There is a helpful StackOverflow answer about this:
https://stackoverflow.com/a/17505846

So you can have this in a .h:

inline returntype
Py_Foo(args)
{
    ...
}

and then in a single .c file (perhaps one dedicated to the extern definitions of public inline functions):

extern PyAPI_FUNC(returntype)
Py_Foo(args);

[gvanrossum]

I'm guessing the (only? by far most common?) use of inline functions in CPython is static inline, and I don't think you can add an extern declaration for those. Possibly the issue title should just be updated to say "static inline" instead of "inline"?

[pitrou]

@gvanrossum I think you're right!

[vstinner]

FYI the rationale for using static inline vs inline was decided when the Py_INCREF() macro was converted to a static inline function: python/cpython#79240 especially this comment: python/cpython#79240 (comment) It was also discussed there to use macros to "always inline" static inline functions (at the end, it was decided to not use that).

The impact on performance of static inline functions (versus macros) was measured in PEP 670 – Convert macros to functions in the Python C API.

[vstinner]

Macros or static inline functions can override/shadow a function with the same name and behavior.

An alternative is to convert static inline functions to regular functions.

So far, I didn't see any conclusive analysis that static inline functions are really faster than regular function calls, especially in third party C extensions.

[pitrou]

FYI the rationale for using static inline vs inline was decided when the Py_INCREF() macro was converted to a static inline function: python/cpython#79240 especially this comment: python/cpython#79240 (comment)

I don't want to diminish your work, but I don't see any "rationale" in this discussion, just a number of comments that seem to be talking past each other, and an arbitrary decision from you at the end :-)

So far, I didn't see any conclusive analysis that static inline functions are really faster than regular function calls, especially in third party C extensions.

I agree that performance claims should generally be backed with benchmark numbers, but I don't think a lot of analysis is required to conclude that Py_INCREF will generally be faster if inlined than if called through a regular function call.

Yes, most C extensions might not benefit from inlining of trivial functions such as Py_INCREF, PyTuple_GET_ITEM, etc. But some of them will. Even if they are a minority, you can't ignore that part of the ecosystem.

[vstinner]

I agree that performance claims should generally be backed with benchmark numbers, but I don't think a lot of analysis is required to conclude that Py_INCREF will generally be faster if inlined than if called through a regular function call. Yes, most C extensions might not benefit from inlining of trivial functions such as Py_INCREF, PyTuple_GET_ITEM, etc.

This issue isn't about Py_INCREF() or PyTuple_GET_ITEM(), but more general. I'm not suggesting to convert Py_INCREF() or PyTuple_GET_ITEM() to regular functions.

I'm just saying that we should design a guideline to decide which functions are seen as performance critical and should have a faster variant, and which ones don't need that. I don't think that all of the ~1600 C API functions need to have a faster variant (only a small numbers have it right now).

I don't want to diminish your work, but

I don't see the point of your remark unless you want to diminish my work. Maybe just omit it next time.

I don't see any "rationale" in this discussion, just a number of comments that seem to be talking past each other, and an arbitrary decision from you at the end :-)

I'm just giving pointers to past discussions if some people want to understand how static inline was selected. You see the choice as "arbitrary". Things changed since 2018, the C API has way more static inline functions, constraints about performance changes, we have a better understand on how the C API is consumed, etc.

We can move from static inline to something else if needed. I don't have a strong opinion about it. I just saw that so far, we didn't get major issues specific to static inline. I just vaguely recall issues with inline and duplicate function definitions in different C files. If someone needs more details, I can try to look into the details (that I now forgot).

[vstinner]

Message moved to a new issue: Define a public "maximize performance" API and separate it better from the regular "maximize compatibility" API.

[pitrou]

I'm just saying that we should design a guideline to decide which functions are seen as performance critical and should have a faster variant, and which ones don't need that.

This is derailing the discussion from the subject of this issue ("avoid macros and static inline functions"). Perhaps you should open a separate issue for that guideline?

[vstinner]

This is derailing the discussion from the subject of this issue ("avoid macros and static inline functions"). Perhaps you should open a separate issue for that guideline?

Right. I created #45 for my previous message.

[vstinner]

All functions should be exported as proper symbols, so they are usable without a C preprocessor/parser.

I'm not sure that it's possible if we take in account the name of the static inline function which should also have a good name. The most common case is that the static inline function is used. Using a different name is as annoying as having the opaque function with a different name.

The problem is to define a static inline function and a regular function with the same name (in the same header file):

PyAPI_FUNC(void) _Py_SetRefcnt(PyObject *ob, Py_ssize_t refcnt);

static inline void Py_SET_REFCNT(PyObject *ob, Py_ssize_t refcnt) {
#if defined(Py_LIMITED_API) && Py_LIMITED_API+0 >= 0x030d0000
    _Py_SetRefcnt(ob, refcnt);
#else
    ...
#endif  // Py_LIMITED_API+0 < 0x030d0000
}

If the two functions have the same name:

PyAPI_FUNC(void) Py_SET_REFCNT(PyObject *ob, Py_ssize_t refcnt);
static inline void Py_SET_REFCNT(PyObject *ob, Py_ssize_t refcnt) { ... }

The compiler fails with:

Include/object.h:357:20: error: static declaration of 'Py_SET_REFCNT' follows non-static declaration

---

For example, in gdb, currently, I can put a breakpoint on Py_SET_REFCNT static inline function and it just works.

(gdb) b Py_SET_REFCNT
Breakpoint 1 at 0x4f37d0: Py_SET_REFCNT. (5 locations)

(gdb) run
Starting program: /home/vstinner/python/main/python 

Breakpoint 1.4, Py_SET_REFCNT (ob=<unknown at remote 0xa72560>, refcnt=1)
    at ./Include/object.h:368
368	    _Py_SetRefcnt(ob, refcnt);

In Python 3.10, static inline functions had different names than the public C API. For example, Py_INCREF() API became _Py_INCREF() static inline function. It made debugging more complicated (well, at least for me). In Python 3.11, the static inline function was renamed to the expected name: Py_INCREF().

Symbols have different use cases:

• Get a symbol to link a program or dynamic library
• Put a breakpoint in a debugger
• Get a backtrace in a debugger or a profiler

With your proposed change, all functions implemented as static inline functions must now be referred as their private name: _Py_foo_impl instead of the public name Py_foo.

Existing functions implemented as static inline functions and opaque function calls:

• Py_INCREF() calls _Py_IncRef().
• Py_DECREF() calls _Py_DecRef().
• Py_SET_REFCNT() calls _Py_SetRefcnt().
• Py_REFCNT() calls _Py_GetRefcnt() -- gh-111506: Implement Py_REFCNT() as opaque function call python/cpython#112747 under review.

[encukou]

With your proposed change, all functions implemented as static inline functions must now be referred as their private name: _Py_foo_impl instead of the public name Py_foo.

No. This is what the (C-specific) alias is there for: #define Py_Foo _Py_Foo_impl

[vstinner]

No. This is what the (C-specific) alias is there for: #define Py_Foo _Py_Foo_impl

The macro keeps API compatibility: same API name. But at the ABI level, such as names in gdb, _Py_Foo_impl() will be used with such macro.

[pitrou]

These pitfalls would be avoided by switching to non-static inline...

[vstinner]

I wrote python/cpython#112794 which may satisfy all requirements:

• Provide Py_SET_REFCNT() as a static inline function in the regular C API.
• Export Py_SET_REFCNT() as an opaque function in the stable ABI: used by limited C API version 3.13.
• In the limited C API 3.13, Py_SET_REFCNT() is no longer declared as static inline function, only as an opaque function call: PyAPI_FUNC(void) Py_SET_REFCNT(PyObject *ob, Py_ssize_t refcnt);.

Yes, obviously, my implementation uses and abuses macros. But differently that what @encukou proposes, since the static inline function is only renamed if the new _Py_STABLE_ABI_IMPL macro is defined, which is only defined in the new Objects/object_abi.c file.

This way, C extensions built with the static inline functions continue to use Py_SET_REFCNT() name: compilers, linkers, debuggers and profilers all use the same name.

C extensions built with the limited C API version 3.13 and newer use the Py_SET_REFCNT() name. At the ABI level, the Py_SET_REFCNT() function is called.

[vstinner]

These pitfalls would be avoided by switching to non-static inline...

And to compare with @pitrou's suggestion, I wrote an implementation where the regular API and the stable ABI both use Py_SET_REFCNT name by implementing Py_SET_REFCNT() as an inline function, instead of a static inline function: python/cpython#112798

[encukou]

I must admit I'm not too familiar with (non-static) inline. This reference text is worrying:

in C++, if a function is declared inline, it must be declared inline in every translation unit, and also every definition of an inline function must be exactly the same (in C, the definitions may be different, and depending on the differences only results in unspecified behavior).

It seems that if we make any change to an inline function in a point release, and an extension compiled with new headers is run on older CPython, then:

• For a C extension, either the old version (from CPython) or the new one (from headers) is used. That is fine.
• For a C++ extension, this is undefined behaviour 💥☠️🔥

How does that work in practice? With dynamically loaded libraries?
Am I overthinking this?

[pitrou]

an extension compiled with new headers is run on older CPython

So, this can technically only happen if using the stable ABI (limited API), in which case the definition isn't found in the headers, so the definition from the CPython library should be used.

[vstinner]

@encukou:

All functions should be exported as proper symbols, so they are usable without a C preprocessor/parser.

I think that it's reasonable goal to make sure that it's possible to access any function using dlopen() + dlsym().

The issue title is "Avoid macros and static inline functions". I'm not sure that I understand the title. Do you suggest to convert all macros and static inline functions to regular functions? Your example keeps the static inline and export the function as a regular function.

Maybe the title should be updated to clarify the goal, such as "All functions should be exported as proper symbols".

[encukou]

So, this can technically only happen if using the stable ABI

No, you can build with 3.13.5 and run on 3.13.0.

Do you suggest to convert all macros and static inline functions to regular functions?

No. All the guideline issues here are for new API.

Maybe the title should be updated to clarify the goal, such as "All functions should be exported as proper symbols".

Yes, and explain what “proper” means.
I hope to have a public PEP draft this week, let's do the wording discussion then.

For non-function macros there's #37

[pitrou]

No, you can build with 3.13.5 and run on 3.13.0.

Oh, you're right. I'm not sure what the concrete consequences would be, even in C++. Presumably the compiler wouldn't decompile the implementation from the shared library and say "oh, this is different from the definition in the current compilation unit, so this is UB and I'm allowed to do whatever I want".

That said, using static inline neatly avoids the entire issue by making things more deterministic:

• on builds against the non-limited API, the function implementation from the header file would always be compiled into the current compilation unit
• on builds against the limited API, the function implementation from the shared library would always be called from the current compilation unit

@gpshead Any opinions on this topic?

[encukou]

Added to the draft in #53.