| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Rename AST functions of pycore_ast.h to use the "_PyAST_" prefix. Remove macros creating aliases without prefix. For example, Module() becomes _PyAST_Module(). Update Grammar/python.gram to use _PyAST_xxx() functions.
|
@pablogsal: Does this change look reasonable to you? I already pushed the commit d36d6a9 to prepare the code base (add "Py" prefix in Python-ast.c and ast_opt.c). My intent is to not pollule the _Py_ C namespace with generic names like _Py_Call() and _Py_Tuple() which look too close to PyObject_Call() and PyTuple_New() names. |
Sorry, something went wrong.
|
This looks reasonable, but one downside of this is that backporting grammar fixes to 3.9 is going to be nontrivial because this will create merge conflicts in the grammar actions :( |
Sorry, something went wrong.
Is that a common thing? I count 7 commits modifying Grammar/python.gram since v3.9.0 (October 2020, 6 months ago) in the 3.9 branch. |
Sorry, something went wrong.
|
cc @lysnikolaou @isidentical who modified Grammar/python.gram in the last 6 months. |
Sorry, something went wrong.
|
Not a whole lot to add here. It's true that it's not that common, but it happens from time to time. My feeling is that we can live with it, since with the two parsers being there in 3.9 we already have conflicts most (all?) of the time in the generated C file, when backporting fixes, so it already requires some manual. One line more to change wouldn't make that big of a difference. |
Sorry, something went wrong.
We have made some precautions to make backporting easier in the past because the merge conflicts can be a bit nasty but as @lysnikolaou says, we could live with it. |
Sorry, something went wrong.
|
An alternative would be to create a header file full of #define xxx _PyAST_xxx (one per symbol) and only use it in the parser. Other files already access directly _PyAST_xxx() symbols (currently _Py_xxx()). So you can use directly Module() rather than _Py_Module or _PyAST_Module. Right now, Grammar/python.gram already refer to _Py_xxx() symbol. |
Sorry, something went wrong.
|
An alternative is to only remove aliases and always use _Py_ prefix on function names: I created #25256 for that. I changes less lines in Grammar/python.gram. It was my first attempt, but I though that if we are going to change things anyway, it can be an opportunity to "fix" the prefix from _Py_ to _PyAST_. |
Sorry, something went wrong.
|
Nah, I don't think is worth it. I am fine with this PR |
Sorry, something went wrong.
|
@pablogsal @lysnikolaou: Since you are the one backporting grammar/parser/compiler fixes, I would prefer if one of you could approve officially the PR. I'm also fine with the status quo (keep aliases), it's more up to you ;-) |
Sorry, something went wrong.
|
Thanks for the review @pablogsal and @lysnikolaou! I merged my PR. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Rename AST functions of pycore_ast.h to use the "PyAST" prefix.
Remove macros creating aliases without prefix. For example, Module()
becomes _PyAST_Module(). Update Grammar/python.gram to use
_PyAST_xxx() functions.
https://bugs.python.org/issue43244