The existing Tools/c-globals/check-c-globals.py script is an important tool for achieving a per-interpreter GIL (and generally for the health of the CPython code base). However, it has a number of problems that keep it from being helpful in reaching that objective.
First we'll review the characteristics of the tool. Then we'll look at the deficiencies. Finally, we'll cover the specific tasks that will address those problems.
Purpose
The script has the following 2 core purposes:
make sure there are no "unsupported" static variables in CPython
help address failures when such static variables are found
including making it easy to see the current related state of the CPython code base
(More or less, "unsupported" means the variable holds per-interpreter runtime state.)
Constraints
The script also has the following core constraints:
produce correct results (identify exactly all unsupported static variables)
be up-to-date (relative to ignored variables and to what "unsupported" means)
have low maintenance cost (few defects, easy to understand, easy to update/extend)
Deficiencies
In context of those purposes and constraints, the script has some critical deficiencies (in rough order of priority):
not actually used by core developers (so statics keep getting added)
static locals are not detected
detection relies on a platform-specific tool (nm on linux)
the code is a mess (impacting maintainability)
no tests (impacting maintainability)
relies on naive hard-coded rules to ignore many statics (impacting correctness & maintainability)
relies on a simple large list of names (from a basic flat text file) to ignore the remaining statics (impacting correctness & maintainability)
(Note that non-critical deficiencies (and nice-to-haves) are covered in #49.)
Solution
(Reminder: we're only aiming to addresss critical deficiencies here.)
High-level Plan
[x] rename Tools/c-globals to Tools/c-analyzer
[ ] add c-statics.py
the "full-featured" replacement for check-c-statics.py
only provide minimal necessary functionality to meet core purposes (and be fast enough)
correct the critical deficiencies
(detailed tasks below)
[ ] rewrite check-c-globals.py as a single-purpose script named check-c-statics.py
no args
share code with c-statics.py (i.e. call Tools/c-analyzer/c_statics/__main__.py:main())
refer to the c-statics.py script in output
[ ] support ignoring some "unsupported" statics
add Tools/c-analyzer/c_statics/ignored.tsv
add a --ignore option to the c-statics.py script
[ ] add Lib/test/test_check_c_statics.py
1 test that effectively runs the check-c-statics.py script
ignore all statics in ignored.tsv for now (will still catch any new statics)
ensures check-c-statics.py is run at some point in CI
[ ] make sure it's clear how to resolve "unsupported" statics
At that point we can use the tool sans nearly all deficiencies. The remaining tasks would then be:
(see #33)
The existing
Tools/c-globals/check-c-globals.py
script is an important tool for achieving a per-interpreter GIL (and generally for the health of the CPython code base). However, it has a number of problems that keep it from being helpful in reaching that objective.First we'll review the characteristics of the tool. Then we'll look at the deficiencies. Finally, we'll cover the specific tasks that will address those problems.
Purpose
The script has the following 2 core purposes:
(More or less, "unsupported" means the variable holds per-interpreter runtime state.)
Constraints
The script also has the following core constraints:
Deficiencies
In context of those purposes and constraints, the script has some critical deficiencies (in rough order of priority):
nm
on linux)(Note that non-critical deficiencies (and nice-to-haves) are covered in #49.)
Solution
(Reminder: we're only aiming to addresss critical deficiencies here.)
High-level Plan
Tools/c-globals
toTools/c-analyzer
c-statics.py
check-c-statics.py
check-c-globals.py
as a single-purpose script namedcheck-c-statics.py
c-statics.py
(i.e. callTools/c-analyzer/c_statics/__main__.py:main()
)c-statics.py
script in outputTools/c-analyzer/c_statics/ignored.tsv
--ignore
option to thec-statics.py
scriptLib/test/test_check_c_statics.py
check-c-statics.py
scriptignored.tsv
for now (will still catch any new statics)At that point we can use the tool sans nearly all deficiencies. The remaining tasks would then be:
c-statics.py
script results independentlyDirectory Structure
c-statics.py CLI
The
c-statics.py
script will initially have the following basic CLI:show [--ignored FILE] [--known FILE] [DIR,...]
check [--ignored FILE] [--known FILE] [DIR,...]
Specific Tasks for
c-globals.py
(Each task includes adding sufficient tests.)
Tools/c-globals/_cg/
: add__init__.py
Lib/test/test_toolcglobals.py
: frame out the tests_cg/__main__.py
: add a basicparse_args()
,main()
, andif __name__ == '__main__
:`__main__.py
: stub out a "show" command w/ file args__main__.py
: stub out a "check" command w/ file argsTools/c-globals/c-globals.py
: copyif __name__ == '__main__
:from
_cg/main.py`info.py
: addStaticVar
(filename, funcname, name, vartype)find.py
: add a fake "statics()" func (w/ hard-coded [StaticVar, supported] list)show.py
: addshow_basic()
(group by supported / unsupported)__main__.py
: implement the "show" command__main__.py
: implement the "check" commandscan.py
: add "iter_statics(file)" with a fake list ofStaticVar
supported.py
: add "is_supported(var)" with a simple testable heuristicfind.py
: implement "statics()"scan.py
: add "normalize_vartype(text)"scan.py
: implement "iter_statics()"known.py
: add "self_check()"Lib/test/test_toolcglobals.py
: add black-box system tests forc-globals.py
(see below)Lib/test/test_toolcglobals.py
: add a "self-check" system testSystem Tests
nm
outputc-globals.py show
against dummy filesc-globals.py check
against dummy files:dummy files: