[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