This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Add missing dependencies between AST/parser header files
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: vstinner
Priority: normal Keywords: patch

Created on 2018-11-06 14:08 by vstinner, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 10361 merged vstinner, 2018-11-06 14:09
PR 10664 merged vstinner, 2018-11-22 17:12
Messages (6)
msg329359 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-11-06 14:08
Currently, there are *implicit* dependencies between AST/parser header files. For example, ast.h uses node and mod_ty types but don't include node.h nor Python-ast.h. And parsetok.h uses node and grammer but don't include nor grammar.h.

Because of that, C files have to include header files in the "correct order" and need to include header files even if they don't use directly.

At the end, we get something like pythonrun.c:

#include "Python-ast.h"
#include "pycore_state.h"
#include "grammar.h"
#include "node.h"
#include "token.h"
#include "parsetok.h"
#include "errcode.h"
#include "code.h"
#include "symtable.h"
#include "ast.h"
#include "marshal.h"
#include "osdefs.h"
#include <locale.h>

whereas most header files are useless, pythonrun.c still compiles with:

#include "pycore_state.h"
#include "token.h"      /* INDENT in err_input() */
#include "parsetok.h"   /* PyParser_ParseFileObject() */
#include "errcode.h"    /* E_EOF */
#include "symtable.h"   /* PySymtable_BuildObject() */
#include "ast.h"        /* PyAST_FromNodeObject() */
#include "marshal.h"    /* PyMarshal_ReadLongFromFile */
#include <locale.h>

I propose to add explicit dependencies in header files directly, rather than using black magic in C files.

Attached PR fix this issue.
msg329696 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-11-11 23:56
New changeset 5f2df88b63e50d23914e97ec778861a52abdeaad by Victor Stinner in branch 'master':
bpo-35177: Add dependencies between header files (GH-10361)
https://github.com/python/cpython/commit/5f2df88b63e50d23914e97ec778861a52abdeaad
msg329699 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-11-12 00:08
This change doesn't try to fix "all dependencies issues", but it fix a few of them :-) See bpo-35197 for another example of funny header issue.

Please open a new issues if you see other header dependencies issues. I close this one since the initial bug has been fixed.
msg329738 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-11-12 15:39
Oh, the Yield warning made its come back :-( Example:

  import.c
c:\program files (x86)\windows kits\10\include\10.0.17134.0\um\winbase.h(102): warning C4005: 'Yield': macro redefinition [C:\projects\cpython\PCbuild\pythoncore.vcxproj]
  c:\projects\cpython\include\python-ast.h(549): note: see previous definition of 'Yield'
msg330272 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-11-22 17:38
New changeset 3bb183d7fb83ad6a84ec13dea90f95d67be35c69 by Victor Stinner in branch 'master':
bpo-35177, Python-ast.h: Fix "Yield" compiler warning (GH-10664)
https://github.com/python/cpython/commit/3bb183d7fb83ad6a84ec13dea90f95d67be35c69
msg330273 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-11-22 17:39
Ok, I fixed the "Yield warning" again.
History
Date User Action Args
2022-04-11 14:59:07adminsetgithub: 79358
2018-11-22 17:39:41vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg330273

stage: patch review -> resolved
2018-11-22 17:38:41vstinnersetmessages: + msg330272
2018-11-22 17:12:45vstinnersetstage: resolved -> patch review
pull_requests: + pull_request9917
2018-11-12 15:39:46vstinnersetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg329738
2018-11-12 00:08:23vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg329699

stage: patch review -> resolved
2018-11-11 23:56:22vstinnersetmessages: + msg329696
2018-11-06 14:09:36vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request9661
2018-11-06 14:08:41vstinnercreate