bpo-35081: Enhance Python-ast.h and ast.h#10277
bpo-35081: Enhance Python-ast.h and ast.h#10277vstinner wants to merge 1 commit intopython:masterfrom vstinner:pythonasth
Conversation
Modify asdl_c.py to enhance Python-ast.h:
* Add #ifndef/#define PYTHON_AST_H to be able to include the header
twice
* Add "extern { ... }" for C++
* Undefine "Yield" macro conflicting with winbase.h
* ast.h: add node.h and Python-ast.h includes
|
What problem does it solve? I have not found anything on bpo-35081, may be open a new issue for discussing? |
I wanted to reorder includes in pylifecycle.c, but ast.h doesn't declare properly its dependencies: it uses node which comes from node.h, and mod_ty which comes from Python-ast.h. This PR fix this. Undefining the "Yield" macro fix a warning on Windows. In practice, it's more a cleanup.
Well, it's more a "cleanup" change, so I didn't want to open a new issue just for that. |
| #ifndef PYTHON_AST_H | ||
| #define PYTHON_AST_H | ||
| #ifdef __cplusplus | ||
| extern "C" { |
There was a problem hiding this comment.
Do we need this? This is an internal generated header, it is not purposed for direct use.
There was a problem hiding this comment.
Yes, it's needed if ast.h is included in file compiled in C++. I know that some people do that. At least, it doesn't hurt to add extern "C" { ... }.
| extern "C" { | ||
| #endif | ||
|
|
||
| #include "node.h" /* node */ |
There was a problem hiding this comment.
Other option is to add forward declarations, as in compile.h.
There was a problem hiding this comment.
Well, maybe, but it seems like node.h is included anyway in all C files including ast.h. I don't see the benefit of avoiding to include node.h here. I prefer to be explicit and link ast.h to node.h.
There was a problem hiding this comment.
#include "Python-ast.h" is already surrounded with extern "C" { in ast.h.
|
Superseded by https://bugs.python.org/issue35177 and PR #10361. |
5 participants
Modify asdl_c.py to enhance Python-ast.h:
twice
https://bugs.python.org/issue35081