Author: d0k Date: Thu Apr 28 06:21:29 2016 New Revision: 267868 URL: http://llvm.org/viewvc/llvm-project?rev=267868&view=rev Log: [include-fixer] Add an option to minimize include paths.
This will always pick the shortest possible path based on -I options. Based on the #include suggestion code for modules. Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp clang-tools-extra/trunk/include-fixer/IncludeFixer.h clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=267868&r1=267867&r2=267868&view=diff ============================================================================== --- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original) +++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Thu Apr 28 06:21:29 2016 @@ -10,6 +10,7 @@ #include "IncludeFixer.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/TextDiagnosticPrinter.h" +#include "clang/Lex/HeaderSearch.h" #include "clang/Lex/Preprocessor.h" #include "clang/Parse/ParseAST.h" #include "clang/Rewrite/Core/Rewriter.h" @@ -59,7 +60,8 @@ private: class Action : public clang::ASTFrontendAction, public clang::ExternalSemaSource { public: - explicit Action(XrefsDB &Xrefs) : Xrefs(Xrefs) {} + explicit Action(XrefsDB &Xrefs, bool MinimizeIncludePaths) + : Xrefs(Xrefs), MinimizeIncludePaths(MinimizeIncludePaths) {} std::unique_ptr<clang::ASTConsumer> CreateASTConsumer(clang::CompilerInstance &Compiler, @@ -147,13 +149,40 @@ public: FirstIncludeOffset = Offset; } + /// Get the minimal include for a given path. + std::string minimizeInclude(StringRef Include, + clang::SourceManager &SourceManager, + clang::HeaderSearch &HeaderSearch) { + if (!MinimizeIncludePaths) + return Include; + + // Get the FileEntry for the include. + StringRef StrippedInclude = Include.trim("\"<>"); + const FileEntry *Entry = + SourceManager.getFileManager().getFile(StrippedInclude); + + // If the file doesn't exist return the path from the database. + // FIXME: This should never happen. + if (!Entry) + return Include; + + bool IsSystem; + std::string Suggestion = + HeaderSearch.suggestPathToFileForDiagnostics(Entry, &IsSystem); + + return IsSystem ? '<' + Suggestion + '>' : '"' + Suggestion + '"'; + } + /// Generate replacements for the suggested includes. /// \return true if changes will be made, false otherwise. bool Rewrite(clang::SourceManager &SourceManager, + clang::HeaderSearch &HeaderSearch, std::vector<clang::tooling::Replacement> &replacements) { for (const auto &ToTry : Untried) { - DEBUG(llvm::dbgs() << "Adding include " << ToTry << "\n"); - std::string ToAdd = "#include " + ToTry + "\n"; + std::string ToAdd = "#include " + + minimizeInclude(ToTry, SourceManager, HeaderSearch) + + "\n"; + DEBUG(llvm::dbgs() << "Adding " << ToAdd << "\n"); if (FirstIncludeOffset == -1U) FirstIncludeOffset = 0; @@ -228,6 +257,9 @@ private: /// Includes we have left to try. std::set<std::string> Untried; + + /// Whether we should use the smallest possible include path. + bool MinimizeIncludePaths = true; }; void PreprocessorHooks::FileChanged(clang::SourceLocation Loc, @@ -271,8 +303,10 @@ void PreprocessorHooks::InclusionDirecti } // namespace IncludeFixerActionFactory::IncludeFixerActionFactory( - XrefsDB &Xrefs, std::vector<clang::tooling::Replacement> &Replacements) - : Xrefs(Xrefs), Replacements(Replacements) {} + XrefsDB &Xrefs, std::vector<clang::tooling::Replacement> &Replacements, + bool MinimizeIncludePaths) + : Xrefs(Xrefs), Replacements(Replacements), + MinimizeIncludePaths(MinimizeIncludePaths) {} IncludeFixerActionFactory::~IncludeFixerActionFactory() = default; @@ -294,11 +328,14 @@ bool IncludeFixerActionFactory::runInvoc Compiler.createSourceManager(*Files); // Run the parser, gather missing includes. - auto ScopedToolAction = llvm::make_unique<Action>(Xrefs); + auto ScopedToolAction = + llvm::make_unique<Action>(Xrefs, MinimizeIncludePaths); Compiler.ExecuteAction(*ScopedToolAction); // Generate replacements. - ScopedToolAction->Rewrite(Compiler.getSourceManager(), Replacements); + ScopedToolAction->Rewrite(Compiler.getSourceManager(), + Compiler.getPreprocessor().getHeaderSearchInfo(), + Replacements); // Technically this should only return true if we're sure that we have a // parseable file. We don't know that though. Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.h?rev=267868&r1=267867&r2=267868&view=diff ============================================================================== --- clang-tools-extra/trunk/include-fixer/IncludeFixer.h (original) +++ clang-tools-extra/trunk/include-fixer/IncludeFixer.h Thu Apr 28 06:21:29 2016 @@ -22,8 +22,10 @@ class IncludeFixerActionFactory : public public: /// \param Xrefs A source for matching symbols to header files. /// \param Replacements Storage for the output of the fixer. + /// \param MinimizeIncludePaths whether inserted include paths are optimized. IncludeFixerActionFactory( - XrefsDB &Xrefs, std::vector<clang::tooling::Replacement> &Replacements); + XrefsDB &Xrefs, std::vector<clang::tooling::Replacement> &Replacements, + bool MinimizeIncludePaths = true); ~IncludeFixerActionFactory(); bool @@ -38,6 +40,9 @@ private: /// Replacements are written here. std::vector<clang::tooling::Replacement> &Replacements; + + /// Whether inserted include paths should be optimized. + bool MinimizeIncludePaths; }; } // namespace include_fixer Modified: clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp?rev=267868&r1=267867&r2=267868&view=diff ============================================================================== --- clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp (original) +++ clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp Thu Apr 28 06:21:29 2016 @@ -32,6 +32,11 @@ cl::opt<DatabaseFormatTy> DatabaseFormat cl::opt<std::string> Input("input", cl::desc("String to initialize the database"), cl::cat(IncludeFixerCategory)); + +cl::opt<bool> + MinimizeIncludePaths("minimize-paths", + cl::desc("Whether to minimize added include paths"), + cl::init(true), cl::cat(IncludeFixerCategory)); } // namespace int main(int argc, const char **argv) { @@ -66,7 +71,8 @@ int main(int argc, const char **argv) { // Now run our tool. std::vector<tooling::Replacement> Replacements; - include_fixer::IncludeFixerActionFactory Factory(*XrefsDB, Replacements); + include_fixer::IncludeFixerActionFactory Factory(*XrefsDB, Replacements, + MinimizeIncludePaths); tool.run(&Factory); // Always succeeds. Modified: clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp?rev=267868&r1=267867&r2=267868&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp (original) +++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp Thu Apr 28 06:21:29 2016 @@ -7,9 +7,9 @@ // //===----------------------------------------------------------------------===// -#include "unittests/Tooling/RewriterTestContext.h" #include "InMemoryXrefsDB.h" #include "IncludeFixer.h" +#include "unittests/Tooling/RewriterTestContext.h" #include "clang/Tooling/Tooling.h" #include "gtest/gtest.h" using namespace clang; @@ -19,34 +19,43 @@ namespace include_fixer { namespace { static bool runOnCode(tooling::ToolAction *ToolAction, StringRef Code, - StringRef FileName) { + StringRef FileName, + const std::vector<std::string> &ExtraArgs) { llvm::IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem( new vfs::InMemoryFileSystem); llvm::IntrusiveRefCntPtr<FileManager> Files( new FileManager(FileSystemOptions(), InMemoryFileSystem)); + std::vector<std::string> Args = {"include_fixer", "-fsyntax-only", FileName}; + Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end()); tooling::ToolInvocation Invocation( - {std::string("include_fixer"), std::string("-fsyntax-only"), - FileName.str()}, - ToolAction, Files.get(), std::make_shared<PCHContainerOperations>()); + Args, ToolAction, Files.get(), + std::make_shared<PCHContainerOperations>()); InMemoryFileSystem->addFile(FileName, 0, llvm::MemoryBuffer::getMemBuffer(Code)); InMemoryFileSystem->addFile("foo.h", 0, llvm::MemoryBuffer::getMemBuffer("\n")); - InMemoryFileSystem->addFile("bar.h", 0, + InMemoryFileSystem->addFile("dir/bar.h", 0, + llvm::MemoryBuffer::getMemBuffer("\n")); + InMemoryFileSystem->addFile("dir/otherdir/qux.h", 0, llvm::MemoryBuffer::getMemBuffer("\n")); return Invocation.run(); } -static std::string runIncludeFixer(StringRef Code) { +static std::string runIncludeFixer( + StringRef Code, + const std::vector<std::string> &ExtraArgs = std::vector<std::string>()) { std::map<std::string, std::vector<std::string>> XrefsMap = { - {"std::string", {"<string>"}}, {"std::string::size_type", {"<string>"}}}; + {"std::string", {"<string>"}}, + {"std::string::size_type", {"<string>"}}, + {"a::b::foo", {"dir/otherdir/qux.h"}}, + }; auto XrefsDB = llvm::make_unique<include_fixer::InMemoryXrefsDB>(std::move(XrefsMap)); std::vector<clang::tooling::Replacement> Replacements; IncludeFixerActionFactory Factory(*XrefsDB, Replacements); - runOnCode(&Factory, Code, "input.cc"); + runOnCode(&Factory, Code, "input.cc", ExtraArgs); clang::RewriterTestContext Context; clang::FileID ID = Context.createInMemoryFile("input.cc", Code); clang::tooling::applyAllReplacements(Replacements, Context.Rewrite); @@ -59,9 +68,9 @@ TEST(IncludeFixer, Typo) { EXPECT_EQ( "// comment\n#include <string>\n#include \"foo.h\"\nstd::string foo;\n" - "#include \"bar.h\"\n", + "#include \"dir/bar.h\"\n", runIncludeFixer("// comment\n#include \"foo.h\"\nstd::string foo;\n" - "#include \"bar.h\"\n")); + "#include \"dir/bar.h\"\n")); EXPECT_EQ("#include <string>\n#include \"foo.h\"\nstd::string foo;\n", runIncludeFixer("#include \"foo.h\"\nstd::string foo;\n")); @@ -82,6 +91,24 @@ TEST(IncludeFixer, IncompleteType) { "namespace std {\nclass string;\n}\nstring foo;\n")); } +TEST(IncludeFixer, MinimizeInclude) { + std::vector<std::string> IncludePath = {"-Idir/"}; + EXPECT_EQ("#include \"otherdir/qux.h\"\na::b::foo bar;\n", + runIncludeFixer("a::b::foo bar;\n", IncludePath)); + + IncludePath = {"-isystemdir"}; + EXPECT_EQ("#include <otherdir/qux.h>\na::b::foo bar;\n", + runIncludeFixer("a::b::foo bar;\n", IncludePath)); + + IncludePath = {"-iquotedir"}; + EXPECT_EQ("#include \"otherdir/qux.h\"\na::b::foo bar;\n", + runIncludeFixer("a::b::foo bar;\n", IncludePath)); + + IncludePath = {"-Idir", "-Idir/otherdir"}; + EXPECT_EQ("#include \"qux.h\"\na::b::foo bar;\n", + runIncludeFixer("a::b::foo bar;\n", IncludePath)); +} + } // namespace } // namespace include_fixer } // namespace clang _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits