omtcyfz updated this revision to Diff 154383.
omtcyfz marked 10 inline comments as done.
omtcyfz added a comment.

Address the comments, add a couple of FIXMEs for the future.


https://reviews.llvm.org/D49012

Files:
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/unittests/clangd/QualityTests.cpp
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/lib/Sema/SemaCodeComplete.cpp

Index: clang/lib/Sema/SemaCodeComplete.cpp
===================================================================
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -859,12 +859,12 @@
   }
 
   // Look through using declarations.
-  if (const UsingShadowDecl *Using =
-          dyn_cast<UsingShadowDecl>(R.Declaration)) {
-    MaybeAddResult(Result(Using->getTargetDecl(),
-                          getBasePriority(Using->getTargetDecl()),
-                          R.Qualifier),
-                   CurContext);
+  if (const UsingShadowDecl *Using = dyn_cast<UsingShadowDecl>(R.Declaration)) {
+    CodeCompletionResult Result(Using->getTargetDecl(),
+                                getBasePriority(Using->getTargetDecl()),
+                                R.Qualifier);
+    Result.ShadowDecl = Using;
+    MaybeAddResult(Result, CurContext);
     return;
   }
 
@@ -977,10 +977,11 @@
 
   // Look through using declarations.
   if (const UsingShadowDecl *Using = dyn_cast<UsingShadowDecl>(R.Declaration)) {
-    AddResult(Result(Using->getTargetDecl(),
-                     getBasePriority(Using->getTargetDecl()),
-                     R.Qualifier),
-              CurContext, Hiding);
+    CodeCompletionResult Result(Using->getTargetDecl(),
+                                getBasePriority(Using->getTargetDecl()),
+                                R.Qualifier);
+    Result.ShadowDecl = Using;
+    AddResult(Result, CurContext, Hiding);
     return;
   }
 
@@ -1004,10 +1005,10 @@
   if (AsNestedNameSpecifier) {
     R.StartsNestedNameSpecifier = true;
     R.Priority = CCP_NestedNameSpecifier;
-  }
-  else if (Filter == &ResultBuilder::IsMember && !R.Qualifier && InBaseClass &&
-           isa<CXXRecordDecl>(R.Declaration->getDeclContext()
-                                                  ->getRedeclContext()))
+  } else if (Filter == &ResultBuilder::IsMember && !R.Qualifier &&
+             InBaseClass &&
+             isa<CXXRecordDecl>(
+                 R.Declaration->getDeclContext()->getRedeclContext()))
     R.QualifierIsInformative = true;
 
   // If this result is supposed to have an informative qualifier, add one.
Index: clang/include/clang/Sema/CodeCompleteConsumer.h
===================================================================
--- clang/include/clang/Sema/CodeCompleteConsumer.h
+++ clang/include/clang/Sema/CodeCompleteConsumer.h
@@ -45,8 +45,9 @@
 class NamedDecl;
 class NestedNameSpecifier;
 class Preprocessor;
-class Sema;
 class RawComment;
+class Sema;
+class UsingShadowDecl;
 
 /// Default priority values for code-completion results based
 /// on their kind.
@@ -836,6 +837,12 @@
   /// informative rather than required.
   NestedNameSpecifier *Qualifier = nullptr;
 
+  /// If this Decl was unshadowed by using declaration, this can store a
+  /// pointer to the UsingShadowDecl which was used in the unshadowing process.
+  /// This information can be used to uprank CodeCompletionResults / which have
+  /// corresponding `using decl::qualified::name;` nearby.
+  const UsingShadowDecl *ShadowDecl = nullptr;
+
   /// Build a result that refers to a declaration.
   CodeCompletionResult(const NamedDecl *Declaration, unsigned Priority,
                        NestedNameSpecifier *Qualifier = nullptr,
@@ -847,7 +854,7 @@
         QualifierIsInformative(QualifierIsInformative),
         StartsNestedNameSpecifier(false), AllParametersAreInformative(false),
         DeclaringEntity(false), Qualifier(Qualifier) {
-    //FIXME: Add assert to check FixIts range requirements.
+    // FIXME: Add assert to check FixIts range requirements.
     computeCursorKindAndAvailability(Accessible);
   }
 
Index: clang-tools-extra/unittests/clangd/QualityTests.cpp
===================================================================
--- clang-tools-extra/unittests/clangd/QualityTests.cpp
+++ clang-tools-extra/unittests/clangd/QualityTests.cpp
@@ -79,8 +79,12 @@
   Test.HeaderCode = R"cpp(
     int header();
     int header_main();
+
+    namespace hdr { class Bar {}; } // namespace hdr
     )cpp";
   Test.Code = R"cpp(
+    using hdr::Bar;
+
     int ::header_main() {}
     int main();
 
@@ -109,7 +113,26 @@
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "header_main"), 42));
   EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 1.0f)
-      << "Current file and header";
+      << "Current file and definition in header";
+
+  Relevance = {};
+  const auto SymbolName = "Bar";
+  const auto *Shadow =
+      *dyn_cast<UsingDecl>(
+           &findAnyDecl(AST,
+                        [&](const NamedDecl &ND) -> bool {
+                          if (const UsingDecl *Using = dyn_cast<UsingDecl>(&ND))
+                            if (Using->shadow_size() &&
+                                Using->getQualifiedNameAsString() == SymbolName)
+                              return true;
+                          return false;
+                        }))
+           ->shadow_begin();
+  CodeCompletionResult Result(Shadow->getTargetDecl(), 42);
+  Result.ShadowDecl = Shadow;
+  Relevance.merge(Result);
+  EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 1.0f)
+      << "Using declaration in main file";
 
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "X"), 42));
Index: clang-tools-extra/clangd/Quality.cpp
===================================================================
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -34,13 +34,27 @@
 static bool hasDeclInMainFile(const Decl &D) {
   auto &SourceMgr = D.getASTContext().getSourceManager();
   for (auto *Redecl : D.redecls()) {
+    // FIXME: Consider ExpansionLoc here and probably add a test for that.
     auto Loc = SourceMgr.getSpellingLoc(Redecl->getLocation());
     if (SourceMgr.isWrittenInMainFile(Loc))
       return true;
   }
   return false;
 }
 
+static bool hasUsingDeclInMainFile(const CodeCompletionResult &R) {
+  const auto &Context = R.Declaration->getASTContext();
+  const auto &SourceMgr = Context.getSourceManager();
+  if (R.ShadowDecl) {
+    // FIXME(kbobyrev): Add a test for ExpansionLoc as suggested by ioeric
+    // here: https://reviews.llvm.org/D49012#inline-429901
+    const auto Loc = SourceMgr.getExpansionLoc(R.ShadowDecl->getLocation());
+    if (SourceMgr.isWrittenInMainFile(Loc))
+      return true;
+  }
+  return false;
+}
+
 static SymbolQualitySignals::SymbolCategory categorize(const NamedDecl &ND) {
   class Switch
       : public ConstDeclVisitor<Switch, SymbolQualitySignals::SymbolCategory> {
@@ -231,8 +245,10 @@
     // We boost things that have decls in the main file. We give a fixed score
     // for all other declarations in sema as they are already included in the
     // translation unit.
-    float DeclProximity =
-        hasDeclInMainFile(*SemaCCResult.Declaration) ? 1.0 : 0.6;
+    float DeclProximity = (hasDeclInMainFile(*SemaCCResult.Declaration) ||
+                           hasUsingDeclInMainFile(SemaCCResult))
+                              ? 1.0
+                              : 0.6;
     SemaProximityScore = std::max(DeclProximity, SemaProximityScore);
   }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to