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
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.
The reason will be displayed to describe this comment to others. Learn more.
Do we need this? This is an internal generated header, it is not purposed for direct use.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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" { ... }.
| @@ -4,6 +4,9 @@ | |||
| extern "C" { | |||
| #endif | |||
|
|
|||
| #include "node.h" /* node */ | |||
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Other option is to add forward declarations, as in compile.h.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
The reason will be displayed to describe this comment to others. Learn more.
#include "Python-ast.h" is already surrounded with extern "C" { in ast.h.
|
Superseded by https://bugs.python.org/issue35177 and PR #10361. |
Modify asdl_c.py to enhance Python-ast.h:
twice
https://bugs.python.org/issue35081