[clang-tools-extra] ac3f9e4 - [clangd] Improve documentation for auto and implicit specs

Kadir Cetinkaya via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 19 02:59:31 PST 2019


Author: Kadir Cetinkaya
Date: 2019-12-19T11:55:22+01:00
New Revision: ac3f9e48421712168884d59cbfe8b294dd76a19b

URL: https://github.com/llvm/llvm-project/commit/ac3f9e48421712168884d59cbfe8b294dd76a19b
DIFF: https://github.com/llvm/llvm-project/commit/ac3f9e48421712168884d59cbfe8b294dd76a19b.diff

LOG: [clangd] Improve documentation for auto and implicit specs

Summary:
Clangd didn't fill documentation for `auto` when it wasn't available in
index. Also it wasn't showing any documentations for implicit instantiations.

This patch ensures auto and normal decl case behaves in the same way and also
makes use of the explicit template specialization while fetching comments for
implicit instantiations.

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/FindTarget.cpp
    clang-tools-extra/clangd/FindTarget.h
    clang-tools-extra/clangd/Hover.cpp
    clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp
index 81d4e93ed3c2..55bb8a0b70ea 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -438,17 +438,8 @@ targetDecl(const ast_type_traits::DynTypedNode &N, DeclRelationSet Mask) {
   return Result;
 }
 
-namespace {
-/// Find declarations explicitly referenced in the source code defined by \p N.
-/// For templates, will prefer to return a template instantiation whenever
-/// possible. However, can also return a template pattern if the specialization
-/// cannot be picked, e.g. in dependent code or when there is no corresponding
-/// Decl for a template instantitation, e.g. for templated using decls:
-///    template <class T> using Ptr = T*;
-///    Ptr<int> x;
-///    ^~~ there is no Decl for 'Ptr<int>', so we return the template pattern.
 llvm::SmallVector<const NamedDecl *, 1>
-explicitReferenceTargets(DynTypedNode N, DeclRelationSet Mask = {}) {
+explicitReferenceTargets(DynTypedNode N, DeclRelationSet Mask) {
   assert(!(Mask & (DeclRelation::TemplatePattern |
                    DeclRelation::TemplateInstantiation)) &&
          "explicitRefenceTargets handles templates on its own");
@@ -478,6 +469,7 @@ explicitReferenceTargets(DynTypedNode N, DeclRelationSet Mask = {}) {
   return Targets;
 }
 
+namespace {
 llvm::SmallVector<ReferenceLoc, 2> refInDecl(const Decl *D) {
   struct Visitor : ConstDeclVisitor<Visitor> {
     llvm::SmallVector<ReferenceLoc, 2> Refs;

diff  --git a/clang-tools-extra/clangd/FindTarget.h b/clang-tools-extra/clangd/FindTarget.h
index bd5d7690ceed..39c00b0f6df0 100644
--- a/clang-tools-extra/clangd/FindTarget.h
+++ b/clang-tools-extra/clangd/FindTarget.h
@@ -182,6 +182,18 @@ inline DeclRelationSet operator&(DeclRelation L, DeclRelation R) {
 inline DeclRelationSet operator~(DeclRelation R) { return ~DeclRelationSet(R); }
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, DeclRelationSet);
 
+/// Find declarations explicitly referenced in the source code defined by \p N.
+/// For templates, will prefer to return a template instantiation whenever
+/// possible. However, can also return a template pattern if the specialization
+/// cannot be picked, e.g. in dependent code or when there is no corresponding
+/// Decl for a template instantitation, e.g. for templated using decls:
+///    template <class T> using Ptr = T*;
+///    Ptr<int> x;
+///    ^~~ there is no Decl for 'Ptr<int>', so we return the template pattern.
+/// \p Mask should not contain TemplatePattern or TemplateInstantiation.
+llvm::SmallVector<const NamedDecl *, 1>
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+                         DeclRelationSet Mask = {});
 } // namespace clangd
 } // namespace clang
 

diff  --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 623aa962b85a..94a532b1ac47 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/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,29 @@ const FunctionDecl *getUnderlyingFunction(const Decl *D) {
   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->getTemplateInstantiationPattern();
+  if (auto *VTSD = llvm::dyn_cast<VarTemplateSpecializationDecl>(D))
+    if (!VTSD->isExplicitInstantiationOrSpecialization())
+      return VTSD->getTemplateInstantiationPattern();
+  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);
+  assert(&ND == getDeclForComment(&ND));
   // 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 +322,10 @@ HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
 
   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 +363,6 @@ HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
   }
 
   HI.Definition = printDefinition(D);
-  enhanceFromIndex(HI, D, Index);
   return HI;
 }
 
@@ -358,10 +374,11 @@ HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx,
   if (const auto *D = T->getAsTagDecl()) {
     HI.Name = printName(ASTCtx, *D);
     HI.Kind = index::getSymbolInfo(D).Kind;
-    enhanceFromIndex(HI, D, Index);
-  }
 
-  if (HI.Name.empty()) {
+    const auto *CommentD = getDeclForComment(D);
+    HI.Documentation = getDeclComment(ASTCtx, *CommentD);
+    enhanceFromIndex(HI, *CommentD, Index);
+  } else {
     // Builtin types
     llvm::raw_string_ostream OS(HI.Name);
     PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
@@ -397,7 +414,6 @@ HoverInfo getHoverContents(const DefinedMacro &Macro, ParsedAST &AST) {
   }
   return HI;
 }
-
 } // namespace
 
 llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
@@ -421,8 +437,7 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
     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);
+      auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias);
       if (!Decls.empty()) {
         HI = getHoverContents(Decls.front(), Index);
         // Look for a close enclosing expression to show the value of.

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 8435544f37c5..0c71d76e3c03 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/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 @@ TEST(Hover, Structured) {
           )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 @@ class Foo {})cpp";
          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(
@@ -464,8 +451,6 @@ void foo())cpp";
          HI.Type = "enum Color";
          HI.Value = "GREEN (1)"; // Symbolic when hovering on an expression.
        }},
-      // FIXME: We should use the Decl referenced, even if from an implicit
-      // instantiation. Then the scope would be Add<1, 2>.
       {R"cpp(
         template<int a, int b> struct Add {
           static constexpr int result = a + b;
@@ -474,11 +459,11 @@ void foo())cpp";
         )cpp",
        [](HoverInfo &HI) {
          HI.Name = "result";
-         HI.Definition = "static constexpr int result = a + b";
+         HI.Definition = "static constexpr int result = 1 + 2";
          HI.Kind = index::SymbolKind::StaticProperty;
          HI.Type = "const int";
          HI.NamespaceScope = "";
-         HI.LocalScope = "Add<a, b>::";
+         HI.LocalScope = "Add<1, 2>::";
          HI.Value = "3";
        }},
       {R"cpp(
@@ -1116,14 +1101,13 @@ TEST(Hover, All) {
             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 +1255,7 @@ TEST(Hover, All) {
           [](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 +1267,7 @@ TEST(Hover, All) {
           [](HoverInfo &HI) {
             HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "trailing return type";
           }},
       {
           R"cpp(// auto in function return
@@ -1293,6 +1279,7 @@ TEST(Hover, All) {
           [](HoverInfo &HI) {
             HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "auto in function return";
           }},
       {
           R"cpp(// auto& in function return
@@ -1305,6 +1292,7 @@ TEST(Hover, All) {
           [](HoverInfo &HI) {
             HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "auto& in function return";
           }},
       {
           R"cpp(// auto* in function return
@@ -1317,6 +1305,7 @@ TEST(Hover, All) {
           [](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 +1318,7 @@ TEST(Hover, All) {
           [](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 +1330,7 @@ TEST(Hover, All) {
           [](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 +1395,8 @@ TEST(Hover, All) {
           [](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 +1442,7 @@ TEST(Hover, All) {
           [](HoverInfo &HI) {
             HI.Name = "cls";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "auto on alias";
           }},
       {
           R"cpp(// auto on alias
@@ -1459,6 +1453,7 @@ TEST(Hover, All) {
           [](HoverInfo &HI) {
             HI.Name = "templ<int>";
             HI.Kind = index::SymbolKind::Struct;
+            HI.Documentation = "auto on alias";
           }},
   };
 
@@ -1503,6 +1498,93 @@ TEST(Hover, All) {
   }
 }
 
+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


        


More information about the cfe-commits mailing list