I created bpo-43502 as an attempt to remove the worst C macro pitfalls in the C API. For example, there are currently 88 macros that reuse their arguments under the
Include/ directories. There are cases in the CPython code base where some of these are called with the increment operator, thus possibly executing the increment multiple times instead of once. One solution is to fix the callers. Another is to fix the macro, for example by implementing it fully or partially using static inline functions (or ordinary functions). The former is a quick fix, but does not fix the real problem. The latter fixes the real problem (the macro), but has multiple issues. Comments from @vstinner:
- Backward incompatible C API changes may be introduced by mistake
- New compiler warnings may be introduced (functions are typed, macros are not)
Comments from @pablogsal:
- Macros transformed into function may or may not get inlined so there is a risk of affecting performance if we just transform them in bulk
- Because the macro is defined as ‘static inline’ in the header file, then it can be inlined everywhere if the compiled decides to do it. It will be one copy symbol in the text segment on every compilation unit. Notice that ‘inline’ has a special meaning in C99 and it doesn’t only mean “inline this function”. It has visibility implications (see section 6.7.4 of the C99 standard).
- There is risk to introducing a backwards-incompatible ABI change by mistake as Victor mentions.
- This can affect also the link patterns of extension modules because these symbols may be visible in the resulting binaries when they were macro-level-inlined before. I cannot see any problem that can happen because of this but we should carefully consider it.
Comments from @rhettinger:
“Be careful about performance. We know for sure that macros will inline. In contrast, inline functions might or might not inline (there are rules for when it can be done). When applied across source files, inlining often only occurs with an LTO build — any other build can’t inline across object files. Historically, we’ve never had to depend on LTO, so that would be a regression for some users. We’ve already have a performance bug that needed to be reverted because of too aggressive conversion of macros to inline functions.”
Note: I do not propose getting rid of all macros, and I do not propose converting macros blindly; I only propose getting rid of obvious unsafe macros that are likely to be misused (for example, there’s an index argument) if the fix has no negative side-effects (see above).
Is it worth it pursuing this? If no, I’ll close the issue