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

The following are metrics for explicit member access completions. There is no
noticeable impact on other completion types.

Before:
EXPLICIT_MEMBER_ACCESS

  Total measurements: 24382
  All measurements: MRR: 62.27  Top10: 80.21%   Top-100: 94.48%
  Full identifiers: MRR: 98.81  Top10: 99.89%   Top-100: 99.95%
  0-5 filter len:

MRR:  13.25     46.31   62.47   67.77   70.40   81.91
        Top-10:  29%    74%     84%     91%     91%     97%
        Top-100:  67%   99%     99%     99%     99%     100%

After:
EXPLICIT_MEMBER_ACCESS

  Total measurements: 24382
  All measurements: MRR: 63.18  Top10: 80.58%   Top-100: 95.07%
  Full identifiers: MRR: 98.79  Top10: 99.89%   Top-100: 99.95%
  0-5 filter len:

MRR:  13.84     48.39   63.55   68.83   71.28   82.64
        Top-10:  30%    75%     84%     91%     91%     97%
        Top-100:  70%   99%     99%     99%     99%     100%

- Top-N: wanted result is found in the first N completion results.
- MRR: Mean reciprocal rank.

Remark: the change seems to have minor positive impact. Although the improvement
is relatively small, down-ranking non-instance members in instance member access
should reduce noise in the completion results.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49543

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  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/Sema/CodeCompleteConsumer.h"
 #include "llvm/Support/Casting.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -227,6 +228,14 @@
   EXPECT_EQ(Scoped.evaluate(), Default.evaluate());
   Scoped.Query = SymbolRelevanceSignals::CodeComplete;
   EXPECT_GT(Scoped.evaluate(), Default.evaluate());
+
+  SymbolRelevanceSignals Instance;
+  Instance.IsInstanceMember = false;
+  EXPECT_EQ(Instance.evaluate(), Default.evaluate());
+  Instance.SemaContext = CodeCompletionContext::CCC_DotMemberAccess;
+  EXPECT_LT(Instance.evaluate(), Default.evaluate());
+  Instance.IsInstanceMember = true;
+  EXPECT_EQ(Instance.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, SortText) {
@@ -267,6 +276,35 @@
   EXPECT_EQ(Ctor.Scope, SymbolRelevanceSignals::GlobalScope);
 }
 
+TEST(QualityTests, IsInstanceMember) {
+  auto Header = TestTU::withHeaderCode(R"cpp(
+    class Foo {
+    public:
+      static void foo() {}
+      void bar() {}
+    };
+  )cpp");
+  auto Symbols = Header.headerSymbols();
+
+  SymbolRelevanceSignals Rel;
+  const Symbol &FooSym = findSymbol(Symbols, "Foo::foo");
+  Rel.merge(FooSym);
+  EXPECT_FALSE(Rel.IsInstanceMember);
+  const Symbol &BarSym = findSymbol(Symbols, "Foo::bar");
+  Rel.merge(BarSym);
+  EXPECT_TRUE(Rel.IsInstanceMember);
+
+  auto AST = Header.build();
+  const NamedDecl *Foo = &findDecl(AST, "Foo::foo");
+  const NamedDecl *Bar = &findDecl(AST, "Foo::bar");
+
+  Rel.IsInstanceMember = false;
+  Rel.merge(CodeCompletionResult(Foo, /*Priority=*/0));
+  EXPECT_FALSE(Rel.IsInstanceMember);
+  Rel.merge(CodeCompletionResult(Bar, /*Priority=*/0));
+  EXPECT_TRUE(Rel.IsInstanceMember);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
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 "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include <algorithm>
@@ -98,6 +99,11 @@
     Generic,
   } Query = Generic;
 
+  CodeCompletionContext::Kind SemaContext = CodeCompletionContext::CCC_Other;
+
+  // Whether symbol is an instance member of a class.
+  bool IsInstanceMember = false;
+
   void merge(const CodeCompletionResult &SemaResult);
   void merge(const Symbol &IndexResult);
 
Index: clangd/Quality.cpp
===================================================================
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -11,6 +11,7 @@
 #include "URI.h"
 #include "index/Index.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclVisitor.h"
 #include "clang/Basic/CharInfo.h"
