kadircet updated this revision to Diff 296402.
kadircet marked 6 inline comments as done.
kadircet added a comment.
- Perform sub-scope matching rather than substring match for partial namespaces.
- Apply a penalty for partially matching namespaces.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88814/new/
https://reviews.llvm.org/D88814
Files:
clang-tools-extra/clangd/FindSymbols.cpp
clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -141,7 +141,10 @@
EXPECT_THAT(getSymbols(TU, "::"), ElementsAre(QName("ans1")));
EXPECT_THAT(getSymbols(TU, "::a"), ElementsAre(QName("ans1")));
EXPECT_THAT(getSymbols(TU, "ans1::"),
- UnorderedElementsAre(QName("ans1::ai1"), QName("ans1::ans2")));
+ UnorderedElementsAre(QName("ans1::ai1"), QName("ans1::ans2"),
+ QName("ans1::ans2::ai2")));
+ EXPECT_THAT(getSymbols(TU, "ans2::"),
+ UnorderedElementsAre(QName("ans1::ans2::ai2")));
EXPECT_THAT(getSymbols(TU, "::ans1"), ElementsAre(QName("ans1")));
EXPECT_THAT(getSymbols(TU, "::ans1::"),
UnorderedElementsAre(QName("ans1::ai1"), QName("ans1::ans2")));
@@ -256,6 +259,17 @@
EXPECT_THAT(getSymbols(TU, "::"), ElementsAre(QName("func"), QName("ns")));
}
+TEST(WorkspaceSymbols, RankingPartilNamespace) {
+ TestTU TU;
+ TU.Code = R"cpp(
+ namespace ns1 {
+ namespace ns2 { struct Foo {}; }
+ }
+ namespace ns2 { struct FooB {}; })cpp";
+ EXPECT_THAT(getSymbols(TU, "ns2::f"),
+ ElementsAre(QName("ns2::FooB"), QName("ns1::ns2::Foo")));
+}
+
TEST(WorkspaceSymbols, WithLimit) {
TestTU TU;
TU.AdditionalFiles["foo.h"] = R"cpp(
Index: clang-tools-extra/clangd/FindSymbols.cpp
===================================================================
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -18,6 +18,9 @@
#include "clang/Index/IndexDataConsumer.h"
#include "clang/Index/IndexSymbol.h"
#include "clang/Index/IndexingAction.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
@@ -38,6 +41,37 @@
}
};
+// Returns true if \p Query can be found as a sub-scope inside \p Scope.
+bool approximateScopeMatch(llvm::StringRef Scope,
+ llvm::ArrayRef<llvm::StringRef> Query) {
+ if (Query.empty())
+ return true;
+
+ llvm::SmallVector<llvm::StringRef, 2> Scopes;
+ Scope.split(Scopes, "::");
+
+ llvm::ArrayRef<llvm::StringRef> ScopesRef(Scopes);
+ // We perform a naive string search below, assuming scopes as our characters.
+ while (ScopesRef.size() >= Query.size()) {
+ // Trim Scopes from left until we find the first scope of Query. E.g:
+ // Scopes: a::b::c::
+ // Query: b::c::
+ // This will trim `a::` from Scopes.
+ ScopesRef = ScopesRef.drop_while(
+ [FirstQueryScope = Query.front()](llvm::StringRef Scope) {
+ return Scope != FirstQueryScope;
+ });
+ if (ScopesRef.size() < Query.size())
+ break;
+ // Check if ScopesRef prefix matches Query.
+ if (ScopesRef.take_front(Query.size()).equals(Query))
+ return true;
+ // If not try with the rest.
+ ScopesRef = ScopesRef.drop_front();
+ }
+ return false;
+}
+
} // namespace
llvm::Expected<Location> indexToLSPLocation(const SymbolLocation &Loc,
@@ -71,25 +105,48 @@
if (Query.empty() || !Index)
return Result;
+ // Lookup for qualified names are performed as:
+ // - Exact namespaces are boosted by the index.
+ // - Approximate matches are (sub-scope match) included via AnyScope logic.
+ // - Non-matching namespaces (no sub-scope match) are post-filtered.
auto Names = splitQualifiedName(Query);
FuzzyFindRequest Req;
Req.Query = std::string(Names.second);
- // FuzzyFind doesn't want leading :: qualifier
- bool IsGlobalQuery = Names.first.consume_front("::");
- // Restrict results to the scope in the query string if present (global or
- // not).
- if (IsGlobalQuery || !Names.first.empty())
+ // FuzzyFind doesn't want leading :: qualifier.
+ auto HasLeadingColons = Names.first.consume_front("::");
+ // Limit the query to specific namespace if it is fully-qualified.
+ Req.AnyScope = !HasLeadingColons;
+ // Boost symbols from desired namespace.
+ if (HasLeadingColons || !Names.first.empty())
Req.Scopes = {std::string(Names.first)};
- else
- Req.AnyScope = true;
- if (Limit)
+ if (Limit) {
Req.Limit = Limit;
+ // If we are boosting a specific scope allow more results to be retrieved,
+ // since some symbols from preferred namespaces might not make the cut.
+ if (Req.AnyScope && !Req.Scopes.empty())
+ *Req.Limit *= 5;
+ }
TopN<ScoredSymbolInfo, ScoredSymbolGreater> Top(
Req.Limit ? *Req.Limit : std::numeric_limits<size_t>::max());
FuzzyMatcher Filter(Req.Query);
- Index->fuzzyFind(Req, [HintPath, &Top, &Filter](const Symbol &Sym) {
+
+ llvm::SmallVector<llvm::StringRef, 2> ReqScopes;
+ Names.first.consume_back("::");
+ if (!Names.first.empty())
+ Names.first.split(ReqScopes, "::");
+ Index->fuzzyFind(Req, [HintPath, &Top, &Filter, &ReqScopes,
+ AnyScope = Req.AnyScope,
+ ReqScope = Names.first](const Symbol &Sym) {
+ std::string Scope = std::string(Sym.Scope);
+ llvm::StringRef ScopeRef = Scope;
+ ScopeRef.consume_back("::");
+ // Fuzzyfind might return symbols from irrelevant namespaces if query was
+ // not fully-qualified, drop those.
+ if (AnyScope && !approximateScopeMatch(ScopeRef, ReqScopes))
+ return;
+
auto Loc = symbolToLocation(Sym, HintPath);
if (!Loc) {
log("Workspace symbols: {0}", Loc.takeError());
@@ -97,9 +154,6 @@
}
SymbolKind SK = indexSymbolKindToSymbolKind(Sym.SymInfo.Kind);
- std::string Scope = std::string(Sym.Scope);
- llvm::StringRef ScopeRef = Scope;
- ScopeRef.consume_back("::");
SymbolInformation Info = {(Sym.Name + Sym.TemplateSpecializationArgs).str(),
SK, *Loc, std::string(ScopeRef)};
@@ -108,6 +162,8 @@
SymbolRelevanceSignals Relevance;
Relevance.Name = Sym.Name;
Relevance.Query = SymbolRelevanceSignals::Generic;
+ // If symbol and request scopes do not match exactly, apply a penalty.
+ Relevance.InBaseClass = AnyScope && ScopeRef != ReqScope;
if (auto NameMatch = Filter.match(Sym.Name))
Relevance.NameMatch = *NameMatch;
else {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
