Author: ioeric
Date: Tue Jul 24 01:51:52 2018
New Revision: 337816
URL: http://llvm.org/viewvc/llvm-project?rev=337816&view=rev
Log:
[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
Modified:
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
Modified: clang-tools-extra/trunk/clangd/Quality.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=337816&r1=337815&r2=337816&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Quality.cpp (original)
+++ clang-tools-extra/trunk/clangd/Quality.cpp Tue Jul 24 01:51:52 2018
@@ -67,6 +67,7 @@ static SymbolQualitySignals::SymbolCateg
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() c
Score *= 0.2f;
break;
case Unknown:
+ case Constructor: // No boost constructors so they are after class types.
break;
}
Modified: clang-tools-extra/trunk/clangd/Quality.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.h?rev=337816&r1=337815&r2=337816&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Quality.h (original)
+++ clang-tools-extra/trunk/clangd/Quality.h Tue Jul 24 01:51:52 2018
@@ -58,6 +58,7 @@ struct SymbolQualitySignals {
Macro,
Type,
Function,
+ Constructor,
Namespace,
Keyword,
} Category = Unknown;
Modified: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp?rev=337816&r1=337815&r2=337816&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Tue Jul 24
01:51:52 2018
@@ -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, SymbolQualitySignalsS
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