@@ -139,6 +140,25 @@
   llvm_unreachable("Unknown index::SymbolKind");
 }
 
+static bool isInstanceMember(const NamedDecl *ND) {
+  if (!ND)
+    return false;
+  if (const auto *CM = dyn_cast<CXXMethodDecl>(ND))
+    return !CM->isStatic();
+  return isa<FieldDecl>(ND); // Note that static fields are VarDecl.
+}
+
+static bool isInstanceMember(const index::SymbolInfo &D) {
+  switch (D.Kind) {
+  case index::SymbolKind::InstanceMethod:
+  case index::SymbolKind::InstanceProperty:
+  case index::SymbolKind::Field:
+    return true;
+  default:
+    return false;
+  }
+}
+
 void SymbolQualitySignals::merge(const CodeCompletionResult &SemaCCResult) {
   if (SemaCCResult.Availability == CXAvailability_Deprecated)
     Deprecated = true;
@@ -231,6 +251,7 @@
   // relevant to non-completion requests, we should recognize class members etc.
 
   SymbolURI = IndexResult.CanonicalDeclaration.FileURI;
+  IsInstanceMember |= isInstanceMember(IndexResult.SymInfo);
 }
 
 void SymbolRelevanceSignals::merge(const CodeCompletionResult &SemaCCResult) {
@@ -247,6 +268,7 @@
                               ? 1.0
                               : 0.6;
     SemaProximityScore = std::max(DeclProximity, SemaProximityScore);
+    IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
   }
 
   // Declarations are scoped, others (like macros) are assumed global.
@@ -296,13 +318,22 @@
     }
   }
 
+  // Penalize non-instance members when they are accessed via a class instance.
+  if (!IsInstanceMember &&
+      (SemaContext == CodeCompletionContext::CCC_DotMemberAccess ||
+       SemaContext == CodeCompletionContext::CCC_ArrowMemberAccess)) {
+    Score *= 0.5;
+  }
+
   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("\tIsInstanceMember: {0}\n", S.IsInstanceMember);
+  OS << formatv("\tContext: {0}\n", getCompletionKindString(S.SemaContext));
   OS << formatv("\tSymbol URI: {0}\n", S.SymbolURI);
   if (S.FileProximityMatch) {
     auto Score = proximityScore(S.SymbolURI, S.FileProximityMatch);
Index: clangd/CodeComplete.h
===================================================================
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -21,6 +21,7 @@
 #include "Protocol.h"
 #include "index/Index.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Sema/CodeCompleteOptions.h"
 #include "clang/Tooling/CompilationDatabase.h"
 
@@ -144,6 +145,7 @@
 struct CodeCompleteResult {
   std::vector<CodeCompletion> Completions;
   bool HasMore = false;
+  CodeCompletionContext::Kind SemaContext = CodeCompletionContext::CCC_Other;
 };
 raw_ostream &operator<<(raw_ostream &, const CodeCompleteResult &);
 
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -1062,6 +1062,7 @@
       Output.Completions.back().Score = C.second;
     }
     Output.HasMore = Incomplete;
+    Output.SemaContext = Recorder->CCContext.getKind();
     return Output;
   }
 
@@ -1156,6 +1157,7 @@
                     CompletionCandidate::Bundle Bundle) {
     SymbolQualitySignals Quality;
     SymbolRelevanceSignals Relevance;
+    Relevance.SemaContext = Recorder->CCContext.getKind();
     Relevance.Query = SymbolRelevanceSignals::CodeComplete;
     Relevance.FileProximityMatch = FileProximity.getPointer();
     auto &First = Bundle.front();
@@ -1290,6 +1292,7 @@
 
 raw_ostream &operator<<(raw_ostream &OS, const CodeCompleteResult &R) {
   OS << "CodeCompleteResult: " << R.Completions.size() << (R.HasMore ? "+" : "")
+     << " (" << getCompletionKindString(R.SemaContext) << ")"
      << " items:\n";
   for (const auto &C : R.Completions)
     OS << C << "\n";
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to