[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]

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