[PATCH] D135956: [include-cleaner] Add include-cleaner tool, with initial HTML report
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 17 02:36:06 PDT 2022
hokein added a comment.
+1, it looks a great start to me (left some initial comments). The plan also looks good to me.
================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Hooks.h:1
+//===--- Hooks.h - Record compiler events -------------------------- C++-*-===//
+//
----------------
nit: the name `Hooks.h` seems a little vague, `Record.h` seems slightly better to me.
================
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.
----------------
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.
================
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;
+};
----------------
nit: I'd probably mention the "main file" bit in the name, MainTopLevelDecls?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135956/new/
https://reviews.llvm.org/D135956
More information about the cfe-commits
mailing list