Author: ioeric Date: Mon Jun 13 14:05:07 2016 New Revision: 272576 URL: http://llvm.org/viewvc/llvm-project?rev=272576&view=rev Log: [include-fixer] only deduplicate symbols after matching symbols with the undefined identifier.
Summary: we should only deduplicate symbols after matching symbols with the undefined identifier. This patch tries to fix the situation where matching symbols are deleted when it has the same header as a non-matching symbol, which can prevent finding the correct header even if there is a matched symbol. Reviewers: bkramer Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D21290 Modified: clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp Modified: clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp?rev=272576&r1=272575&r2=272576&view=diff ============================================================================== --- clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp (original) +++ clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp Mon Jun 13 14:05:07 2016 @@ -67,8 +67,8 @@ SymbolIndexManager::search(llvm::StringR // Eventually we will either hit a class (namespaces aren't in the database // either) and can report that result. bool TookPrefix = false; - std::vector<std::string> Results; - while (Results.empty() && !Names.empty()) { + std::vector<clang::find_all_symbols::SymbolInfo> MatchedSymbols; + while (MatchedSymbols.empty() && !Names.empty()) { std::vector<clang::find_all_symbols::SymbolInfo> Symbols; for (const auto &DB : SymbolIndices) { auto Res = DB->search(Names.back().str()); @@ -78,8 +78,6 @@ SymbolIndexManager::search(llvm::StringR DEBUG(llvm::dbgs() << "Searching " << Names.back() << "... got " << Symbols.size() << " results...\n"); - rankByPopularity(Symbols); - for (const auto &Symbol : Symbols) { // Match the identifier name without qualifier. if (Symbol.getName() == Names.back()) { @@ -120,15 +118,7 @@ SymbolIndexManager::search(llvm::StringR Symbol.getSymbolKind() == SymbolInfo::SymbolKind::Macro)) continue; - // 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. - std::string FilePath = Symbol.getFilePath().str(); - Results.push_back((FilePath[0] == '"' || FilePath[0] == '<') - ? FilePath - : "\"" + FilePath + "\""); + MatchedSymbols.push_back(Symbol); } } } @@ -136,6 +126,20 @@ SymbolIndexManager::search(llvm::StringR TookPrefix = true; } + rankByPopularity(MatchedSymbols); + std::vector<std::string> Results; + for (const auto &Symbol : MatchedSymbols) { + // 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. + std::string FilePath = Symbol.getFilePath().str(); + Results.push_back((FilePath[0] == '"' || FilePath[0] == '<') + ? FilePath + : "\"" + FilePath + "\""); + } + return Results; } 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=272576&r1=272575&r2=272576&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp (original) +++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp Mon Jun 13 14:05:07 2016 @@ -60,13 +60,20 @@ static std::string runIncludeFixer( SymbolInfo("foo", SymbolInfo::SymbolKind::Class, "\"dir/otherdir/qux.h\"", 1, {{SymbolInfo::ContextType::Namespace, "b"}, {SymbolInfo::ContextType::Namespace, "a"}}), - SymbolInfo("bar", SymbolInfo::SymbolKind::Class, "\"bar.h\"", - 1, {{SymbolInfo::ContextType::Namespace, "b"}, - {SymbolInfo::ContextType::Namespace, "a"}}), - SymbolInfo("Green", SymbolInfo::SymbolKind::Class, "\"color.h\"", - 1, {{SymbolInfo::ContextType::EnumDecl, "Color"}, - {SymbolInfo::ContextType::Namespace, "b"}, - {SymbolInfo::ContextType::Namespace, "a"}}), + SymbolInfo("bar", SymbolInfo::SymbolKind::Class, "\"bar.h\"", 1, + {{SymbolInfo::ContextType::Namespace, "b"}, + {SymbolInfo::ContextType::Namespace, "a"}}), + SymbolInfo("Green", SymbolInfo::SymbolKind::Class, "\"color.h\"", 1, + {{SymbolInfo::ContextType::EnumDecl, "Color"}, + {SymbolInfo::ContextType::Namespace, "b"}, + {SymbolInfo::ContextType::Namespace, "a"}}), + SymbolInfo("Vector", SymbolInfo::SymbolKind::Class, "\"Vector.h\"", 1, + {{SymbolInfo::ContextType::Namespace, "__a"}, + {SymbolInfo::ContextType::Namespace, "a"}}, + /*num_occurrences=*/2), + SymbolInfo("Vector", SymbolInfo::SymbolKind::Class, "\"Vector.h\"", 2, + {{SymbolInfo::ContextType::Namespace, "a"}}, + /*num_occurrences=*/1), }; auto SymbolIndexMgr = llvm::make_unique<include_fixer::SymbolIndexManager>(); SymbolIndexMgr->addSymbolIndex( @@ -209,6 +216,11 @@ TEST(IncludeFixer, InsertAndSortSingleHe EXPECT_EQ(Expected, runIncludeFixer(Code)); } +TEST(IncludeFixer, DoNotDeleteMatchedSymbol) { + EXPECT_EQ("#include \"Vector.h\"\na::Vector v;", + runIncludeFixer("a::Vector v;")); +} + } // 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
