[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