This is an archive of the discontinued LLVM Phabricator instance.

Improve gen_ast_dump_json_test.py
ClosedPublic

Authored by arichardson on Nov 12 2019, 4:07 AM.

Details

Summary

It is now possible to use --update to update the CHECK: lines instead of
generating a new file and the --clang/--opts/--filters arguments are no
longer required.
It should address all the issues listed in D69564

This is a series of 7 small commits, but it's probably easier to review the squashed diff.


Add a --update flag to gen_ast_dump_json_test.py

This will allow updating the JSON tests for new format changes. Instead of
simply appending the JSON to the input file, the script will now make a
copy of the input file up to the "CHECK lines have been autogenerated"
disclaimer and then append the new JSON.


Parse RUN: line for gen_ast_dump_json_test.py with --update


Infer --filters flags when using gen_ast_dump_json_test.py --update


Skip manual tests when using gen_ast_dump_json_tests.py --update


[gen_ast_dump_json_test.py] Copy to binary directory to omit --clang argument

The script will now check if a clang binary exists in the same directory
and default to that instead of requiring a --clang argument. The script
is copied to the clang build directory using CMake configure_file() with
COPYONLY. This ensures that the version in the build directory is updated
any time the source version changes.


[gen_ast_dump_json_test.py] Allow updating multiple files in one go

With this change it is possible to update all JSON dump tests using the
following command:
python $LLVM_BINDIR/gen_ast_dump_json_test.py --update --source $LLVM_SRC/clang/test/AST/*-json.*


[NFC] Regenerate ast-dump-json tests

Only changes are whitespace and line endings.

Diff Detail

Event Timeline

arichardson created this revision.Nov 12 2019, 4:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 12 2019, 4:07 AM
aaron.ballman accepted this revision.Nov 14 2019, 12:20 PM

LGTM aside from some nits. Thank you for this!

clang/test/AST/gen_ast_dump_json_test.py
3 ↗(On Diff #228856)

Is this needed? I don't see anything using print_function.

11 ↗(On Diff #228856)

Can you keep this in alphabetical order?

This revision is now accepted and ready to land.Nov 14 2019, 12:20 PM
arichardson marked 2 inline comments as done.Nov 14 2019, 3:18 PM
arichardson added inline comments.
clang/test/AST/gen_ast_dump_json_test.py
3 ↗(On Diff #228856)

Thanks for reviewing!

Despite the mismatched name, all python 3 print features depend on this. For example the sep= and function like syntax. Python2 prints a tuple for paren expressions instead of treating print as a function.

11 ↗(On Diff #228856)

Will fix before committing.

This revision was automatically updated to reflect the committed changes.
clang/test/AST/ast-dump-decl-json.m