[PATCH] D20420: [find-all-symbol] Add macro support.

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Thu May 19 03:18:00 PDT 2016


ioeric added inline comments.

================
Comment at: include-fixer/find-all-symbols/FindAllMacros.h:22
@@ +21,3 @@
+
+/// \brief A preprocessor collects macro symbols. The contexts of a macro will
+/// be ignored since they are not available during preprocessing period.
----------------
nit: "A preprocessor that collects..."

================
Comment at: include-fixer/find-all-symbols/FindAllMacros.h:36
@@ +35,3 @@
+  SymbolReporter *const Reporter;
+  // A remapping header file collector allowing clients include a different
+  // header.
----------------
nit: "allowing clients to include..."

================
Comment at: include-fixer/find-all-symbols/FindAllMacros.h:39
@@ +38,3 @@
+  HeaderMapCollector *const Collector;
+  // SourceManager.
+  SourceManager *const SM;
----------------
This comment does not introduce useful information IMO.

================
Comment at: include-fixer/find-all-symbols/SymbolReporter.h:18
@@ +17,3 @@
+
+/// \brief An interface for classes collect symbols.
+class SymbolReporter {
----------------
This doesn't make sense to me? 

Also, I think it should be "An interface for classes that/to collect symbols" grammatically.


http://reviews.llvm.org/D20420





More information about the cfe-commits mailing list