[clangd] Improve documentation for auto and implicit specs
authorKadir Cetinkaya <kadircet@google.com>
Tue, 17 Dec 2019 11:13:28 +0000 (12:13 +0100)
committerKadir Cetinkaya <kadircet@google.com>
Thu, 19 Dec 2019 10:55:22 +0000 (11:55 +0100)
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

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

index 81d4e93..55bb8a0 100644 (file)
@@ -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;
index bd5d769..39c00b0 100644 (file)
@@ -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
 
index 623aa96..94a532b 100644 (file)
@@ -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.
index 8435544..0c71d76 100644 (file)
@@ -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