Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Nov 6, 2018

  • ast.h now includes Python-ast.h and node.h

  • parsetok.h now includes node.h and grammar.h

  • symtable.h now includes Python-ast.h

  • 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
  • Remove "#undef Yield" from C files, it's now done in Python-ast.h

  • Remove now useless includes in C files

https://bugs.python.org/issue35177

@vstinner
Copy link
Member Author

vstinner commented Nov 6, 2018

I added the "skip news" label: my changes don't impact the public C API, and is not backward incompatible.

* ast.h now includes Python-ast.h and node.h
* parsetok.h now includes node.h and grammar.h
* symtable.h now includes Python-ast.h
* Modify asdl_c.py to enhance Python-ast.h:

  * Add #ifndef/#define Py_PYTHON_AST_H to be able to include the header
    twice
  * Add "extern { ... }" for C++
  * Undefine "Yield" macro conflicting with winbase.h

* Remove "#undef Yield" from C files, it's now done in Python-ast.h
* Remove now useless includes in C files
@vstinner
Copy link
Member Author

vstinner commented Nov 6, 2018

@serhiy-storchaka @ncoghlan: I tried to go further in this PR, but also add more missing includes. Tell if it looks better than my previous attempt, or if you prefer to leave C file unchanged for example (only remove #undef Yield?).

@vstinner
Copy link
Member Author

vstinner commented Nov 9, 2018

I plan to merge this PR next week if I don't get feedback before.

@vstinner vstinner merged commit 5f2df88 into python:master Nov 11, 2018
@vstinner vstinner deleted the header_dep branch November 11, 2018 23:56
@vstinner
Copy link
Member Author

This PR doesn't try to fix "all dependencies issues", but it fix a few of them :-) See https://bugs.python.org/issue35197 for another example of funny header issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants