kadircet updated this revision to Diff 234514.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Address comments
- Extend handling to:
  - var and function templates
  - non-auto case


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71596/new/

https://reviews.llvm.org/D71596

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "AST.h"
 #include "Annotations.h"
 #include "Hover.h"
 #include "TestIndex.h"
@@ -130,12 +131,9 @@
           )cpp",
        [](HoverInfo &HI) {
          HI.NamespaceScope = "";
-         HI.Name = "vector";
+         HI.Name = "vector<int>";
          HI.Kind = index::SymbolKind::Class;
-         HI.Definition = "template <typename T> class vector {}";
-         HI.TemplateParameters = {
-             {std::string("typename"), std::string("T"), llvm::None},
-         };
+         HI.Definition = "template <> class vector<int> {}";
        }},
       // Class template
       {R"cpp(
@@ -181,21 +179,10 @@
          HI.NamespaceScope = "";
          HI.Name = "foo";
          HI.Kind = index::SymbolKind::Function;
-         HI.Definition =
-             R"cpp(template <template <typename, bool...> class C, typename = char, int = 0,
-          bool Q = false, class... Ts>
-void foo())cpp";
+         HI.Definition = "template <> void foo<Foo, char, 0, false, <>>()";
          HI.ReturnType = "void";
          HI.Type = "void ()";
          HI.Parameters.emplace();
-         HI.TemplateParameters = {
-             {std::string("template <typename, bool...> class"),
-              std::string("C"), llvm::None},
-             {std::string("typename"), llvm::None, std::string("char")},
-             {std::string("int"), llvm::None, std::string("0")},
-             {std::string("bool"), std::string("Q"), std::string("false")},
-             {std::string("class..."), std::string("Ts"), llvm::None},
-         };
        }},
       // Function decl
       {R"cpp(
@@ -1116,14 +1103,13 @@
             HI.Name = "foo";
             HI.Kind = index::SymbolKind::Function;
             HI.NamespaceScope = "";
-            HI.Type = "T ()";
-            HI.Definition = "template <typename T> T foo()";
+            HI.Type = "int ()";
+            HI.Definition = "template <> int foo<int>()";
             HI.Documentation = "Templated function";
-            HI.ReturnType = "T";
+            HI.ReturnType = "int";
             HI.Parameters = std::vector<HoverInfo::Param>{};
-            HI.TemplateParameters = {
-                {std::string("typename"), std::string("T"), llvm::None},
-            };
+            // FIXME: We should populate template parameters with arguments in
+            // case of instantiations.
           }},
       {
           R"cpp(// Anonymous union
@@ -1271,6 +1257,7 @@
           [](HoverInfo &HI) {
             HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "auto function return with trailing type";
           }},
       {
           R"cpp(// trailing return type
@@ -1282,6 +1269,7 @@
           [](HoverInfo &HI) {
             HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "trailing return type";
           }},
       {
           R"cpp(// auto in function return
@@ -1293,6 +1281,7 @@
           [](HoverInfo &HI) {
             HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "auto in function return";
           }},
       {
           R"cpp(// auto& in function return
@@ -1305,6 +1294,7 @@
           [](HoverInfo &HI) {
             HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "auto& in function return";
           }},
       {
           R"cpp(// auto* in function return
@@ -1317,6 +1307,7 @@
           [](HoverInfo &HI) {
             HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "auto* in function return";
           }},
       {
           R"cpp(// const auto& in function return
@@ -1329,6 +1320,7 @@
           [](HoverInfo &HI) {
             HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "const auto& in function return";
           }},
       {
           R"cpp(// decltype(auto) in function return
@@ -1340,6 +1332,7 @@
           [](HoverInfo &HI) {
             HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "decltype(auto) in function return";
           }},
       {
           R"cpp(// decltype(auto) reference in function return
@@ -1404,6 +1397,8 @@
           [](HoverInfo &HI) {
             HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation =
+                "decltype of function with trailing return type.";
           }},
       {
           R"cpp(// decltype of var with decltype.
@@ -1449,6 +1444,7 @@
           [](HoverInfo &HI) {
             HI.Name = "cls";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "auto on alias";
           }},
       {
           R"cpp(// auto on alias
@@ -1459,6 +1455,7 @@
           [](HoverInfo &HI) {
             HI.Name = "templ<int>";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "auto on alias";
           }},
   };
 
