-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
bpo-41194: Convert _ast extension to PEP 489 #21293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 :) |
Convert the _ast extension module to PEP 489 "Multiphase initialization". Replace the global _ast state with a module state.
* 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
.
Convert the _ast extension module to PEP 489 "Multiphase
initialization". Replace the global _ast state with a module state.
https://bugs.python.org/issue41194