[PATCH] D135956: [include-cleaner] Add include-cleaner tool, with initial HTML report

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Hooks.h:9
+//
+// Where Analysis.h analyzes AST nodes and recorded preprocessor events, this
+// file defines ways to capture AST and preprocessor information from a parse.
----------------
hokein wrote:
> I assume we are all on the same page of the design -- I'd create the Analysis.h file, and move the `writeHTMLReport` function there in this patch.
This patch already does a fair amount of yak-shaving, and writeHTMLReport is very tangential to that header - I don't think it's possible to review e.g. a good file comment at this point.


================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Hooks.h:39
+  // (Traversing the TranslationUnitDecl would find uses inside headers!)
+  std::vector<Decl *> TopLevelDecls;
+};
----------------
hokein wrote:
> nit: I'd probably mention the "main file" bit in the name, MainTopLevelDecls?
Agree it's missing detail, but I think `MainTopLevelDecls` is too wordy.
How about `Roots` instead? It needs a bit more context to understand, but it's in the comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135956/new/

https://reviews.llvm.org/D135956