[PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

Benjamin Kramer via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 26 05:22:51 PDT 2016


bkramer added a comment.

First, please run this through clang-format with LLVM style. This aligns all the & and * to the name instead of the type. There are also some underscore_names left that need conversion to PascalCase.


================
Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:168
@@ +167,3 @@
+  const clang::SourceManager *SM = Result.SourceManager;
+  // Ignore STL header for now.
+  if (SM->isInSystemHeader(ND->getLocStart()))
----------------
Why?

================
Comment at: include-fixer/find-all-symbols/FindAllSymbols.h:26
@@ +25,3 @@
+
+    virtual void ReportResult(const std::string &file_name,
+                              const SymbolInfo& Symbol) = 0;
----------------
StringRef for the argument. we also start methods with a small letter in LLVM (reportResult).

================
Comment at: include-fixer/find-all-symbols/SymbolInfo.cpp:104
@@ +103,3 @@
+bool SymbolInfo::operator<(const SymbolInfo& Symbol) const {
+  if (Identifier != Symbol.Identifier)
+    return Identifier < Symbol.Identifier;
----------------
This could be written as std::tie(Identifier, FilePath, LineNumber) < std::tie(Symbol.Identifier, Symbol.FilePath, Symbol.LineNumber)

================
Comment at: include-fixer/find-all-symbols/SymbolInfo.cpp:111
@@ +110,3 @@
+
+std::string SymbolInfoToYaml(SymbolInfo Symbol) {
+  std::string S;
----------------
Please write directly to the stream instead of going through std::string.


Repository:
  rL LLVM

http://reviews.llvm.org/D19482





More information about the cfe-commits mailing list