@@ -1503,6 +1500,93 @@
   }
 }
 
+TEST(Hover, DocsFromIndex) {
+  Annotations T(R"cpp(
+  template <typename T> class X {};
+  void foo() {
+    au^to t = X<int>();
+    X^<int> w;
+    (void)w;
+  })cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  for (const auto &D : AST.getDiagnostics())
+    ADD_FAILURE() << D;
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  Symbol IndexSym;
+  IndexSym.ID = *getSymbolID(&findDecl(AST, "X"));
+  IndexSym.Documentation = "comment from index";
+  SymbolSlab::Builder Symbols;
+  Symbols.insert(IndexSym);
+  auto Index =
+      MemIndex::build(std::move(Symbols).build(), RefSlab(), RelationSlab());
+
+  for (const auto &P : T.points()) {
+    auto H = getHover(AST, P, format::getLLVMStyle(), Index.get());
+    ASSERT_TRUE(H);
+    EXPECT_EQ(H->Documentation, IndexSym.Documentation);
+  }
+}
+
+TEST(Hover, DocsFromAST) {
+  Annotations T(R"cpp(
+  // doc
+  template <typename T> class X {};
+  // doc
+  template <typename T> void bar() {}
+  // doc
+  template <typename T> T baz;
+  void foo() {
+    au^to t = X<int>();
+    X^<int>();
+    b^ar<int>();
+    au^to T = ba^z<X<int>>;
+    ba^z<int> = 0;
+  })cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  for (const auto &D : AST.getDiagnostics())
+    ADD_FAILURE() << D;
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  for (const auto &P : T.points()) {
+    auto H = getHover(AST, P, format::getLLVMStyle(), nullptr);
+    ASSERT_TRUE(H);
+    EXPECT_EQ(H->Documentation, "doc");
+  }
+}
+
+TEST(Hover, DocsFromMostSpecial) {
+  Annotations T(R"cpp(
+  // doc1
+  template <typename T> class $doc1^X {};
+  // doc2
+  template <> class $doc2^X<int> {};
+  // doc3
+  template <typename T> class $doc3^X<T*> {};
+  void foo() {
+    X$doc1^<char>();
+    X$doc2^<int>();
+    X$doc3^<int*>();
+  })cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  for (const auto &D : AST.getDiagnostics())
+    ADD_FAILURE() << D;
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  for (auto Comment : {"doc1", "doc2", "doc3"}) {
+    for (const auto &P : T.points(Comment)) {
+      auto H = getHover(AST, P, format::getLLVMStyle(), nullptr);
+      ASSERT_TRUE(H);
+      EXPECT_EQ(H->Documentation, Comment);
+    }
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -183,15 +184,28 @@
   return D->getAsFunction();
 }
 
+// Returns the decl that should be used for querying comments, either from index
+// or ast.
+const NamedDecl *getDeclForComment(const NamedDecl *D) {
+  if (auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(D))
+    if (!CTSD->isExplicitInstantiationOrSpecialization())
+      return CTSD->getSpecializedTemplate();
+  if (auto *VTSD = llvm::dyn_cast<VarTemplateSpecializationDecl>(D))
+    if (!VTSD->isExplicitInstantiationOrSpecialization())
+      return VTSD->getSpecializedTemplate();
+  if (auto *FD = D->getAsFunction())
+    if (FD->isTemplateInstantiation())
+      return FD->getTemplateSpecializationInfo()->getTemplate();
+  return D;
+}
+
 // Look up information about D from the index, and add it to Hover.
-void enhanceFromIndex(HoverInfo &Hover, const Decl *D,
+void enhanceFromIndex(HoverInfo &Hover, const NamedDecl &ND,
                       const SymbolIndex *Index) {
-  if (!Index || !llvm::isa<NamedDecl>(D))
-    return;
-  const NamedDecl &ND = *cast<NamedDecl>(D);
   // We only add documentation, so don't bother if we already have some.
-  if (!Hover.Documentation.empty())
+  if (!Hover.Documentation.empty() || !Index)
     return;
+
   // Skip querying for non-indexable symbols, there's no point.
   // We're searching for symbols that might be indexed outside this main file.
   if (!SymbolCollector::shouldCollectSymbol(ND, ND.getASTContext(),
@@ -307,8 +321,10 @@
 
   PrintingPolicy Policy = printingPolicyForDecls(Ctx.getPrintingPolicy());
   if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D)) {
-    HI.Documentation = getDeclComment(Ctx, *ND);
     HI.Name = printName(Ctx, *ND);
+    ND = getDeclForComment(ND);
+    HI.Documentation = getDeclComment(Ctx, *ND);
+    enhanceFromIndex(HI, *ND, Index);
   }
 
   HI.Kind = index::getSymbolInfo(D).Kind;
@@ -346,7 +362,6 @@
   }
 
   HI.Definition = printDefinition(D);
-  enhanceFromIndex(HI, D, Index);
   return HI;
 }
 
@@ -356,12 +371,13 @@
   HoverInfo HI;
 
   if (const auto *D = T->getAsTagDecl()) {
-    HI.Name = printName(ASTCtx, *D);
     HI.Kind = index::getSymbolInfo(D).Kind;
-    enhanceFromIndex(HI, D, Index);
-  }
+    HI.Name = printName(ASTCtx, *D);
 
-  if (HI.Name.empty()) {
+    const auto *ND = getDeclForComment(D);
+    HI.Documentation = getDeclComment(ASTCtx, *ND);
+    enhanceFromIndex(HI, *ND, Index);
+  } else {
     // Builtin types
     llvm::raw_string_ostream OS(HI.Name);
     PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
@@ -398,6 +414,22 @@
   return HI;
 }
 
+const Decl *preferInstantiation(llvm::iterator_range<const Decl **> Range) {
+  if (Range.empty())
+    return nullptr;
+
+  auto *D = *Range.begin();
+  for (const Decl *C : Range) {
+    if (llvm::isa<VarTemplateSpecializationDecl>(C) ||
+        llvm::isa<ClassTemplateSpecializationDecl>(C) ||
+        (C->getAsFunction() && C->getAsFunction()->isTemplateInstantiation())) {
+      D = C;
+      break;
+    }
+  }
+  return D;
+}
+
 } // namespace
 
 llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
@@ -421,10 +453,10 @@
     SelectionTree Selection(AST.getASTContext(), AST.getTokens(), *Offset);
     std::vector<const Decl *> Result;
     if (const SelectionTree::Node *N = Selection.commonAncestor()) {
-      DeclRelationSet Rel = DeclRelation::TemplatePattern | DeclRelation::Alias;
-      auto Decls = targetDecl(N->ASTNode, Rel);
-      if (!Decls.empty()) {
-        HI = getHoverContents(Decls.front(), Index);
+      DeclRelationSet Rel = DeclRelation::TemplateInstantiation |
+                            DeclRelation::TemplatePattern | DeclRelation::Alias;
+      if (const Decl *D = preferInstantiation(targetDecl(N->ASTNode, Rel))) {
+        HI = getHoverContents(D, Index);
         // Look for a close enclosing expression to show the value of.
         if (!HI->Value)
           HI->Value = printExprValue(N, AST.getASTContext());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to