[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[STINNER Victor]

STINNER Victor  added the comment:

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

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[hai shi]

hai shi  added the comment:

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

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[STINNER Victor]

STINNER Victor  added the comment:

> 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 
"""

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[STINNER Victor]

STINNER Victor  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[hai shi]

hai shi  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[hai shi]

Change by hai shi :


--
pull_requests: +18377
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/19028

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[Dong-hee Na]

Dong-hee Na  added the comment:

I reopen this issue for the above problem

--
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[Dong-hee Na]

Dong-hee Na  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[Dong-hee Na]

Dong-hee Na  added the comment:

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

--
nosy: +corona10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[hai shi]

hai shi  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[STINNER Victor]

STINNER Victor  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[STINNER Victor]

STINNER Victor  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[STINNER Victor]

STINNER Victor  added the comment:

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.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[STINNER Victor]

STINNER Victor  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[Raymond Hettinger]

Raymond Hettinger  added the comment:

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.

--
nosy: +rhettinger, tim.peters

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39968] port extension modules' macros of `get_module_state()` to inline function.

[hai shi]

Change by hai shi :


--
title: move extension modules' macros of `get_module_state()` to inline 
function. -> port extension modules' macros of `get_module_state()` to inline 
function.

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com