Index: include-fixer/CMakeLists.txt =================================================================== --- include-fixer/CMakeLists.txt +++ include-fixer/CMakeLists.txt @@ -5,6 +5,7 @@ add_clang_library(clangIncludeFixer IncludeFixer.cpp InMemoryXrefsDB.cpp + XrefsDBManager.cpp YamlXrefsDB.cpp LINK_LIBS Index: include-fixer/InMemoryXrefsDB.h =================================================================== --- include-fixer/InMemoryXrefsDB.h +++ include-fixer/InMemoryXrefsDB.h @@ -21,13 +21,15 @@ /// Xref database with fixed content. class InMemoryXrefsDB : public XrefsDB { public: - InMemoryXrefsDB(std::map> LookupTable) - : LookupTable(std::move(LookupTable)) {} + InMemoryXrefsDB( + const std::map> &LookupTable); - std::vector search(llvm::StringRef Identifier) override; + std::vector + search(llvm::StringRef Identifier) override; private: - std::map> LookupTable; + std::map> + LookupTable; }; } // namespace include_fixer Index: include-fixer/InMemoryXrefsDB.cpp =================================================================== --- include-fixer/InMemoryXrefsDB.cpp +++ include-fixer/InMemoryXrefsDB.cpp @@ -9,10 +9,34 @@ #include "InMemoryXrefsDB.h" +using clang::find_all_symbols::SymbolInfo; + namespace clang { namespace include_fixer { -std::vector InMemoryXrefsDB::search(llvm::StringRef Identifier) { +InMemoryXrefsDB::InMemoryXrefsDB( + const std::map> &LookupTable) { + for (const auto &Entry : LookupTable) { + llvm::StringRef Identifier(Entry.first); + llvm::SmallVector Names; + Identifier.split(Names, "::"); + for (const auto &Header : Entry.second) { + // FIXME: create a complete instance with static member function when it + // is implemented. + SymbolInfo Info; + Info.Name = Names.back(); + Info.FilePath = Header; + for (auto IdentiferContext = Names.rbegin() + 1; + IdentiferContext != Names.rend(); ++IdentiferContext) { + Info.Contexts.push_back( + {SymbolInfo::ContextType::Namespace, *IdentiferContext}); + } + this->LookupTable[Info.Name].push_back(Info); + } + } +} + +std::vector InMemoryXrefsDB::search(llvm::StringRef Identifier) { auto I = LookupTable.find(Identifier); if (I != LookupTable.end()) return I->second; Index: include-fixer/IncludeFixer.h =================================================================== --- include-fixer/IncludeFixer.h +++ include-fixer/IncludeFixer.h @@ -10,7 +10,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXER_H #define LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXER_H -#include "XrefsDB.h" +#include "XrefsDBManager.h" #include "clang/Tooling/Core/Replacement.h" #include "clang/Tooling/Tooling.h" #include @@ -27,11 +27,12 @@ class IncludeFixerActionFactory : public clang::tooling::ToolAction { public: - /// \param Xrefs A source for matching symbols to header files. + /// \param XrefsDBMgr 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 &Replacements, + XrefsDBManager &XrefsDBMgr, + std::vector &Replacements, bool MinimizeIncludePaths = true); ~IncludeFixerActionFactory() override; @@ -43,7 +44,7 @@ private: /// The client to use to find cross-references. - XrefsDB &Xrefs; + XrefsDBManager &XrefsDBMgr; /// Replacements are written here. std::vector &Replacements; Index: include-fixer/IncludeFixer.cpp =================================================================== --- include-fixer/IncludeFixer.cpp +++ include-fixer/IncludeFixer.cpp @@ -60,8 +60,8 @@ class Action : public clang::ASTFrontendAction, public clang::ExternalSemaSource { public: - explicit Action(XrefsDB &Xrefs, bool MinimizeIncludePaths) - : Xrefs(Xrefs), MinimizeIncludePaths(MinimizeIncludePaths) {} + explicit Action(XrefsDBManager &XrefsDBMgr, bool MinimizeIncludePaths) + : XrefsDBMgr(XrefsDBMgr), MinimizeIncludePaths(MinimizeIncludePaths) {} std::unique_ptr CreateASTConsumer(clang::CompilerInstance &Compiler, @@ -224,7 +224,7 @@ DEBUG(llvm::dbgs() << "Looking up " << Query << " ... "); std::string error_text; - auto SearchReply = Xrefs.search(Query); + auto SearchReply = XrefsDBMgr.search(Query); DEBUG(llvm::dbgs() << SearchReply.size() << " replies\n"); if (SearchReply.empty()) return clang::TypoCorrection(); @@ -240,7 +240,7 @@ } /// The client to use to find cross-references. - XrefsDB &Xrefs; + XrefsDBManager &XrefsDBMgr; // Remeber things we looked up to avoid querying things twice. llvm::StringSet<> SeenQueries; @@ -303,9 +303,10 @@ } // namespace IncludeFixerActionFactory::IncludeFixerActionFactory( - XrefsDB &Xrefs, std::vector &Replacements, + XrefsDBManager &XrefsDBMgr, + std::vector &Replacements, bool MinimizeIncludePaths) - : Xrefs(Xrefs), Replacements(Replacements), + : XrefsDBMgr(XrefsDBMgr), Replacements(Replacements), MinimizeIncludePaths(MinimizeIncludePaths) {} IncludeFixerActionFactory::~IncludeFixerActionFactory() = default; @@ -329,7 +330,7 @@ // Run the parser, gather missing includes. auto ScopedToolAction = - llvm::make_unique(Xrefs, MinimizeIncludePaths); + llvm::make_unique(XrefsDBMgr, MinimizeIncludePaths); Compiler.ExecuteAction(*ScopedToolAction); // Generate replacements. Index: include-fixer/XrefsDB.h =================================================================== --- include-fixer/XrefsDB.h +++ include-fixer/XrefsDB.h @@ -10,26 +10,26 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_XREFSDB_H #define LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_XREFSDB_H +#include "find-all-symbols/SymbolInfo.h" #include "llvm/ADT/StringRef.h" #include namespace clang { namespace include_fixer { -/// This class provides an interface for finding the header files corresponding -/// to an indentifier in the source code. +/// This class provides an interface for finding all `SymbolInfo`s corresponding +/// to a symbol name from a symbol database. class XrefsDB { public: virtual ~XrefsDB() = default; - /// Search for header files to be included for an identifier. - /// \param Identifier The identifier being searched for. May or may not be - /// fully qualified. - /// \returns A list of inclusion candidates, in a format ready for being - /// pasted after an #include token. + /// Search for all `SymbolInfo`s corresponding to an identifier. + /// \param Identifier The unqualified identifier being searched for. + /// \returns A list of `SymbolInfo` candidates. // FIXME: Expose the type name so we can also insert using declarations (or // fix the usage) - virtual std::vector search(llvm::StringRef Identifier) = 0; + virtual std::vector + search(llvm::StringRef Identifier) = 0; }; } // namespace include_fixer Index: include-fixer/XrefsDBManager.h =================================================================== --- include-fixer/XrefsDBManager.h +++ include-fixer/XrefsDBManager.h @@ -1,4 +1,4 @@ -//===-- XrefsDB.h - Interface for symbol-header matching --------*- C++ -*-===// +//===-- XrefsDBManager.h - Managing multiple XrefsDBs -----------*- C++ -*-===// // // The LLVM Compiler Infrastructure // @@ -7,20 +7,22 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_XREFSDB_H -#define LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_XREFSDB_H +#ifndef LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_XREFSDBMANAGER_H +#define LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_XREFSDBMANAGER_H +#include "XrefsDB.h" #include "llvm/ADT/StringRef.h" -#include namespace clang { namespace include_fixer { /// This class provides an interface for finding the header files corresponding -/// to an indentifier in the source code. -class XrefsDB { +/// to an indentifier in the source code from multiple symbol databases. +class XrefsDBManager { public: - virtual ~XrefsDB() = default; + void addXrefsDB(std::unique_ptr DB) { + XrefsDBs.push_back(std::move(DB)); + } /// Search for header files to be included for an identifier. /// \param Identifier The identifier being searched for. May or may not be @@ -29,10 +31,15 @@ /// pasted after an #include token. // FIXME: Expose the type name so we can also insert using declarations (or // fix the usage) - virtual std::vector search(llvm::StringRef Identifier) = 0; + // FIXME: Move mapping from SymbolInfo to headers out of + // XrefsDBManager::search and return SymbolInfos instead of header paths. + std::vector search(llvm::StringRef Identifier) const; + +private: + std::vector> XrefsDBs; }; } // namespace include_fixer } // namespace clang -#endif // LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_XREFSDB_H +#endif Index: include-fixer/XrefsDBManager.cpp =================================================================== --- include-fixer/XrefsDBManager.cpp +++ include-fixer/XrefsDBManager.cpp @@ -1,4 +1,4 @@ -//===-- YamlXrefsDB.cpp ---------------------------------------------------===// +//===-- XrefsDBManager.cpp - Managing multiple XrefsDBs ---------*- C++ -*-===// // // The LLVM Compiler Infrastructure // @@ -7,35 +7,33 @@ // //===----------------------------------------------------------------------===// -#include "YamlXrefsDB.h" - +#include "XrefsDBManager.h" +#include "find-all-symbols/SymbolInfo.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/Support/FileSystem.h" -#include "llvm/Support/MemoryBuffer.h" -#include -#include +#include "llvm/Support/Debug.h" + +#define DEBUG_TYPE "include-fixer" namespace clang { namespace include_fixer { -YamlXrefsDB::YamlXrefsDB(llvm::StringRef FilePath) { - int ReadFD = 0; - if (llvm::sys::fs::openFileForRead(FilePath, ReadFD)) - return; - auto Buffer = llvm::MemoryBuffer::getOpenFile(ReadFD, FilePath, -1); - if (!Buffer) - return; - Symbols = clang::find_all_symbols::ReadSymbolInfosFromYAML( - Buffer.get()->getBuffer()); -} - -std::vector YamlXrefsDB::search(llvm::StringRef Identifier) { - llvm::SmallVector Names; - std::vector Results; - +std::vector +XrefsDBManager::search(llvm::StringRef Identifier) const { // The identifier may be fully qualified, so split it and get all the context // names. + llvm::SmallVector Names; Identifier.split(Names, "::"); + + std::vector Symbols; + for (const auto &DB : XrefsDBs) { + auto Res = DB->search(Names.back().str()); + Symbols.insert(Symbols.end(), Res.begin(), Res.end()); + } + + DEBUG(llvm::dbgs() << "Searching " << Names.back() << "... got " + << Symbols.size() << " results...\n"); + + std::vector Results; for (const auto &Symbol : Symbols) { // Match the identifier name without qualifier. if (Symbol.Name == Names.back()) { @@ -53,7 +51,15 @@ } if (IsMatched) { - Results.push_back("\"" + Symbol.FilePath + "\""); + // FIXME: file path should never be in the form of <...> or "...", but + // the unit test with fixed database use <...> file path, which might + // need to be changed. + // FIXME: if the file path is a system header name, we want to use angle + // brackets. + Results.push_back( + (Symbol.FilePath[0] == '"' || Symbol.FilePath[0] == '<') + ? Symbol.FilePath + : "\"" + Symbol.FilePath + "\""); } } } Index: include-fixer/YamlXrefsDB.h =================================================================== --- include-fixer/YamlXrefsDB.h +++ include-fixer/YamlXrefsDB.h @@ -23,7 +23,8 @@ public: YamlXrefsDB(llvm::StringRef FilePath); - std::vector search(llvm::StringRef Identifier) override; + std::vector + search(llvm::StringRef Identifier) override; private: std::vector Symbols; Index: include-fixer/YamlXrefsDB.cpp =================================================================== --- include-fixer/YamlXrefsDB.cpp +++ include-fixer/YamlXrefsDB.cpp @@ -15,6 +15,8 @@ #include #include +using clang::find_all_symbols::SymbolInfo; + namespace clang { namespace include_fixer { @@ -29,33 +31,11 @@ Buffer.get()->getBuffer()); } -std::vector YamlXrefsDB::search(llvm::StringRef Identifier) { - llvm::SmallVector Names; - std::vector Results; - - // The identifier may be fully qualified, so split it and get all the context - // names. - Identifier.split(Names, "::"); +std::vector YamlXrefsDB::search(llvm::StringRef Identifier) { + std::vector Results; for (const auto &Symbol : Symbols) { - // Match the identifier name without qualifier. - if (Symbol.Name == Names.back()) { - bool IsMatched = true; - auto SymbolContext = Symbol.Contexts.begin(); - // Match the remaining context names. - for (auto IdentiferContext = Names.rbegin() + 1; - IdentiferContext != Names.rend() && - SymbolContext != Symbol.Contexts.end(); - ++IdentiferContext, ++SymbolContext) { - if (SymbolContext->second != *IdentiferContext) { - IsMatched = false; - break; - } - } - - if (IsMatched) { - Results.push_back("\"" + Symbol.FilePath + "\""); - } - } + if (Symbol.Name == Identifier) + Results.push_back(Symbol); } return Results; } Index: include-fixer/find-all-symbols/SymbolInfo.h =================================================================== --- include-fixer/find-all-symbols/SymbolInfo.h +++ include-fixer/find-all-symbols/SymbolInfo.h @@ -21,6 +21,7 @@ namespace find_all_symbols { /// \brief Contains all information for a Symbol. +// FIXME: add static members for creating complete instances. struct SymbolInfo { enum SymbolKind { Function, Index: include-fixer/tool/ClangIncludeFixer.cpp =================================================================== --- include-fixer/tool/ClangIncludeFixer.cpp +++ include-fixer/tool/ClangIncludeFixer.cpp @@ -9,6 +9,7 @@ #include "InMemoryXrefsDB.h" #include "IncludeFixer.h" +#include "XrefsDBManager.h" #include "YamlXrefsDB.h" #include "clang/Frontend/TextDiagnosticPrinter.h" #include "clang/Rewrite/Core/Rewriter.h" @@ -48,8 +49,8 @@ tooling::ClangTool tool(options.getCompilations(), options.getSourcePathList()); - // Set up the data source. - std::unique_ptr XrefsDB; + // Set up data source. + auto XrefsDBMgr = llvm::make_unique(); switch (DatabaseFormat) { case fixed: { // Parse input and fill the database with it. @@ -67,19 +68,20 @@ Headers.push_back(Header.trim()); XrefsMap[Split.first.trim()] = std::move(Headers); } - XrefsDB = - llvm::make_unique(std::move(XrefsMap)); + XrefsDBMgr->addXrefsDB( + llvm::make_unique(std::move(XrefsMap))); break; } case yaml: { - XrefsDB = llvm::make_unique(Input); + XrefsDBMgr->addXrefsDB( + llvm::make_unique(Input)); break; } } // Now run our tool. std::vector Replacements; - include_fixer::IncludeFixerActionFactory Factory(*XrefsDB, Replacements, + include_fixer::IncludeFixerActionFactory Factory(*XrefsDBMgr, Replacements, MinimizeIncludePaths); tool.run(&Factory); // Always succeeds. Index: unittests/include-fixer/IncludeFixerTest.cpp =================================================================== --- unittests/include-fixer/IncludeFixerTest.cpp +++ unittests/include-fixer/IncludeFixerTest.cpp @@ -9,6 +9,7 @@ #include "InMemoryXrefsDB.h" #include "IncludeFixer.h" +#include "XrefsDBManager.h" #include "unittests/Tooling/RewriterTestContext.h" #include "clang/Tooling/Tooling.h" #include "gtest/gtest.h" @@ -51,10 +52,12 @@ {"std::string::size_type", {""}}, {"a::b::foo", {"dir/otherdir/qux.h"}}, }; - auto XrefsDB = - llvm::make_unique(std::move(XrefsMap)); + auto XrefsDBMgr = llvm::make_unique(); + XrefsDBMgr->addXrefsDB( + llvm::make_unique(std::move(XrefsMap))); + std::vector Replacements; - IncludeFixerActionFactory Factory(*XrefsDB, Replacements); + IncludeFixerActionFactory Factory(*XrefsDBMgr, Replacements); runOnCode(&Factory, Code, "input.cc", ExtraArgs); clang::RewriterTestContext Context; clang::FileID ID = Context.createInMemoryFile("input.cc", Code); @@ -79,8 +82,10 @@ "#include \n#include \"foo.h\"\nstd::string::size_type foo;\n", runIncludeFixer("#include \"foo.h\"\nstd::string::size_type foo;\n")); - // The fixed xrefs db doesn't know how to handle string without std::. - EXPECT_EQ("string foo;\n", runIncludeFixer("string foo;\n")); + // string without "std::" can also be fixed since fixed db results go through + // XrefsDBManager, and XrefsDBManager matches unqualified identifiers too. + EXPECT_EQ("#include \nstring foo;\n", + runIncludeFixer("string foo;\n")); } TEST(IncludeFixer, IncompleteType) {