Issue 39968: port extension modules' macros of `get_module_state()` to inline function.

as victor and petr said in PR18613：inline function is more better than macros, so I plan move all extension modules' macros of `get_xx_state()` to inline function.

Note: some inline get_xx_state() can not be used directly in `tp_traverse`、`tp_free` and `tp_clear`(issue39824).

Tim, do you approve of this kind of wholesale macro-to-inline function conversion?

My thought is that in most cases, there is no advantage to conversion and that sometimes there will be minor disadvantage in code generation when the macro is used across files.  I like inline functions as well as next person, but that doesn't mean all macros have to be rewritten.  Mostly, this seems like unnecessary code churn.

New changeset f707d94af68a15afc27c1a9da5835f9456259fea by Hai Shi in branch 'master':
bpo-39968: Convert extension modules' macros of get_module_state() to inline functions (GH-19017)
https://github.com/python/cpython/commit/f707d94af68a15afc27c1a9da5835f9456259fea

There are multiple good reasons to replace macros with static inline functions:

* Parameter types and return value are well defined
* Less error-prone: avoid completely https://gcc.gnu.org/onlinedocs/gcc-9.3.0/cpp/Macro-Pitfalls.html#Macro-Pitfalls
* Variables have a well defined scope, reduce the risk of unexpected side effects

I merged the PR 19017: it uses better names for the function and it adds assert(state != NULL).

Be careful of bpo-39824: in some cases, the module state *can* be NULL: but I checked, and it's not the case here. Moreover, bpo-39824 may prevent NULL module state (let's see ;-)).

Thanks Hai Shi!


> minor disadvantage in code generation

Micro-benchmarks and machine code analysis was done in previous issues replacing macros with static inline functions, and the outcome is that there is no effect on performance. See for example bpo-35059: performance critical Py_INCREF() and Py_DECREF() functions are now static inline functions.

If someone is curious about the the machine code, you should use -O3 with PGO+LTO, which is what should be used on Linux distributions. If static inline functions are not inlined for whatever reason, I suggest to tune the compiler rather than moving back to ugly macros.

If you use configure --enable-shared, I now also strongly suggest to use -fno-semantic-interposition: it's now used on Fedora and RHEL to build Python libpython.

Note: Support for "static inline" is now explicitly required by the PEP 7 to build Python, since Python 3.6. And it's more and more used in Python header files.

Note: Hai Shi is working hard on converting modules to multiphase module initialization (PEP 489) which helps to cleanup the Python state at exit, and subinterpreters will likely benefit of that. This issue is related to this work.

Oh, I forgot a cool advantage that I discovered late: debuggers and profilers understand inlined code and are able to get the name of the static inline function, whereas it's not possible to do that with macros. If I recall correctly, it works even if the function is inlined.

Wow, thanks, victor. those msgs is very helpful to me.

> cpython/Modules/readline.c:90:20: error: unknown type name
      'PyModule'
get_readline_state(PyModule *module)

Compile is failure after PR 19017 is merged on macOS.

See also: https://github.com/python/cpython/runs/509226542#step:4:305

I reopen this issue for the above problem

> See also: https://github.com/python/cpython/runs/509226542#step:4:305
Oh, thanks, Dong-hee Na. I create a PR for this typo error.

New changeset 5f104d56fa10f88098338b3f1ea74bcbe6924ca9 by Hai Shi in branch 'master':
bpo-39968: Fix a typo error in get_readline_state() (GH-19028)
https://github.com/python/cpython/commit/5f104d56fa10f88098338b3f1ea74bcbe6924ca9

> Compile is failure after PR 19017 is merged on macOS.

Oh, macOS job was marked as success on PR 19017 :-(
https://github.com/python/cpython/runs/509226542

But I confirm that I can read there:

"""
Failed to build these modules:
_tkinter              readline         
"""

> Oh, macOS job was marked as success on PR 19017 :-(

yeah, it's weird. This macOS job should be enhanced?

> This macOS job should be enhanced?

setup.py skips optional dependencies like readline on purpose.

readline is built again onx86-64 macOS 3.x buildbot worker and test_readline passed. I close the issue.

Thanks for the quick fix.

stage: patch review -> resolved

stage: patch review -> resolved