bpo-41796: Make _ast module state per interpreter#23024
Conversation
The ast module internal state is now per interpreter. * Rename "astmodulestate" to "struct ast_state" * Add pycore_ast.h internal header: the ast_state structure is now declared in pycore_ast.h. * Add PyInterpreterState.ast (struct ast_state) * Remove get_ast_state() * Rename get_global_ast_state() to get_ast_state() * PyAST_obj2mod() now handles get_ast_state() failures
|
In short, this change replaces |
Sorry, something went wrong.
|
Oh, I missed test_peg_generator. I should now be fixed with my second commit. |
Sorry, something went wrong.
lysnikolaou
left a comment
There was a problem hiding this comment.
Should the Windows regen file (regen.vcxproj) be updated as well?
Sorry, something went wrong.
|
@isidentical: Thanks for the review. I addressed your review. Please review the updated PR. |
Sorry, something went wrong.
|
@lysnikolaou: "Should the Windows regen file (regen.vcxproj) be updated as well?" Oh, I didn't know this project. Thanks, I updated it. |
Sorry, something went wrong.
|
I merged my PR. Thanks for the useful review @lysnikolaou and @isidentical! I dislike having to declare the struct ast_state once in pycore_ast.h and once in Python-ast.c, but Python-ast.c is a strange beast which is used outside Python without Py_BUILD_CORE. So declaring the structure twice is a practical solution for that. @encukou used a different solution: use a pointer to an opaque structure in the PyInterpreterState, and allocate the structure on the heap memory when it's initialized. That would avoid to declare the structure twice. But I chose to follow the current trend of declaring all structures in pycore_interp.h and have a large unique and flat PyInterpreter structure (no pointer indirection). Maybe tomorrow we should revisit this design to have something more flexible, to not have to declare everything in pycore_interp.h. The best would be to be able to have a real module state, but I was biten multiple times by the C API which requires the AST module state, whereas the C API isn't directly related to a module. I tried a solution using an import and we got a crash with Mercurial lazy import, etc. |
Sorry, something went wrong.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️Hi! The buildbot s390x RHEL8 LTO 3.x has failed when building commit 5cf4782. What do you need to do:
You can take a look at the buildbot page here: https://buildbot.python.org/all/#builders/567/builds/926 Summary of the results of the build (if available): == Tests result: ENV CHANGED == 409 tests OK. 10 slowest tests:
1 test altered the execution environment: 14 tests skipped: Total duration: 4 min 49 sec Click to see traceback logsTraceback (most recent call last):
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto/build/Lib/asyncio/sslproto.py", line 321, in __del__
self.close()
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto/build/Lib/asyncio/sslproto.py", line 316, in close
self._ssl_protocol._start_shutdown()
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto/build/Lib/asyncio/sslproto.py", line 590, in _start_shutdown
self._abort()
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto/build/Lib/asyncio/sslproto.py", line 731, in _abort
self._transport.abort()
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto/build/Lib/asyncio/selector_events.py", line 680, in abort
self._force_close(None)
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto/build/Lib/asyncio/selector_events.py", line 731, in _force_close
self._loop.call_soon(self._call_connection_lost, exc)
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto/build/Lib/asyncio/base_events.py", line 746, in call_soon
self._check_closed()
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto/build/Lib/asyncio/base_events.py", line 510, in _check_closed
raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed
|
Sorry, something went wrong.
|
I am relatively sure that this commit introduced reference leaks in the test suite: https://buildbot.python.org/all/#/builders/205/builds/83 OK See also https://bugs.python.org/issue42250 |
Sorry, something went wrong.
The ast module internal state is now per interpreter. * Rename "astmodulestate" to "struct ast_state" * Add pycore_ast.h internal header: the ast_state structure is now declared in pycore_ast.h. * Add PyInterpreterState.ast (struct ast_state) * Remove get_ast_state() * Rename get_global_ast_state() to get_ast_state() * PyAST_obj2mod() now handles get_ast_state() failures
The ast module internal state is now per interpreter.
declared in pycore_ast.h.
https://bugs.python.org/issue41796