[clang-tools-extra] r335723 - [clangd] Sema ranking tweaks: downrank keywords and injected names.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Jun 27 04:43:54 2018
New Revision: 335723

URL: http://llvm.org/viewvc/llvm-project?rev=335723&view=rev
Log:
[clangd] Sema ranking tweaks: downrank keywords and injected names.

Summary:
Injected names being ranked too high was just a bug.
The high boost for keywords was intended, but was too much given how useless
keywords are. We should probably boost them on a case-by-case basis eventually.

Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits

Differential Revision: https://reviews.llvm.org/D48368

Modified:
    clang-tools-extra/trunk/clangd/Quality.cpp
    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=335723&r1=335722&r2=335723&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Quality.cpp (original)
+++ clang-tools-extra/trunk/clangd/Quality.cpp Wed Jun 27 04:43:54 2018
@@ -156,8 +156,8 @@ float SymbolQualitySignals::evaluate() c
     Score *= 0.1f;
 
   switch (Category) {
-    case Keyword:  // Usually relevant, but misses most signals.
-      Score *= 10;
+    case Keyword:  // Often relevant, but misses most signals.
+      Score *= 4;  // FIXME: important keywords should have specific boosts.
       break;
     case Type:
     case Function:
@@ -241,10 +241,14 @@ llvm::raw_ostream &operator<<(llvm::raw_
 }
 
 static SymbolRelevanceSignals::AccessibleScope
-ComputeScope(const NamedDecl &D) {
+ComputeScope(const NamedDecl *D) {
+  // Injected "Foo" within the class "Foo" has file scope, not class scope.
+  const DeclContext *DC = D->getDeclContext();
+  if (auto *R = dyn_cast_or_null<RecordDecl>(D))
+    if (R->isInjectedClassName())
+      DC = DC->getParent();
   bool InClass = false;
-  for (const DeclContext *DC = D.getDeclContext(); !DC->isFileContext();
-       DC = DC->getParent()) {
+  for (; !DC->isFileContext(); DC = DC->getParent()) {
     if (DC->isFunctionOrMethod())
       return SymbolRelevanceSignals::FunctionScope;
     InClass = InClass || DC->isRecord();
@@ -252,7 +256,7 @@ ComputeScope(const NamedDecl &D) {
   if (InClass)
     return SymbolRelevanceSignals::ClassScope;
   // This threshold could be tweaked, e.g. to treat module-visible as global.
-  if (D.getLinkageInternal() < ExternalLinkage)
+  if (D->getLinkageInternal() < ExternalLinkage)
     return SymbolRelevanceSignals::FileScope;
   return SymbolRelevanceSignals::GlobalScope;
 }
@@ -280,7 +284,7 @@ void SymbolRelevanceSignals::merge(const
 
   // Declarations are scoped, others (like macros) are assumed global.
   if (SemaCCResult.Declaration)
-    Scope = std::min(Scope, ComputeScope(*SemaCCResult.Declaration));
+    Scope = std::min(Scope, ComputeScope(SemaCCResult.Declaration));
 }
 
 float SymbolRelevanceSignals::evaluate() const {

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=335723&r1=335722&r2=335723&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Wed Jun 27 04:43:54 2018
@@ -84,6 +84,7 @@ TEST(QualityTests, SymbolRelevanceSignal
     int deprecated() { return 0; }
 
     namespace { struct X { void y() { int z; } }; }
+    struct S{}
   )cpp";
   auto AST = Test.build();
 
@@ -115,6 +116,10 @@ TEST(QualityTests, SymbolRelevanceSignal
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "z"), 42));
   EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FunctionScope);
+  // The injected class name is treated as the outer class name.
+  Relevance = {};
+  Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
+  EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
 }
 
 // Do the signals move the scores in the direction we expect?