summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEric Liu <ioeric@google.com>2018-07-24 08:51:52 +0000
committerEric Liu <ioeric@google.com>2018-07-24 08:51:52 +0000
commit0b104764032a60e9f86aafad4928a432e57476fd (patch)
treeb0fc73086bfcceb70331b3ded1d04c7b06585a97
parent449483e3f50c0d1e6ed79338f6c361480da1d18f (diff)
[clangd] Tune down quality score for class constructors so that it's ranked after class types.
Summary: Currently, class constructors have the same score as the class types, and they are often ranked before class types. This is often not desireable and can be annoying when snippet is enabled and constructor signatures are added. Metrics: ``` ================================================================================================== OVERALL ================================================================================================== Total measurements: 111117 (+0) All measurements: MRR: 64.06 (+0.20) Top-5: 75.73% (+0.14%) Top-100: 93.71% (+0.01%) Full identifiers: MRR: 98.25 (+0.55) Top-5: 99.04% (+0.03%) Top-100: 99.16% (+0.00%) Filter length 0-5: MRR: 15.23 (+0.02) 50.50 (-0.02) 65.04 (+0.11) 70.75 (+0.19) 74.37 (+0.25) 79.43 (+0.32) Top-5: 40.90% (+0.03%) 74.52% (+0.03%) 87.23% (+0.15%) 91.68% (+0.08%) 93.68% (+0.14%) 95.87% (+0.12%) Top-100: 68.21% (+0.02%) 96.28% (+0.07%) 98.43% (+0.00%) 98.72% (+0.00%) 98.74% (+0.01%) 98.81% (+0.00%) ================================================================================================== DEFAULT ================================================================================================== Total measurements: 57535 (+0) All measurements: MRR: 58.07 (+0.37) Top-5: 69.94% (+0.26%) Top-100: 90.14% (+0.03%) Full identifiers: MRR: 97.13 (+1.05) Top-5: 98.14% (+0.06%) Top-100: 98.34% (+0.00%) Filter length 0-5: MRR: 13.91 (+0.00) 38.53 (+0.01) 55.58 (+0.21) 63.63 (+0.30) 69.23 (+0.47) 72.87 (+0.60) Top-5: 24.99% (+0.00%) 62.70% (+0.06%) 82.80% (+0.30%) 88.66% (+0.16%) 92.02% (+0.27%) 93.53% (+0.21%) Top-100: 51.56% (+0.05%) 93.19% (+0.13%) 97.30% (+0.00%) 97.81% (+0.00%) 97.85% (+0.01%) 97.79% (+0.00%) ``` Remark: - The full-id completions have +1.05 MRR improvement. - There is no noticeable impact on EXPLICIT_MEMBER_ACCESS and WANT_LOCAL. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits Differential Revision: https://reviews.llvm.org/D49667 git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@337816 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r--clangd/Quality.cpp7
-rw-r--r--clangd/Quality.h1
-rw-r--r--unittests/clangd/QualityTests.cpp31
3 files changed, 37 insertions, 2 deletions
diff --git a/clangd/Quality.cpp b/clangd/Quality.cpp
index 1dcd2052..1b64f9a9 100644
--- a/clangd/Quality.cpp
+++ b/clangd/Quality.cpp
@@ -67,6 +67,7 @@ static SymbolQualitySignals::SymbolCategory categorize(const NamedDecl &ND) {
MAP(TypeDecl, Type);
MAP(TypeAliasTemplateDecl, Type);
MAP(ClassTemplateDecl, Type);
+ MAP(CXXConstructorDecl, Constructor);
MAP(ValueDecl, Variable);
MAP(VarTemplateDecl, Variable);
MAP(FunctionDecl, Function);
@@ -96,6 +97,8 @@ categorize(const CodeCompletionResult &R) {
return SymbolQualitySignals::Type;
case CXCursor_MemberRef:
return SymbolQualitySignals::Variable;
+ case CXCursor_Constructor:
+ return SymbolQualitySignals::Constructor;
default:
return SymbolQualitySignals::Keyword;
}
@@ -124,10 +127,11 @@ categorize(const index::SymbolInfo &D) {
case index::SymbolKind::InstanceProperty:
case index::SymbolKind::ClassProperty:
case index::SymbolKind::StaticProperty:
- case index::SymbolKind::Constructor:
case index::SymbolKind::Destructor:
case index::SymbolKind::ConversionFunction:
return SymbolQualitySignals::Function;
+ case index::SymbolKind::Constructor:
+ return SymbolQualitySignals::Constructor;
case index::SymbolKind::Variable:
case index::SymbolKind::Field:
case index::SymbolKind::EnumConstant:
@@ -210,6 +214,7 @@ float SymbolQualitySignals::evaluate() const {
Score *= 0.2f;
break;
case Unknown:
+ case Constructor: // No boost constructors so they are after class types.
break;
}
diff --git a/clangd/Quality.h b/clangd/Quality.h
index 10a0a3ea..0aed496b 100644
--- a/clangd/Quality.h
+++ b/clangd/Quality.h
@@ -58,6 +58,7 @@ struct SymbolQualitySignals {
Macro,
Type,
Function,
+ Constructor,
Namespace,
Keyword,
} Category = Unknown;
diff --git a/unittests/clangd/QualityTests.cpp b/unittests/clangd/QualityTests.cpp
index a237e2fe..153eef49 100644
--- a/unittests/clangd/QualityTests.cpp
+++ b/unittests/clangd/QualityTests.cpp
@@ -23,6 +23,7 @@
#include "TestTU.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Type.h"
#include "clang/Sema/CodeCompleteConsumer.h"
#include "llvm/Support/Casting.h"
#include "gmock/gmock.h"
@@ -185,13 +186,16 @@ TEST(QualityTests, SymbolQualitySignalsSanity) {
EXPECT_GT(WithReferences.evaluate(), Default.evaluate());
EXPECT_GT(ManyReferences.evaluate(), WithReferences.evaluate());
- SymbolQualitySignals Keyword, Variable, Macro;
+ SymbolQualitySignals Keyword, Variable, Macro, Constructor, Function;
Keyword.Category = SymbolQualitySignals::Keyword;
Variable.Category = SymbolQualitySignals::Variable;
Macro.Category = SymbolQualitySignals::Macro;
+ Constructor.Category = SymbolQualitySignals::Constructor;
+ Function.Category = SymbolQualitySignals::Function;
EXPECT_GT(Variable.evaluate(), Default.evaluate());
EXPECT_GT(Keyword.evaluate(), Variable.evaluate());
EXPECT_LT(Macro.evaluate(), Default.evaluate());
+ EXPECT_LT(Constructor.evaluate(), Function.evaluate());
}
TEST(QualityTests, SymbolRelevanceSignalsSanity) {
@@ -317,6 +321,31 @@ TEST(QualityTests, IsInstanceMember) {
EXPECT_TRUE(Rel.IsInstanceMember);
}
+TEST(QualityTests, ConstructorQuality) {
+ auto Header = TestTU::withHeaderCode(R"cpp(
+ class Foo {
+ public:
+ Foo(int);
+ };
+ )cpp");
+ auto Symbols = Header.headerSymbols();
+ auto AST = Header.build();
+
+ const NamedDecl *CtorDecl = &findAnyDecl(AST, [](const NamedDecl &ND) {
+ return (ND.getQualifiedNameAsString() == "Foo::Foo") &&
+ llvm::isa<CXXConstructorDecl>(&ND);
+ });
+
+ SymbolQualitySignals Q;
+ Q.merge(CodeCompletionResult(CtorDecl, /*Priority=*/0));
+ EXPECT_EQ(Q.Category, SymbolQualitySignals::Constructor);
+
+ Q.Category = SymbolQualitySignals::Unknown;
+ const Symbol &CtorSym = findSymbol(Symbols, "Foo::Foo");
+ Q.merge(CtorSym);
+ EXPECT_EQ(Q.Category, SymbolQualitySignals::Constructor);
+}
+
} // namespace
} // namespace clangd
} // namespace clang