Repository navigation
Conversation
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
I found no problems in this PR. I compared the objects the base and PR versions of cppcheckdata.py build and found no differences. That covers the platform, language, files, raw tokens, suppressions and every configuration, on multi-configuration C, C++, real-world (lib/token.cpp, 10 configurations) and old-layout dumps. On a truncated dump and on an empty dump, the PR raises the same ParseError as base and leaves the garbage collector enabled. Addon output is the same, and misc.py on lib/token.cpp.dump dropped from 27.0s to 6.5s.
One behaviour change is worth knowing about, though I don't count it as a bug. data.configurations now returns the same cached list every time, and iterconfigurations() reuses it after that. So if an addon changes the configuration objects or the list in one pass, later passes now see those changes, where before each pass parsed the file again. None of the bundled addons depends on that.
Testing
Build: I ran make -j8 cppcheck in pr/, and it built.
Existing tests (all pass):
python3 -m pytest test/addon --cppcheck-binary=$PWD/cppcheck: 114 passed, 3 xfailed. Without--cppcheck-binaryevery fixture errors because nocppcheckis on PATH.PYTHONPATH=addons python3 -m pytest addons/test: 25 passed. This includes the misra--verifyand y2038 tests. WithoutPYTHONPATH=addonsthe tests can't importcppcheckdata.test/cli/dumpfile_test.pyandtest/cli/clang-import_test.py: 54 passed.test/cli/other_test.py -k "addon or misra or dump or premium": 44 passed, 1 skipped.
Old vs new comparison: I wrote scratch/cmp.py, which loads base/addons/cppcheckdata.py and pr/addons/cppcheckdata.py side by side. For each dump it turns the header data and every configuration into nested dicts of their attributes, two levels of nested objects deep (deeper ones are compared by Id), and compares them. I ran it on these dumps:
t1.c: 3#ifdefconfigurations, an inline suppression, typedefs, varargs and string macros.t2.cpp: a template, a lambda, virtual functions, and"<dump x>"and</dump>inside a string literal and a comment.lib/token.cpp: 10 configurations.- An old-layout dump I built from
t1.c.dumpby moving<rawtokens>and<suppressions>before the first<dump>.
All of them had identical header data and configurations, and gc.isenabled() was True afterwards.
Error cases: For a dump truncated after 3000 bytes and for an empty file, base and PR raise the same ParseError, and the garbage collector is still enabled afterwards.
Addon output:
misc.py --clionaddons/test/misc-test.cpp: same output on base and PR.misc.pyonlib/token.cpp.dump: same output; time 27.0s on base vs 6.5s on the PR.misra.pyonlib/token.cpp.dump: same output.
Automated review of 7a3fe1b by Claude. The pull request was built and tested locally.
Loading a dump file in
cppcheckdata.pyis now about 2.5x faster. Addons that iterate over the configurations more than once no longer parse the dump file on each pass. The parsed objects are unchanged.ElementTreeelements first.CppcheckDataconstructor skips the<dump>elements and parses only the rest of the file. Before, it parsed the whole file to reach the raw tokens and suppressions, which are written after the configurations.data.configurationsloads the configurations once and keeps them. Later accesses anditerconfigurations()reuse them, whileiterconfigurations()on its own still parses one configuration at a time.misc.pynow usesdata.configurationsfor its three passes, andmisra.py --verifyparses the file once instead of twice.Every parsed object was compared between the old and new code on dumps of the current layout, the old layout (raw tokens before the configurations) and clang-import dumps. New tests are in
test/addon.Timings
Fastest of several runs on Python 3.12, with peak memory after the slash.
Load the dump file and iterate over the configurations once:
misc.py(three passes over the configurations):misc.pyuses more memory on misra-test.c because it now keeps all 6 configurations in memory at once.