[PATCH] D19314: [include-fixer] Add a prototype for a new include fixing tool.

[Benjamin Kramer via cfe-commits]

bkramer marked 8 inline comments as done.

================
Comment at: include-fixer/IncludeFixer.cpp:132
@@ +131,3 @@
+
+private:
+  /// Query the database for a given identifier.
----------------
klimek wrote:
> Can we sort this so the public interface comes first? Also, why is the public interface so large?
Sorted. Most of it is dealing with clang callbacks. I also don't consider this to be a public interface as it's all in the cpp file.

================
Comment at: include-fixer/IncludeFixer.cpp:184-190
@@ +183,9 @@
+               std::vector<clang::tooling::Replacement> &replacements) {
+    for (const auto &ToTry : Untried) {
+      DEBUG(llvm::dbgs() << "Adding include " << ToTry << "\n");
+      std::string ToAdd = "\n#include " + ToTry;
+      // If this is the only include in the file, add the newline after it, not
+      // before.
+      if (LastIncludeOffset == 0)
+        std::rotate(ToAdd.begin(), ToAdd.begin() + 1, ToAdd.end());
+
----------------
klimek wrote:
> Can we reuse functionality from clang-format here?
Yes we should. I changed it to insert new includes before the first include, clang-format will clean up the rest. clang-format isn't wired up to the tool yet, I'd like to use the new applyAllReplacements with formatting tools for that, should be ready soon.

================
Comment at: include-fixer/IncludeFixer.h:26
@@ +25,3 @@
+  IncludeFixerActionFactory(
+      std::unique_ptr<XrefsDB> Xrefs,
+      std::vector<clang::tooling::Replacement> &Replacements);
----------------
klimek wrote:
> Why are we passing ownership?
Taking a reference now.

================
Comment at: include-fixer/IncludeFixer.h:36
@@ +35,3 @@
+
+  XrefsDB *getXrefsDB() const { return Xrefs.get(); }
+
----------------
klimek wrote:
> That also seems weird in the interface here.
Removed.


http://reviews.llvm.org/D19314