usaxena95 created this revision.
usaxena95 added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
Current implementation of heuristic-based scoring function also contains
computation of derived signals (e.g. whether name contains a word from
context, computing file distances, scope distances.)
This is an attempt to separate out the logic for computation of derived
signals from the scoring function.
This will allow us to have a clean API for scoring functions that will
take only concrete code completion signals as input.
This only refactors Name and ContextWords which are essentially used to
compute whether the name contains a context word or not. If this looks
good then this will be done for other properties that are more of a
utility (used to compute other signals) than a signal themselves.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79500
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/FindSymbols.cpp
clang-tools-extra/clangd/Quality.cpp
clang-tools-extra/clangd/Quality.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/QualityTests.cpp
Index: clang-tools-extra/clangd/unittests/QualityTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/QualityTests.cpp
+++ clang-tools-extra/clangd/unittests/QualityTests.cpp
@@ -294,13 +294,17 @@
EXPECT_LT(InBaseClass.evaluate(), Default.evaluate());
llvm::StringSet<> Words = {"one", "two", "three"};
+
SymbolRelevanceSignals WithoutMatchingWord;
- WithoutMatchingWord.ContextWords = &Words;
- WithoutMatchingWord.Name = "four";
+ WithoutMatchingWord.Util.ContextWords = &Words;
+ WithoutMatchingWord.Util.Name = "four";
+ WithoutMatchingWord.calcNameMatchesContext();
EXPECT_EQ(WithoutMatchingWord.evaluate(), Default.evaluate());
+
SymbolRelevanceSignals WithMatchingWord;
- WithMatchingWord.ContextWords = &Words;
- WithMatchingWord.Name = "TheTwoTowers";
+ WithMatchingWord.Util.ContextWords = &Words;
+ WithMatchingWord.Util.Name = "TheTwoTowers";
+ WithMatchingWord.calcNameMatchesContext();
EXPECT_GT(WithMatchingWord.evaluate(), Default.evaluate());
}
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -427,7 +427,6 @@
SymbolQualitySignals Quality;
Quality.merge(Sym);
SymbolRelevanceSignals Relevance;
- Relevance.Name = Sym.Name;
Relevance.Query = SymbolRelevanceSignals::Generic;
Relevance.merge(Sym);
auto Score =
Index: clang-tools-extra/clangd/Quality.h
===================================================================
--- clang-tools-extra/clangd/Quality.h
+++ clang-tools-extra/clangd/Quality.h
@@ -85,17 +85,18 @@
/// Attributes of a symbol-query pair that affect how much we like it.
struct SymbolRelevanceSignals {
- /// The name of the symbol (for ContextWords). Must be explicitly assigned.
- llvm::StringRef Name;
+ /// Whether name contains some word in context. Must not be set explicitly.
+ /// Computed by calcNameMatchesContext().
+ bool NameMatchesContext = false;
/// 0-1+ fuzzy-match score for unqualified name. Must be explicitly assigned.
float NameMatch = 1;
- /// Lowercase words relevant to the context (e.g. near the completion point).
- llvm::StringSet<>* ContextWords = nullptr;
+
bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
/// Whether fixits needs to be applied for that completion or not.
bool NeedsFixIts = false;
bool InBaseClass = false; // A member from base class of the accessed class.
+ // FIXME: Move this property to utilites.
URIDistance *FileProximityMatch = nullptr;
/// These are used to calculate proximity between the index symbol and the
/// query.
@@ -105,6 +106,7 @@
/// Proximity between best declaration and the query. [0-1], 1 is closest.
float SemaFileProximityScore = 0;
+ // FIXME: Move this property to utilites.
// Scope proximity is only considered (both index and sema) when this is set.
ScopeDistance *ScopeProximityMatch = nullptr;
llvm::Optional<llvm::StringRef> SymbolScope;
@@ -136,6 +138,20 @@
// Whether the item matches the type expected in the completion context.
bool TypeMatchesPreferred = false;
+ // Properties and utilites used to compute derived signals e.g.
+ // IsNameInContext. These are not directly used by the scoring function.
+ struct Utility {
+ /// The name of the symbol (for ContextWords). Must be explicitly assigned.
+ llvm::StringRef Name;
+
+ /// Lowercase words relevant to the context (e.g. near the completion
+ /// point).
+ llvm::StringSet<> *ContextWords = nullptr;
+ } Util;
+
+ // Sets NameMatchesContext to true iff Name contains any of the ContextWords.
+ void calcNameMatchesContext();
+
void merge(const CodeCompletionResult &SemaResult);
void merge(const Symbol &IndexResult);
Index: clang-tools-extra/clangd/Quality.cpp
===================================================================
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -348,6 +348,10 @@
return llvm::None;
}
+void SymbolRelevanceSignals::calcNameMatchesContext() {
+ NameMatchesContext = wordMatching(Util.Name, Util.ContextWords).hasValue();
+}
+
float SymbolRelevanceSignals::evaluate() const {
float Score = 1;
@@ -369,7 +373,7 @@
Score *=
SemaSaysInScope ? 2.0 : scopeBoost(*ScopeProximityMatch, SymbolScope);
- if (wordMatching(Name, ContextWords))
+ if (NameMatchesContext)
Score *= 1.5;
// Symbols like local variables may only be referenced within their scope.
@@ -428,12 +432,12 @@
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
const SymbolRelevanceSignals &S) {
OS << llvm::formatv("=== Symbol relevance: {0}\n", S.evaluate());
- OS << llvm::formatv("\tName: {0}\n", S.Name);
+ OS << llvm::formatv("\tName: {0}\n", S.Util.Name);
OS << llvm::formatv("\tName match: {0}\n", S.NameMatch);
- if (S.ContextWords)
+ if (S.Util.ContextWords)
OS << llvm::formatv(
"\tMatching context word: {0}\n",
- wordMatching(S.Name, S.ContextWords).getValueOr("<none>"));
+ wordMatching(S.Util.Name, S.Util.ContextWords).getValueOr("<none>"));
OS << llvm::formatv("\tForbidden: {0}\n", S.Forbidden);
OS << llvm::formatv("\tNeedsFixIts: {0}\n", S.NeedsFixIts);
OS << llvm::formatv("\tIsInstanceMember: {0}\n", S.IsInstanceMember);
Index: clang-tools-extra/clangd/FindSymbols.cpp
===================================================================
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -109,7 +109,6 @@
SymbolQualitySignals Quality;
Quality.merge(Sym);
SymbolRelevanceSignals Relevance;
- Relevance.Name = Sym.Name;
Relevance.Query = SymbolRelevanceSignals::Generic;
if (auto NameMatch = Filter.match(Sym.Name))
Relevance.NameMatch = *NameMatch;
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1612,14 +1612,16 @@
SymbolQualitySignals Quality;
SymbolRelevanceSignals Relevance;
Relevance.Context = CCContextKind;
- Relevance.Name = Bundle.front().Name;
Relevance.Query = SymbolRelevanceSignals::CodeComplete;
Relevance.FileProximityMatch = FileProximity.getPointer();
if (ScopeProximity)
Relevance.ScopeProximityMatch = ScopeProximity.getPointer();
if (PreferredType)
Relevance.HadContextType = true;
- Relevance.ContextWords = &ContextWords;
+
+ Relevance.Util.Name = Bundle.front().Name;
+ Relevance.Util.ContextWords = &ContextWords;
+ Relevance.calcNameMatchesContext();
auto &First = Bundle.front();
if (auto FuzzyScore = fuzzyScore(First))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
