bpo-41194: Convert _ast extension to PEP 489#21293
bpo-41194: Convert _ast extension to PEP 489#21293vstinner merged 2 commits intopython:masterfrom vstinner:ast_module_state
Conversation
Convert the _ast extension module to PEP 489 "Multiphase initialization". Replace the global _ast state with a module state.
|
With this PR, get_global_ast_state() is still used by multiple functions. It can introduce minor inconsistencies when the _ast extension module is loaded more than once in the same interpreter. But this PR better isolates the _ast module in subinterpreters. The PyType_GetModule() (PEP 573) cannot be used in ast_type_init() (tp_init slot). Even if all AST_Type methods and slots are fixed to use PyType_GetModule(), there are still PyAST_mod2obj() and PyAST_obj2mod() functions which still use get_global_ast_state(). Even if get_global_ast_state() has drawbacks, IMHO this PR is worth it. @corona10: Would you mind to review it? cc @DinoV |
corona10
left a comment
corona10
left a comment
There was a problem hiding this comment.
Looks good with a simple question :)
But https://travis-ci.org/github/python/cpython/builds/704696576 is the build and it completed. I don't get it :-(
|
|
I close/reopen my issue to force a new Travis CI build :-( |
|
I did same thing on #21294 :( |
|
re-open :) |
* bpo-41194: Convert _ast extension to PEP 489 (GH-21293) Convert the _ast extension module to PEP 489 "Multiphase initialization". Replace the global _ast state with a module state. (cherry picked from commit b1cc6ba) * bpo-41204: Fix compiler warning in ast_type_init() (GH-21307) (cherry picked from commit 1f76453)
| if (name == NULL) { | ||
| return NULL; | ||
| } | ||
| PyObject *module = PyImport_GetModule(name); |
There was a problem hiding this comment.
As noted in https://bugs.python.org/issue41631, this is incorrect :(
PyImport_GetModule looks in sys.modules, which is modifiable by the user, and so it's not guaranteed to return _ast.
5 participants
Convert the _ast extension module to PEP 489 "Multiphase
initialization". Replace the global _ast state with a module state.
https://bugs.python.org/issue41194