ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.

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.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49667

Files:
  clangd/Quality.cpp
  clangd/Quality.h
  unittests/clangd/QualityTests.cpp

Index: unittests/clangd/QualityTests.cpp
===================================================================
--- unittests/clangd/QualityTests.cpp
+++ 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,17 @@
   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(Macro.evaluate(), Default.evaluate());
+  EXPECT_LT(Constructor.evaluate(), Function.evaluate());
 }
 
 TEST(QualityTests, SymbolRelevanceSignalsSanity) {
@@ -261,19 +266,13 @@
   auto Symbols = Header.headerSymbols();
   auto AST = Header.build();
 
-  const NamedDecl *Foo = &findDecl(AST, "Foo");
-  SymbolRelevanceSignals Cls;
-  Cls.merge(CodeCompletionResult(Foo, /*Priority=*/0));
-
   const NamedDecl *CtorDecl = &findAnyDecl(AST, [](const NamedDecl &ND) {
     return (ND.getQualifiedNameAsString() == "Foo::Foo") &&
            llvm::isa<CXXConstructorDecl>(&ND);
   });
-  SymbolRelevanceSignals Ctor;
-  Ctor.merge(CodeCompletionResult(CtorDecl, /*Priority=*/0));
-
-  EXPECT_EQ(Cls.Scope, SymbolRelevanceSignals::GlobalScope);
-  EXPECT_EQ(Ctor.Scope, SymbolRelevanceSignals::GlobalScope);
+  SymbolQualitySignals Q;
+  Q.merge(CodeCompletionResult(CtorDecl, /*Priority=*/0));
+  EXPECT_EQ(Q.Category, SymbolQualitySignals::Constructor);
 }
 
 TEST(QualityTests, IsInstanceMember) {
@@ -317,6 +316,31 @@
   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
Index: clangd/Quality.h
===================================================================
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -58,6 +58,7 @@
     Macro,
     Type,
     Function,
+    Constructor,
     Namespace,
     Keyword,
   } Category = Unknown;
Index: clangd/Quality.cpp
===================================================================
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -67,6 +67,7 @@
     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 @@
     return SymbolQualitySignals::Type;
   case CXCursor_MemberRef:
     return SymbolQualitySignals::Variable;
+  case CXCursor_Constructor:
+    return SymbolQualitySignals::Constructor;
   default:
     return SymbolQualitySignals::Keyword;
   }
@@ -124,10 +127,11 @@
   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:
@@ -203,6 +207,9 @@
   case Variable:
     Score *= 1.1f;
     break;
+  case Constructor:
+    Score *= 1.0f; // Rank class constructors after class types.
+    break;
   case Namespace:
     Score *= 0.8f;
     break;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to