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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
