Paths
| Differential D100762
[clang][cli] Extract AST dump format into extra option Changes PlannedPublic Authored by jansvoboda11 on Apr 19 2021, 7:13 AM.
Details Summary When dumping AST, the JSON format can be requested in two ways: -ast-dump=json and -ast-dump-all=json. Both of those command line options control FrontendOptions::ASTDumpFormat. This patch deduplicates the options by introducing -ast-dump-format, removes the _EQ variants of -ast-dump and -ast-dump-all, and simplifies option parsing/generation.
Diff Detail
Event TimelineHerald added subscribers: dang, usaxena95, kadircet, arphaman. · View Herald TranscriptApr 19 2021, 7:13 AM Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptApr 19 2021, 7:13 AM dexonsmith added inline comments.
This revision now requires changes to proceed.Apr 19 2021, 11:32 AM Comment Actions I'm not sure it's a good idea to remove the -ast-dump=json option. While this is -cc1 option, there do seem to be external consumers based on a quick search for "-ast-dump=json". Keeping it would also reduce some of the test churn. Comment Actions
Maybe -ast-dump=json can be changed to an alias for -ast-dump -ast-dump-format json. Comment Actions
+1 to this idea
Comment Actions If -ast-dump=json was a driver flag, it would be trivial to pass -ast-dump -ast-dump-format json to -cc1 instead. However, aliasing a single option to two options within the -cc1 argument parser isn't possible at the moment AFAIK. I can look into how much work adding that capability would be. @arichardson Can you point me to the external consumers? @dexonsmith I agree checking the offset only in a couple of tests would be better in terms of churn. Comment Actions
I just did the following search and saw that there are multiple stack overflow answers etc. recommending the use of -Xclang -ast-dump=json: https://www.google.com/search?q=%22-ast-dump%3Djson%22 I am not sure how many actual consumers there are, but I think it would be good to keep this option to avoid surprises for users. While they probably shouldn't be using internal -cc1 options, this is the only documented way of getting an AST dump. Comment Actions
This gets used on Compiler Explorer as it's the only way to get a JSON dump from that interface. e.g., https://godbolt.org/z/3nf6fvfYz
Revision Contents
Diff 338513 clang-tools-extra/clangd/DumpAST.h
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/AST/ast-dump-comment-json.cpp
clang/test/AST/ast-dump-decl-context-json.cpp
clang/test/AST/ast-dump-decl-json.c
clang/test/AST/ast-dump-decl-json.m
clang/test/AST/ast-dump-enum-json.cpp
clang/test/AST/ast-dump-expr-json.c
clang/test/AST/ast-dump-expr-json.cpp
clang/test/AST/ast-dump-expr-json.m
clang/test/AST/ast-dump-file-line-json.c
clang/test/AST/ast-dump-funcs-json.cpp
clang/test/AST/ast-dump-if-json.cpp
clang/test/AST/ast-dump-macro-json.c
clang/test/AST/ast-dump-namespace-json.cpp
clang/test/AST/ast-dump-objc-arc-json.m
clang/test/AST/ast-dump-record-definition-data-json.cpp
clang/test/AST/ast-dump-records-json.cpp
clang/test/AST/ast-dump-stmt-json.c
clang/test/AST/ast-dump-stmt-json.cpp
clang/test/AST/ast-dump-stmt-json.m
clang/test/AST/ast-dump-template-decls-json.cpp
clang/test/AST/ast-dump-temporaries-json.cpp
clang/test/AST/ast-dump-types-errors-json.cpp
clang/test/AST/ast-dump-types-json.cpp
clang/test/AST/gen_ast_dump_json_test.py
clang/test/AST/multistep-explicit-cast-json.c
clang/test/AST/multistep-explicit-cast-json.cpp
llvm/utils/update_cc_test_checks.py
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This is a lot of noise in the tests just from changing RUN lines. Maybe these tests shouldn't be checking the offset: field.
I suggest: