ioeric created this revision.
ioeric added a reviewer: bkramer.
ioeric added a subscriber: cfe-commits.
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.
http://reviews.llvm.org/D21290
Files:
include-fixer/SymbolIndexManager.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===================================================================
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -60,13 +60,20 @@
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 @@
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
Index: include-fixer/SymbolIndexManager.cpp
===================================================================
--- include-fixer/SymbolIndexManager.cpp
+++ include-fixer/SymbolIndexManager.cpp
@@ -67,8 +67,8 @@
// 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 @@
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,22 +118,28 @@
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);
}
}
}
Names.pop_back();
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
