ioeric updated this revision to Diff 151169.
ioeric added a comment.
- Introduced a one-per-query structure for relevance signals; use
multiplication for proximity; simplify tests a bit; separate index and sema
proximity scores.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47935
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/CodeComplete.cpp
clangd/FindSymbols.cpp
clangd/Quality.cpp
clangd/Quality.h
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
unittests/clangd/ClangdTests.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/QualityTests.cpp
unittests/clangd/TestFS.cpp
unittests/clangd/URITests.cpp
Index: unittests/clangd/URITests.cpp
===================================================================
--- unittests/clangd/URITests.cpp
+++ unittests/clangd/URITests.cpp
@@ -14,46 +14,20 @@
namespace clang {
namespace clangd {
+
+// Force the unittest URI scheme to be linked,
+extern volatile int UnittestSchemeAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED UnittestSchemeAnchorDest =
+ UnittestSchemeAnchorSource;
+
namespace {
using ::testing::AllOf;
MATCHER_P(Scheme, S, "") { return arg.scheme() == S; }
MATCHER_P(Authority, A, "") { return arg.authority() == A; }
MATCHER_P(Body, B, "") { return arg.body() == B; }
-// Assume all files in the schema have a "test-root/" root directory, and the
-// schema path is the relative path to the root directory.
-// So the schema of "/some-dir/test-root/x/y/z" is "test:x/y/z".
-class TestScheme : public URIScheme {
-public:
- static const char *Scheme;
-
- static const char *TestRoot;
-
- llvm::Expected<std::string>
- getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
- llvm::StringRef HintPath) const override {
- auto Pos = HintPath.find(TestRoot);
- assert(Pos != llvm::StringRef::npos);
- return (HintPath.substr(0, Pos + llvm::StringRef(TestRoot).size()) + Body)
- .str();
- }
-
- llvm::Expected<URI>
- uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
- auto Pos = AbsolutePath.find(TestRoot);
- assert(Pos != llvm::StringRef::npos);
- return URI(Scheme, /*Authority=*/"",
- AbsolutePath.substr(Pos + llvm::StringRef(TestRoot).size()));
- }
-};
-
-const char *TestScheme::Scheme = "unittest";
-const char *TestScheme::TestRoot = "/test-root/";
-
-static URISchemeRegistry::Add<TestScheme> X(TestScheme::Scheme, "Test schema");
-
std::string createOrDie(llvm::StringRef AbsolutePath,
llvm::StringRef Scheme = "file") {
auto Uri = URI::create(AbsolutePath, Scheme);
Index: unittests/clangd/TestFS.cpp
===================================================================
--- unittests/clangd/TestFS.cpp
+++ unittests/clangd/TestFS.cpp
@@ -7,6 +7,7 @@
//
//===----------------------------------------------------------------------===//
#include "TestFS.h"
+#include "URI.h"
#include "llvm/Support/Errc.h"
#include "gtest/gtest.h"
@@ -62,5 +63,50 @@
return Path.str();
}
+// Assume all files in the schema have a "test-root/" root directory, and the
+// schema path is the relative path to the root directory.
+// So the schema of "/some-dir/test-root/x/y/z" is "test:x/y/z".
+class TestScheme : public URIScheme {
+public:
+ static const char *Scheme;
+
+ static const char *TestRoot;
+
+ llvm::Expected<std::string>
+ getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
+ llvm::StringRef HintPath) const override {
+ auto Pos = HintPath.find(TestRoot);
+ assert(Pos != llvm::StringRef::npos);
+ return (HintPath.substr(0, Pos + llvm::StringRef(TestRoot).size()) + Body)
+ .str();
+ }
+
+ llvm::Expected<URI>
+ uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
+ auto Pos = AbsolutePath.find(TestRoot);
+ if (Pos == llvm::StringRef::npos)
+ return llvm::make_error<llvm::StringError>(
+ llvm::Twine("Directory ") + TestRoot + " not found in path " +
+ AbsolutePath,
+ llvm::inconvertibleErrorCode());
+
+ return URI(Scheme, /*Authority=*/"",
+ AbsolutePath.substr(Pos + llvm::StringRef(TestRoot).size()));
+ }
+};
+
+const char *TestScheme::Scheme = "unittest";
+#ifdef _WIN32
+const char *TestScheme::TestRoot = "\\test-root\\";
+#else
+const char *TestScheme::TestRoot = "/test-root/";
+#endif
+
+static URISchemeRegistry::Add<TestScheme> X(TestScheme::Scheme, "Test schema");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the plugin.
+volatile int UnittestSchemeAnchorSource = 0;
+
} // namespace clangd
} // namespace clang
Index: unittests/clangd/QualityTests.cpp
===================================================================
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -18,12 +18,19 @@
//===----------------------------------------------------------------------===//
#include "Quality.h"
+#include "TestFS.h"
#include "TestTU.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
namespace clang {
namespace clangd {
+
+// Force the unittest URI scheme to be linked,
+extern volatile int UnittestSchemeAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED UnittestSchemeAnchorDest =
+ UnittestSchemeAnchorSource;
+
namespace {
TEST(QualityTests, SymbolQualitySignalExtraction) {
@@ -77,33 +84,48 @@
)cpp";
auto AST = Test.build();
- SymbolRelevanceSignals Relevance;
+ SymbolRelevanceContext Context(/*ProximityPaths=*/{});
+ {
+ SymbolRelevanceSignals Relevance(Context);
Relevance.merge(CodeCompletionResult(&findDecl(AST, "deprecated"),
/*Priority=*/42, nullptr, false,
/*Accessible=*/false));
- EXPECT_EQ(Relevance.NameMatch, SymbolRelevanceSignals().NameMatch);
+ EXPECT_EQ(Relevance.NameMatch, SymbolRelevanceSignals(Context).NameMatch);
EXPECT_TRUE(Relevance.Forbidden);
EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
-
- Relevance = {};
- Relevance.merge(CodeCompletionResult(&findDecl(AST, "main"), 42));
- EXPECT_FLOAT_EQ(Relevance.ProximityScore, 1.0) << "Decl in current file";
- Relevance = {};
- Relevance.merge(CodeCompletionResult(&findDecl(AST, "header"), 42));
- EXPECT_FLOAT_EQ(Relevance.ProximityScore, 0.0) << "Decl from header";
- Relevance = {};
- Relevance.merge(CodeCompletionResult(&findDecl(AST, "header_main"), 42));
- EXPECT_FLOAT_EQ(Relevance.ProximityScore, 1.0) << "Current file and header";
-
- Relevance = {};
- Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "X"), 42));
- EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FileScope);
- Relevance = {};
- Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "y"), 42));
- EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::ClassScope);
- Relevance = {};
- Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "z"), 42));
- EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FunctionScope);
+ }
+ {
+ SymbolRelevanceSignals Relevance(Context);
+ Relevance.merge(CodeCompletionResult(&findDecl(AST, "main"), 42));
+ EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 1.0)
+ << "Decl in current file";
+ }
+ {
+ SymbolRelevanceSignals Relevance(Context);
+ Relevance.merge(CodeCompletionResult(&findDecl(AST, "header"), 42));
+ EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 0.6) << "Decl from header";
+ }
+ {
+ SymbolRelevanceSignals Relevance(Context);
+ Relevance.merge(CodeCompletionResult(&findDecl(AST, "header_main"), 42));
+ EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 1.0)
+ << "Current file and header";
+ }
+ {
+ SymbolRelevanceSignals Relevance(Context);
+ Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "X"), 42));
+ EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FileScope);
+ }
+ {
+ SymbolRelevanceSignals Relevance(Context);
+ Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "y"), 42));
+ EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::ClassScope);
+ }
+ {
+ SymbolRelevanceSignals Relevance(Context);
+ Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "z"), 42));
+ EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FunctionScope);
+ }
}
// Do the signals move the scores in the direction we expect?
@@ -133,22 +155,28 @@
}
TEST(QualityTests, SymbolRelevanceSignalsSanity) {
- SymbolRelevanceSignals Default;
+ SymbolRelevanceContext Context(/*ProximityPaths=*/{});
+ SymbolRelevanceSignals Default(Context);
EXPECT_EQ(Default.evaluate(), 1);
- SymbolRelevanceSignals Forbidden;
+ SymbolRelevanceSignals Forbidden(Context);
Forbidden.Forbidden = true;
EXPECT_LT(Forbidden.evaluate(), Default.evaluate());
- SymbolRelevanceSignals PoorNameMatch;
+ SymbolRelevanceSignals PoorNameMatch(Context);
PoorNameMatch.NameMatch = 0.2f;
EXPECT_LT(PoorNameMatch.evaluate(), Default.evaluate());
- SymbolRelevanceSignals WithProximity;
- WithProximity.ProximityScore = 0.2f;
+ SymbolRelevanceSignals WithProximity(Context);
+ WithProximity.SemaProximityScore = 0.2f;
EXPECT_GT(WithProximity.evaluate(), Default.evaluate());
- SymbolRelevanceSignals Scoped;
+ SymbolRelevanceSignals WithBetterProximity(Context);
+ WithBetterProximity.IndexProximityScore = 0.3f;
+ WithBetterProximity.SemaProximityScore = 0.2f;
+ EXPECT_GT(WithBetterProximity.evaluate(), WithProximity.evaluate());
+
+ SymbolRelevanceSignals Scoped(Context);
Scoped.Scope = SymbolRelevanceSignals::FileScope;
EXPECT_EQ(Scoped.evaluate(), Default.evaluate());
Scoped.Query = SymbolRelevanceSignals::CodeComplete;
@@ -167,6 +195,77 @@
EXPECT_LT(sortText(0, "a"), sortText(0, "z"));
}
+// {a,b,c} becomes /clangd-test/a/b/c
+std::string joinPaths(llvm::ArrayRef<StringRef> Parts) {
+ return testPath(
+ llvm::join(Parts.begin(), Parts.end(), llvm::sys::path::get_separator()));
+}
+
+static constexpr float ProximityBase = 0.7;
+
+// Calculates a proximity score for an index symbol with declaration file
+// SymPath with the given URI cheme in \p Context.
+float IndexProximity(const SymbolRelevanceContext &Context, StringRef SymPath,
+ StringRef Scheme = "file") {
+ Symbol Sym;
+ std::string Storage;
+ auto U = URI::create(SymPath, Scheme);
+ EXPECT_TRUE(static_cast<bool>(U)) << llvm::toString(U.takeError());
+ Storage = U->toString();
+ Sym.CanonicalDeclaration.FileURI = Storage;
+
+ SymbolRelevanceSignals Relevance(Context);
+ Relevance.merge(Sym);
+ return Relevance.IndexProximityScore;
+}
+
+TEST(QualityTests, IndexSymbolProximityScores) {
+ SymbolRelevanceContext Context(
+ /*ProximityPaths=*/{joinPaths({"a", "b", "c", "d", "x"})});
+
+ EXPECT_FLOAT_EQ(IndexProximity(Context, joinPaths({"a", "b", "c", "d", "x"})),
+ 1);
+ EXPECT_FLOAT_EQ(IndexProximity(Context, joinPaths({"a", "b", "c", "d", "y"})),
+ ProximityBase);
+ EXPECT_FLOAT_EQ(IndexProximity(Context, joinPaths({"a", "y", "z"})),
+ std::pow(ProximityBase, 5));
+ EXPECT_FLOAT_EQ(
+ IndexProximity(Context, joinPaths({"a", "b", "c", "d", "e", "y"})),
+ std::pow(ProximityBase, 2));
+ EXPECT_FLOAT_EQ(
+ IndexProximity(Context, joinPaths({"a", "b", "m", "n", "o", "y"})),
+ std::pow(ProximityBase, 5));
+ EXPECT_FLOAT_EQ(
+ IndexProximity(Context, joinPaths({"a", "t", "m", "n", "o", "y"})),
+ std::pow(ProximityBase, 6));
+ // Note the common directory is /clang-test/
+ EXPECT_FLOAT_EQ(IndexProximity(Context, joinPaths({"m", "n", "o", "p", "y"})),
+ std::pow(ProximityBase, 7));
+ // Different URI schemes.
+ EXPECT_FLOAT_EQ(IndexProximity(Context,
+ joinPaths({"a", "b", "test-root", "a", "y"}),
+ "unittest"),
+ 0.0);
+}
+
+TEST(QualityTests, IndexSymbolProximityScoresWithTestURI) {
+ SymbolRelevanceContext Context(
+ /*ProximityPaths=*/{joinPaths({"a", "test-root", "b", "c", "x"})});
+ EXPECT_FLOAT_EQ(
+ IndexProximity(Context, joinPaths({"remote", "test-root", "b", "c", "x"}),
+ "unittest"),
+ 1);
+ EXPECT_FLOAT_EQ(IndexProximity(Context,
+ joinPaths({"remote", "test-root", "b", "y"}),
+ "unittest"),
+ std::pow(ProximityBase, 2));
+ // unittest:///b/c/x vs unittest:///m/n/y. No common directory.
+ EXPECT_FLOAT_EQ(
+ IndexProximity(Context, joinPaths({"remote", "test-root", "m", "n", "y"}),
+ "unittest"),
+ 0);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/FileIndexTests.cpp
===================================================================
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -96,6 +96,42 @@
M.update(File.Filename, &AST.getASTContext(), AST.getPreprocessorPtr());
}
+class TestURIScheme : public URIScheme {
+public:
+ llvm::Expected<std::string>
+ getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
+ llvm::StringRef /*HintPath*/) const override {
+ llvm_unreachable("ClangdTests never makes absolute path.");
+ }
+
+ llvm::Expected<URI>
+ uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
+ return URI::parse(HardcodedURI);
+ }
+ static const char HardcodedURI[];
+ static const char Scheme[];
+};
+
+const char TestURIScheme::HardcodedURI[] = "ClangdTests:///test";
+const char TestURIScheme::Scheme[] = "ClangdTest";
+
+static URISchemeRegistry::Add<TestURIScheme> X(TestURIScheme::Scheme,
+ "Test scheme for ClangdTests.");
+
+TEST(FileIndexTest, CustomizedURIScheme) {
+ FileIndex M({TestURIScheme::Scheme});
+ update(M, "f", "class string {};");
+
+ FuzzyFindRequest Req;
+ Req.Query = "";
+ bool SeenSymbol = false;
+ M.fuzzyFind(Req, [&](const Symbol &Sym) {
+ EXPECT_EQ(Sym.CanonicalDeclaration.FileURI, TestURIScheme::HardcodedURI);
+ SeenSymbol = true;
+ });
+ EXPECT_TRUE(SeenSymbol);
+}
+
TEST(FileIndexTest, IndexAST) {
FileIndex M;
update(M, "f1", "namespace ns { void f() {} class X {}; }");
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -152,28 +152,6 @@
}
};
-constexpr const char* ClangdTestScheme = "ClangdTests";
-class TestURIScheme : public URIScheme {
-public:
- llvm::Expected<std::string>
- getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
- llvm::StringRef /*HintPath*/) const override {
- llvm_unreachable("ClangdTests never makes absolute path.");
- }
-
- llvm::Expected<URI>
- uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
- llvm_unreachable("ClangdTest never creates a test URI.");
- }
-
- llvm::Expected<std::string> getIncludeSpelling(const URI &U) const override {
- return ("\"" + U.body().trim("/") + "\"").str();
- }
-};
-
-static URISchemeRegistry::Add<TestURIScheme>
- X(ClangdTestScheme, "Test scheme for ClangdTests.");
-
TEST_F(ClangdVFSTest, Parse) {
// FIXME: figure out a stable format for AST dumps, so that we can check the
// output of the dump itself is equal to the expected one, not just that it's
Index: clangd/index/FileIndex.h
===================================================================
--- clangd/index/FileIndex.h
+++ clangd/index/FileIndex.h
@@ -56,6 +56,10 @@
/// \brief This manages symbls from files and an in-memory index on all symbols.
class FileIndex : public SymbolIndex {
public:
+ /// If URISchemes is empty, the default schemes in SymbolCollector will be
+ /// used.
+ FileIndex(std::vector<std::string> URISchemes = {});
+
/// \brief Update symbols in \p Path with symbols in \p AST. If \p AST is
/// nullptr, this removes all symbols in the file.
/// If \p AST is not null, \p PP cannot be null and it should be the
@@ -72,11 +76,14 @@
private:
FileSymbols FSymbols;
MemIndex Index;
+ std::vector<std::string> URISchemes;
};
/// Retrieves namespace and class level symbols in \p AST.
/// Exposed to assist in unit tests.
-SymbolSlab indexAST(ASTContext &AST, std::shared_ptr<Preprocessor> PP);
+/// If URISchemes is empty, the default schemes in SymbolCollector will be used.
+SymbolSlab indexAST(ASTContext &AST, std::shared_ptr<Preprocessor> PP,
+ llvm::ArrayRef<std::string> URISchemes = {});
} // namespace clangd
} // namespace clang
Index: clangd/index/FileIndex.cpp
===================================================================
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -15,15 +15,18 @@
namespace clang {
namespace clangd {
-SymbolSlab indexAST(ASTContext &AST, std::shared_ptr<Preprocessor> PP) {
+SymbolSlab indexAST(ASTContext &AST, std::shared_ptr<Preprocessor> PP,
+ llvm::ArrayRef<std::string> URISchemes) {
SymbolCollector::Options CollectorOpts;
// FIXME(ioeric): we might also want to collect include headers. We would need
// to make sure all includes are canonicalized (with CanonicalIncludes), which
// is not trivial given the current way of collecting symbols: we only have
// AST at this point, but we also need preprocessor callbacks (e.g.
// CommentHandler for IWYU pragma) to canonicalize includes.
CollectorOpts.CollectIncludePath = false;
CollectorOpts.CountReferences = false;
+ if (!URISchemes.empty())
+ CollectorOpts.URISchemes = URISchemes;
SymbolCollector Collector(std::move(CollectorOpts));
Collector.setPreprocessor(PP);
@@ -41,6 +44,9 @@
return Collector.takeSymbols();
}
+FileIndex::FileIndex(std::vector<std::string> URISchemes)
+ : URISchemes(std::move(URISchemes)) {}
+
void FileSymbols::update(PathRef Path, std::unique_ptr<SymbolSlab> Slab) {
std::lock_guard<std::mutex> Lock(Mutex);
if (!Slab)
@@ -79,7 +85,7 @@
} else {
assert(PP);
auto Slab = llvm::make_unique<SymbolSlab>();
- *Slab = indexAST(*AST, PP);
+ *Slab = indexAST(*AST, PP, URISchemes);
FSymbols.update(Path, std::move(Slab));
}
auto Symbols = FSymbols.allSymbols();
Index: clangd/Quality.h
===================================================================
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -26,6 +26,7 @@
//===---------------------------------------------------------------------===//
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_QUALITY_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_QUALITY_H
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/StringRef.h"
#include <algorithm>
#include <functional>
@@ -67,13 +68,41 @@
llvm::raw_ostream &operator<<(llvm::raw_ostream &,
const SymbolQualitySignals &);
+class SymbolRelevanceContext {
+ public:
+ /// \p ProximityPaths are used to compute proximity scores from symbol's
+ /// declaring file. The best score will be used.
+ explicit SymbolRelevanceContext(
+ llvm::ArrayRef<llvm::StringRef> ProximityPaths);
+
+ /// Calculates the best proximity score from proximity paths to the symbol's
+ /// URI. When a path cannot be encoded into the same scheme as \p SymbolURI,
+ /// the proximity will be 0.
+ float uriProximity(llvm::StringRef SymbolURI) const;
+
+ private:
+ llvm::SmallVector<std::string, 2> ProximityPaths;
+ friend llvm::raw_ostream &operator<<(llvm::raw_ostream &,
+ const SymbolRelevanceContext &);
+};
+
/// Attributes of a symbol-query pair that affect how much we like it.
struct SymbolRelevanceSignals {
+ explicit SymbolRelevanceSignals(const SymbolRelevanceContext &Context)
+ : Context(Context) {}
+
+ const SymbolRelevanceContext &Context;
+
/// 0-1+ fuzzy-match score for unqualified name. Must be explicitly assigned.
float NameMatch = 1;
bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
+
+ /// Proximity between the best index symbol and the query. [0-1], 1 is
+ /// closest.
+ float IndexProximityScore = 0;
+ llvm::StringRef IndexSymbolURI;
/// Proximity between best declaration and the query. [0-1], 1 is closest.
- float ProximityScore = 0;
+ float SemaProximityScore = 0;
// An approximate measure of where we expect the symbol to be used.
enum AccessibleScope {
@@ -93,7 +122,7 @@
// Condense these signals down to a single number, higher is better.
float evaluate() const;
-};
+ };
llvm::raw_ostream &operator<<(llvm::raw_ostream &,
const SymbolRelevanceSignals &);
Index: clangd/Quality.cpp
===================================================================
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -7,6 +7,7 @@
//
//===---------------------------------------------------------------------===//
#include "Quality.h"
+#include "URI.h"
#include "index/Index.h"
#include "clang/AST/ASTContext.h"
#include "clang/Basic/CharInfo.h"
@@ -162,6 +163,64 @@
return OS;
}
+/// Calculates a proximity score from \p From and \p To, which are URI strings
+/// that have the same scheme. This does not parse URI. A URI (sans "<scheme>:")
+/// is split into chunks by '/' and each chunk is considered a file/directory.
+/// For example, "uri:///a/b/c" will be treated as /a/b/c
+static float uriProximity(StringRef From, StringRef To) {
+ auto SchemeSplitFrom = From.split(':');
+ auto SchemeSplitTo = To.split(':');
+ assert((SchemeSplitFrom.first == SchemeSplitTo.first) &&
+ "URIs must have the same scheme in order to compute proximity.");
+ auto Split = [](StringRef URIWithoutScheme) {
+ SmallVector<StringRef, 8> Split;
+ URIWithoutScheme.split(Split, '/', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+ return Split;
+ };
+ SmallVector<StringRef, 8> Fs = Split(SchemeSplitFrom.second);
+ SmallVector<StringRef, 8> Ts = Split(SchemeSplitTo.second);
+ auto F = Fs.begin(), T = Ts.begin(), FE = Fs.end(), TE = Ts.end();
+ for (; F != FE && T != TE && *F == *T; ++F, ++T) {
+ }
+ if (F == Fs.begin() && T == Ts.begin()) // No common directory.
+ return 0;
+ // We penalize for traversing up and down from \p From to \p To but penalize
+ // less for traversing down because subprojects are more closely related than
+ // superprojects.
+ int UpDist = FE - F;
+ int DownDist = TE - T;
+ return std::pow(0.7, UpDist + DownDist/2);
+}
+
+SymbolRelevanceContext::SymbolRelevanceContext(
+ ArrayRef<StringRef> ProximityPaths)
+ : ProximityPaths(ProximityPaths.begin(), ProximityPaths.end()) {}
+
+float SymbolRelevanceContext::uriProximity(StringRef SymbolURI) const {
+ float Score = 0;
+ if (!ProximityPaths.empty() && !SymbolURI.empty()) {
+ for (const auto &Path : ProximityPaths)
+ // Only calculate proximity score for two URIs with the same scheme so
+ // that the computation can be purely text-based and thus avoid expensive
+ // URI encoding/decoding.
+ if (auto U = URI::create(Path, SymbolURI.split(':').first)) {
+ Score = std::max(Score, clangd::uriProximity(U->toString(), SymbolURI));
+ } else {
+ llvm::consumeError(U.takeError());
+ }
+ }
+ return Score;
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+ const SymbolRelevanceContext &C) {
+ OS << formatv("=== Symbol relevance context ===\n");
+ OS << formatv(
+ "\tProximityPaths:\n\t\t{0}\n",
+ llvm::join(C.ProximityPaths.begin(), C.ProximityPaths.end(), "\n\t\t"));
+ return OS;
+}
+
static SymbolRelevanceSignals::AccessibleScope
ComputeScope(const NamedDecl &D) {
bool InClass = false;
@@ -182,19 +241,24 @@
void SymbolRelevanceSignals::merge(const Symbol &IndexResult) {
// FIXME: Index results always assumed to be at global scope. If Scope becomes
// relevant to non-completion requests, we should recognize class members etc.
+
+ IndexSymbolURI = IndexResult.CanonicalDeclaration.FileURI;
+ IndexProximityScore =
+ std::max(Context.uriProximity(IndexSymbolURI), IndexProximityScore);
}
void SymbolRelevanceSignals::merge(const CodeCompletionResult &SemaCCResult) {
if (SemaCCResult.Availability == CXAvailability_NotAvailable ||
SemaCCResult.Availability == CXAvailability_NotAccessible)
Forbidden = true;
if (SemaCCResult.Declaration) {
- // We boost things that have decls in the main file.
- // The real proximity scores would be more general when we have them.
+ // We boost things that have decls in the main file. We give a fixed score
+ // for all other declarations in sema as they are already included in the
+ // translation unit.
float DeclProximity =
- hasDeclInMainFile(*SemaCCResult.Declaration) ? 1.0 : 0.0;
- ProximityScore = std::max(DeclProximity, ProximityScore);
+ hasDeclInMainFile(*SemaCCResult.Declaration) ? 1.0 : 0.6;
+ SemaProximityScore = std::max(DeclProximity, SemaProximityScore);
}
// Declarations are scoped, others (like macros) are assumed global.
@@ -212,7 +276,7 @@
// Proximity scores are [0,1] and we translate them into a multiplier in the
// range from 1 to 2.
- Score *= 1 + ProximityScore;
+ Score *= 1 + std::max(IndexProximityScore, SemaProximityScore);
// Symbols like local variables may only be referenced within their scope.
// Conversely if we're in that scope, it's likely we'll reference them.
@@ -236,11 +300,14 @@
return Score;
}
+
raw_ostream &operator<<(raw_ostream &OS, const SymbolRelevanceSignals &S) {
OS << formatv("=== Symbol relevance: {0}\n", S.evaluate());
OS << formatv("\tName match: {0}\n", S.NameMatch);
OS << formatv("\tForbidden: {0}\n", S.Forbidden);
- OS << formatv("\tProximity: {0}\n", S.ProximityScore);
+ OS << formatv("\tIndexProximity: {0}\n", S.IndexProximityScore);
+ OS << formatv("\tIndexSymbolURI: {0}\n", S.IndexSymbolURI);
+ OS << formatv("\tSemaProximity: {0}\n", S.SemaProximityScore);
OS << formatv("\tQuery type: {0}\n", static_cast<int>(S.Query));
OS << formatv("\tScope: {0}\n", static_cast<int>(S.Scope));
return OS;
Index: clangd/FindSymbols.cpp
===================================================================
--- clangd/FindSymbols.cpp
+++ clangd/FindSymbols.cpp
@@ -151,7 +151,8 @@
SymbolQualitySignals Quality;
Quality.merge(Sym);
- SymbolRelevanceSignals Relevance;
+ SymbolRelevanceContext RelevanceContext(/*ProximityPaths=*/{});
+ SymbolRelevanceSignals Relevance(RelevanceContext);
Relevance.Query = SymbolRelevanceSignals::Generic;
if (auto NameMatch = Filter.match(Sym.Name))
Relevance.NameMatch = *NameMatch;
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -848,11 +848,17 @@
bool Incomplete = false; // Would more be available with a higher limit?
llvm::Optional<FuzzyMatcher> Filter; // Initialized once Sema runs.
std::unique_ptr<IncludeInserter> Includes; // Initialized once compiler runs.
+ SymbolRelevanceContext RelevanceContext;
public:
// A CodeCompleteFlow object is only useful for calling run() exactly once.
CodeCompleteFlow(PathRef FileName, const CodeCompleteOptions &Opts)
- : FileName(FileName), Opts(Opts) {}
+ : FileName(FileName), Opts(Opts),
+ // FIXME: also use path of the main header corresponding to FileName to
+ // calculate the file proximity, which would capture include/ and src/
+ // project setup where headers and implementations are not in the same
+ // directory.
+ RelevanceContext({FileName}) {}
CompletionList run(const SemaCompleteInput &SemaCCInput) && {
trace::Span Tracer("CodeCompleteFlow");
@@ -991,7 +997,7 @@
C.Name = IndexResult ? IndexResult->Name : Recorder->getName(*SemaResult);
SymbolQualitySignals Quality;
- SymbolRelevanceSignals Relevance;
+ SymbolRelevanceSignals Relevance(RelevanceContext);
Relevance.Query = SymbolRelevanceSignals::CodeComplete;
if (auto FuzzyScore = fuzzyScore(C))
Relevance.NameMatch = *FuzzyScore;
@@ -1020,7 +1026,7 @@
<< "CodeComplete: " << C.Name << (IndexResult ? " (index)" : "")
<< (SemaResult ? " (sema)" : "") << " = " << Scores.finalScore
<< "\n"
- << Quality << Relevance << "\n");
+ << Quality << RelevanceContext << Relevance << "\n");
NSema += bool(SemaResult);
NIndex += bool(IndexResult);
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -80,6 +80,10 @@
/// opened files and uses the index to augment code completion results.
bool BuildDynamicSymbolIndex = false;
+ /// URI schemes to use when building the dynamic index.
+ /// If empty, the default schemes in SymbolCollector will be used.
+ std::vector<std::string> URISchemes;
+
/// If set, use this index to augment code completion results.
SymbolIndex *StaticIndex = nullptr;
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -86,7 +86,8 @@
: CompileArgs(CDB, Opts.ResourceDir ? Opts.ResourceDir->str()
: getStandardResourceDir()),
DiagConsumer(DiagConsumer), FSProvider(FSProvider),
- FileIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
+ FileIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex(Opts.URISchemes)
+ : nullptr),
PCHs(std::make_shared<PCHContainerOperations>()),
// Pass a callback into `WorkScheduler` to extract symbols from a newly
// parsed file and rebuild the file index synchronously each time an AST
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
