[PATCH] D19869: Added XrefsDBManager into include-fixer and made XrefsDB return SymbolInfo.

Manuel Klimek via cfe-commits cfe-commits at lists.llvm.org
Tue May 3 08:39:09 PDT 2016


klimek added inline comments.

================
Comment at: include-fixer/InMemoryXrefsDB.cpp:18
@@ +17,3 @@
+InMemoryXrefsDB::InMemoryXrefsDB(
+    std::map<std::string, std::vector<std::string>> LookupTable) {
+  for (const auto &Entry : LookupTable) {
----------------
I'd make that a const ref.

================
Comment at: include-fixer/InMemoryXrefsDB.cpp:24-26
@@ +23,5 @@
+    for (const auto &Header : Entry.second) {
+      SymbolInfo Info;
+      Info.Name = Names.back();
+      Info.FilePath = Header;
+      for (auto IdentiferContext = Names.rbegin() + 1;
----------------
I assume it's intentional that SymbolInfo doesn't have a constructor; what's the reasoning behind that?

================
Comment at: include-fixer/IncludeFixer.h:34
@@ -33,2 +33,3 @@
   IncludeFixerActionFactory(
-      XrefsDB &Xrefs, std::vector<clang::tooling::Replacement> &Replacements,
+      XrefsDBManager &XrefsDBMgr,
+      std::vector<clang::tooling::Replacement> &Replacements,
----------------
Why wouldn't we still use the interface here? (usually we want to take the least specific type we can deal with)


http://reviews.llvm.org/D19869





More information about the cfe-commits mailing list