[PATCH] D137996: Add support for a backdoor driver option that enables emitting header usage information in JSON to a file
Jan Svoboda via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 15 09:09:12 PST 2022
jansvoboda11 added a comment.
Mostly looks good. Is this expected to work with Clang modules at all?
================
Comment at: clang/include/clang/Driver/Driver.h:240
+ unsigned CCPrintHeadersJson : 1;
+
----------------
Please document this.
================
Comment at: clang/include/clang/Driver/Options.td:5659
MarshallingInfoString<DependencyOutputOpts<"HeaderIncludeOutputFile">>;
+def header_include_json : Flag<["-"], "header-include-json">,
+ HelpText<"Print header include info in json">,
----------------
Could this be an enum of the desired format instead? It would default to textual and be overwritten with something like `-header-include-format=json`? I think it would result in simpler interface if in the future we decide to add another format.
================
Comment at: clang/lib/Frontend/HeaderIncludeGen.cpp:70
+ bool OwnsOutputFile;
+ std::set<std::string> IncludedHeaders;
+
----------------
Do we really want this to be a set? I can imagine clients wanting the include order to be preserved and with potential duplicates (for textual headers without guards or `#pragma once`).
If the set semantics are indeed what we want, I'd recommend using `llvm::StringSet<>` instead.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137996/new/
https://reviews.llvm.org/D137996
More information about the cfe-commits
mailing list