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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits