[PATCH] D136723: [include-cleaner] Record main-file macro occurences and includes

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

looks good from my side.



================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:52
 struct Symbol {
   enum Kind {
     // A canonical clang declaration.
----------------
 probably worth adding a comment:  the order of the enumerators must match the  the order of template arguments in Storage. 


================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:80
+// Indicates that a piece of code refers to a symbol.
+struct SymbolReference {
+  // The symbol referred to.
----------------
thinking more about it (no action required) -- this structure seems a natural good fit to be used in `UsedSymbolCB` in `Analysis.h` rather than using ` SourceLocation RefLoc, Symbol Target` separately.



================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:20
 
+class PPRecorder : public PPCallbacks {
+public:
----------------
this should be hidden in anonymous namespace, right? 


================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:25
+
+  virtual void FileChanged(SourceLocation Loc, FileChangeReason Reason,
+                           SrcMgr::CharacteristicKind FileType,
----------------
nit: virtual can be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136723