[clang][Index] Introduce a TemplateParm SymbolKind

[kadircet]

Summary

Currently template parameters has symbolkind Unknown. This patch
introduces a new kind TemplateParm for templatetemplate, templatetype and
nontypetemplate parameters.

Also adds tests in clangd hover feature.

[merge_guards_bot]

Unit tests: fail. 62337 tests passed, 1 failed and 838 were skipped.

failed: libc++.std/containers/sequences/array/array_creation/to_array.fail.cpp

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[sammccall]

Libclang changes need tests I think.

clang-tools-extra/clangd/Protocol.cpp

265

this seems kind of dubious, maybe worth a comment?
(If we had the decl here we'd distinguish between type and non-type, right?)

clang-tools-extra/clangd/unittests/HoverTests.cpp

586

I do think we're going to want to have HI.Type = "typename" here, maybe with some special-case in the rendering.

(Especially with concepts where the TTP can be constrained as Integral or something)

Fine to leave for now but FIXME somewhere?

clang/include/clang-c/Index.h

6257 ↗

(On Diff #241419)

As far as hover goes, calling this "template parameter" and making int/class the "type" is cute and fits well.

But I worry we're not exposing enough info here and at the libclang level (and I guess Index) we should be distinguishing the 3 cases. WDYT?

[kadircet]

• As discussed offline, do not expose new symbol kinds to libclang
• Set types for templateparms in HoverInfo.

[kadircet]

clang-tools-extra/clangd/Protocol.cpp

265

we have all 3 of them now

[kadircet]

ping :)

[hokein]

clang-tools-extra/clangd/CodeComplete.cpp

98

I think these two should belong to CompletionItemKind::TypeParameter?

clang/lib/Index/IndexSymbol.cpp

360

nit: could we move these newly-added lines above the case Decl::ClassTemplatePartialSpecialization:. I think we usually put the llvm_unreachable cases at the end of switch statement..

[kadircet]

clang-tools-extra/clangd/CodeComplete.cpp

98

right, moved the ones below but forgot to move these :D thanks!

clang/lib/Index/IndexSymbol.cpp

360

yes but this one is specially, there's also a default label below.

Also the unreachable ones are for making sure certain valid types don't arrive in here rather than checking for invalid types are passed.
We usually put the unreachable to the end if it is protecting against invalid usages, I believe.

[kadircet]

• Address comments

clang-tools-extra/clangd/CodeComplete.cpp

CompletionItemKind toCompletionItemKind(index::SymbolKind Kind) {
   // FIXME(ioeric): use LSP struct instead of class when it is suppoted in the
   // protocol.
   case SK::Struct:
   case SK::Class:
   case SK::Protocol:
   case SK::Extension:
   case SK::Union:
     return CompletionItemKind::Class;
   case SK::TypeAlias:
     // We use the same kind as the VSCode C++ extension.
     // FIXME: pick a better option when we have one.
     return CompletionItemKind::Interface;
   case SK::Using:
     return CompletionItemKind::Reference;
   case SK::Function:
   // FIXME(ioeric): this should probably be an operator. This should be fixed
   // when `Operator` is support type in the protocol.
   case SK::ConversionFunction:
     return CompletionItemKind::Function;
   case SK::Variable:
   case SK::Parameter:
+  case SK::NonTypeTemplateParm:
     return CompletionItemKind::Variable;
   case SK::Field:
     return CompletionItemKind::Field;
   // FIXME(ioeric): use LSP enum constant when it is supported in the protocol.
   case SK::EnumConstant:
     return CompletionItemKind::Value;
   case SK::InstanceMethod:
   case SK::ClassMethod:
   case SK::StaticMethod:
   case SK::Destructor:
     return CompletionItemKind::Method;
   case SK::InstanceProperty:
   case SK::ClassProperty:
   case SK::StaticProperty:
     return CompletionItemKind::Property;
   case SK::Constructor:
     return CompletionItemKind::Constructor;
+  case SK::TemplateTypeParm:
+  case SK::TemplateTemplateParm:
+    return CompletionItemKind::TypeParameter;
   }
   llvm_unreachable("Unhandled clang::index::SymbolKind.");
 }
 
 CompletionItemKind
 toCompletionItemKind(CodeCompletionResult::ResultKind ResKind,
                      const NamedDecl *Decl,
                      CodeCompletionContext::Kind CtxKind) {

clang-tools-extra/clangd/Hover.cpp

std::string printDefinition(const Decl *D) {
   Policy.IncludeTagDefinition = false;
   Policy.SuppressTemplateArgsInCXXConstructors = true;
   Policy.SuppressTagKeyword = true;
   D->print(OS, Policy);
   OS.flush();
   return Definition;
 }
 
-void printParams(llvm::raw_ostream &OS,
-                 const std::vector<HoverInfo::Param> &Params) {
-  for (size_t I = 0, E = Params.size(); I != E; ++I) {
-    if (I)
-      OS << ", ";
-    OS << Params.at(I);
-  }
-}
-
 std::string printType(QualType QT, const PrintingPolicy &Policy) {
   // TypePrinter doesn't resolve decltypes, so resolve them here.
   // FIXME: This doesn't handle composite types that contain a decltype in them.
   // We should rather have a printing policy for that.
   while (const auto *DT = QT->getAs<DecltypeType>())
     QT = DT->getUnderlyingType();
   return QT.getAsString(Policy);
 }
 
+std::string printType(const TemplateTypeParmDecl *TTP) {
+  std::string Res = TTP->wasDeclaredWithTypename() ? "typename" : "class";
+  if (TTP->isParameterPack())
+    Res += "...";
+  return Res;
+}
+
+std::string printType(const NonTypeTemplateParmDecl *NTTP,
+                      const PrintingPolicy &PP) {
+  std::string Res = printType(NTTP->getType(), PP);
+  if (NTTP->isParameterPack())
+    Res += "...";
+  return Res;
+}
+
+std::string printType(const TemplateTemplateParmDecl *TTP,
+                      const PrintingPolicy &PP) {
+  std::string Res;
+  llvm::raw_string_ostream OS(Res);
+  OS << "template <";
+  llvm::StringRef Sep = "";
+  for (const Decl *Param : *TTP->getTemplateParameters()) {
+    OS << Sep;
+    Sep = ", ";
+    if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(Param))
+      OS << printType(TTP);
+    else if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(Param))
+      OS << printType(NTTP, PP);
+    else if (const auto *TTPD = dyn_cast<TemplateTemplateParmDecl>(Param))
+      OS << printType(TTPD, PP);
+  }
+  // FIXME: TemplateTemplateParameter doesn't store the info on whether this
+  // param was a "typename" or "class".
+  OS << "> class";
+  return OS.str();
+}
+
 std::vector<HoverInfo::Param>
 fetchTemplateParameters(const TemplateParameterList *Params,
                         const PrintingPolicy &PP) {
   assert(Params);
   std::vector<HoverInfo::Param> TempParameters;
 
   for (const Decl *Param : *Params) {
     HoverInfo::Param P;
     if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(Param)) {
-      P.Type = TTP->wasDeclaredWithTypename() ? "typename" : "class";
-      if (TTP->isParameterPack())
-        *P.Type += "...";
+      P.Type = printType(TTP);
 
       if (!TTP->getName().empty())
         P.Name = TTP->getNameAsString();
+
       if (TTP->hasDefaultArgument())
         P.Default = TTP->getDefaultArgument().getAsString(PP);
     } else if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(Param)) {
+      P.Type = printType(NTTP, PP);
+
       if (IdentifierInfo *II = NTTP->getIdentifier())
         P.Name = II->getName().str();
 
-      P.Type = printType(NTTP->getType(), PP);
-      if (NTTP->isParameterPack())
-        *P.Type += "...";
-
       if (NTTP->hasDefaultArgument()) {
         P.Default.emplace();
         llvm::raw_string_ostream Out(*P.Default);
         NTTP->getDefaultArgument()->printPretty(Out, nullptr, PP);
       }
     } else if (const auto *TTPD = dyn_cast<TemplateTemplateParmDecl>(Param)) {
-      P.Type.emplace();
-      llvm::raw_string_ostream OS(*P.Type);
-      OS << "template <";
-      printParams(OS,
-                  fetchTemplateParameters(TTPD->getTemplateParameters(), PP));
-      OS << "> class"; // FIXME: TemplateTemplateParameter doesn't store the
-                       // info on whether this param was a "typename" or
-                       // "class".
+      P.Type = printType(TTPD, PP);
+
       if (!TTPD->getName().empty())
         P.Name = TTPD->getNameAsString();
+
       if (TTPD->hasDefaultArgument()) {
         P.Default.emplace();
         llvm::raw_string_ostream Out(*P.Default);
         TTPD->getDefaultArgument().getArgument().print(PP, Out);
       }
     }
     TempParameters.push_back(std::move(P));
   }
if (const TemplateDecl *TD = D->getDescribedTemplate()) {
     }
   }
 
   // Fill in types and params.
   if (const FunctionDecl *FD = getUnderlyingFunction(D))
     fillFunctionTypeAndParams(HI, D, FD, Policy);
   else if (const auto *VD = dyn_cast<ValueDecl>(D))
     HI.Type = printType(VD->getType(), Policy);
+  else if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(D))
+    HI.Type = TTP->wasDeclaredWithTypename() ? "typename" : "class";
+  else if (const auto *TTP = dyn_cast<TemplateTemplateParmDecl>(D))
+    HI.Type = printType(TTP, Policy);
 
   // Fill in value with evaluated initializer if possible.
   if (const auto *Var = dyn_cast<VarDecl>(D)) {
     if (const Expr *Init = Var->getInit())
       HI.Value = printExprValue(Init, Ctx);
   } else if (const auto *ECD = dyn_cast<EnumConstantDecl>(D)) {
     // Dependent enums (e.g. nested in template classes) don't have values yet.
     if (!ECD->getType()->isDependentType())

clang-tools-extra/clangd/Protocol.cpp

 //===--- Protocol.cpp - Language Server Protocol Implementation -----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains the serialization code for the LSP structs.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Protocol.h"
 #include "Logger.h"
 #include "URI.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
SymbolKind indexSymbolKindToSymbolKind(index::SymbolKind Kind) {
   case index::SymbolKind::StaticProperty:
     return SymbolKind::Property;
   case index::SymbolKind::Constructor:
   case index::SymbolKind::Destructor:
     return SymbolKind::Constructor;
   case index::SymbolKind::ConversionFunction:
     return SymbolKind::Function;
   case index::SymbolKind::Parameter:
+  case index::SymbolKind::NonTypeTemplateParm:
     return SymbolKind::Variable;
   case index::SymbolKind::Using:
     return SymbolKind::Namespace;
+  case index::SymbolKind::TemplateTemplateParm:
+  case index::SymbolKind::TemplateTypeParm:
+    return SymbolKind::TypeParameter;
   }
   llvm_unreachable("invalid symbol kind");
 }
 
 bool fromJSON(const llvm::json::Value &Params, ClientCapabilities &R) {
   const llvm::json::Object *O = Params.getAsObject();
   if (!O)
     return false;

clang-tools-extra/clangd/Quality.cpp

case index::SymbolKind::Macro:
     return SymbolQualitySignals::Macro;
   case index::SymbolKind::Enum:
   case index::SymbolKind::Struct:
   case index::SymbolKind::Class:
   case index::SymbolKind::Protocol:
   case index::SymbolKind::Extension:
   case index::SymbolKind::Union:
   case index::SymbolKind::TypeAlias:
+  case index::SymbolKind::TemplateTypeParm:
+  case index::SymbolKind::TemplateTemplateParm:
     return SymbolQualitySignals::Type;
   case index::SymbolKind::Function:
   case index::SymbolKind::ClassMethod:
   case index::SymbolKind::InstanceMethod:
   case index::SymbolKind::StaticMethod:
   case index::SymbolKind::InstanceProperty:
   case index::SymbolKind::ClassProperty:
   case index::SymbolKind::StaticProperty:
   case index::SymbolKind::ConversionFunction:
     return SymbolQualitySignals::Function;
   case index::SymbolKind::Destructor:
     return SymbolQualitySignals::Destructor;
   case index::SymbolKind::Constructor:
     return SymbolQualitySignals::Constructor;
   case index::SymbolKind::Variable:
   case index::SymbolKind::Field:
   case index::SymbolKind::EnumConstant:
   case index::SymbolKind::Parameter:
+  case index::SymbolKind::NonTypeTemplateParm:
     return SymbolQualitySignals::Variable;
   case index::SymbolKind::Using:
   case index::SymbolKind::Module:
   case index::SymbolKind::Unknown:
     return SymbolQualitySignals::Unknown;
   }
   llvm_unreachable("Unknown index::SymbolKind");
 }

clang-tools-extra/clangd/unittests/HoverTests.cpp

class Foo {})cpp";
          HI.Name = "Foo<int *>";
          HI.Kind = index::SymbolKind::Class;
          HI.NamespaceScope = "";
          HI.Definition = "template <> class Foo<int *>";
          // FIXME: Maybe force instantiation to make use of real template
          // pattern.
          HI.Documentation = "comment from primary";
        }},
+      {// Template Type Parameter
+       R"cpp(
+          template <typename [[^T]] = int> void foo();
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "T";
+         HI.Kind = index::SymbolKind::TemplateTypeParm;
+         HI.NamespaceScope = "";
+         HI.Definition = "typename T = int";
+         HI.LocalScope = "foo::";
+         HI.Type = "typename";
+       }},
+      {// TemplateTemplate Type Parameter
+       R"cpp(
+          template <template<typename> class [[^T]]> void foo();
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "T";
+         HI.Kind = index::SymbolKind::TemplateTemplateParm;
+         HI.NamespaceScope = "";
+         HI.Definition = "template <typename> class T";
+         HI.LocalScope = "foo::";
+         HI.Type = "template <typename> class";
+       }},
+      {// NonType Template Parameter
+       R"cpp(
+          template <int [[^T]] = 5> void foo();
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "T";
+         HI.Kind = index::SymbolKind::NonTypeTemplateParm;
+         HI.NamespaceScope = "";
+         HI.Definition = "int T = 5";
+         HI.LocalScope = "foo::";
+         HI.Type = "int";
+       }},
   };
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Code);
 
     Annotations T(Case.Code);
     TestTU TU = TestTU::withCode(T.code());
     TU.ExtraArgs.push_back("-std=c++17");
     // Types might be different depending on the target triplet, we chose a

clang/include/clang/Index/IndexSymbol.h

enum class SymbolKind : uint8_t {
   StaticProperty,
 
   Constructor,
   Destructor,
   ConversionFunction,
 
   Parameter,
   Using,
+  TemplateTypeParm,
+  TemplateTemplateParm,
+  NonTypeTemplateParm,
 };
 
 enum class SymbolLanguage : uint8_t {
   C,
   ObjC,
   CXX,
   Swift,
 };

clang/lib/Index/IndexSymbol.cpp

if (const TagDecl *TD = dyn_cast<TagDecl>(D)) {
     case Decl::VarTemplateSpecialization:
     case Decl::VarTemplatePartialSpecialization:
     case Decl::ImplicitParam:
     case Decl::ParmVar:
     case Decl::Var:
     case Decl::VarTemplate:
       llvm_unreachable("variables handled before");
       break;
+    case Decl::TemplateTypeParm:
+      Info.Kind = SymbolKind::TemplateTypeParm;
+      break;
+    case Decl::TemplateTemplateParm:
+      Info.Kind = SymbolKind::TemplateTemplateParm;
+      break;
+    case Decl::NonTypeTemplateParm:
+      Info.Kind = SymbolKind::NonTypeTemplateParm;
+      break;
     // Other decls get the 'unknown' kind.
     default:
       break;
     }
   }
 
   if (Info.Kind == SymbolKind::Unknown)
     return Info;
StringRef index::getSymbolKindString(SymbolKind K) {
   case SymbolKind::InstanceProperty: return "instance-property";
   case SymbolKind::ClassProperty: return "class-property";
   case SymbolKind::StaticProperty: return "static-property";
   case SymbolKind::Constructor: return "constructor";
   case SymbolKind::Destructor: return "destructor";
   case SymbolKind::ConversionFunction: return "conversion-func";
   case SymbolKind::Parameter: return "param";
   case SymbolKind::Using: return "using";
+  case SymbolKind::TemplateTypeParm: return "template-type-param";
+  case SymbolKind::TemplateTemplateParm: return "template-template-param";
+  case SymbolKind::NonTypeTemplateParm: return "non-type-template-param";
   }
   llvm_unreachable("invalid symbol kind");
 }
 
 StringRef index::getSymbolSubKindString(SymbolSubKind K) {
   switch (K) {
   case SymbolSubKind::None: return "<none>";
   case SymbolSubKind::CXXCopyConstructor: return "cxx-copy-ctor";

clang/tools/libclang/CXIndexDataConsumer.cpp

 
 static CXIdxEntityKind getEntityKindFromSymbolKind(SymbolKind K, SymbolLanguage Lang) {
   switch (K) {
   case SymbolKind::Unknown:
   case SymbolKind::Module:
   case SymbolKind::Macro:
   case SymbolKind::ClassProperty:
   case SymbolKind::Using:
+  case SymbolKind::TemplateTypeParm:
+  case SymbolKind::TemplateTemplateParm:
+  case SymbolKind::NonTypeTemplateParm:
     return CXIdxEntity_Unexposed;
 
   case SymbolKind::Enum: return CXIdxEntity_Enum;
   case SymbolKind::Struct: return CXIdxEntity_Struct;
   case SymbolKind::Union: return CXIdxEntity_Union;
   case SymbolKind::TypeAlias:
     if (Lang == SymbolLanguage::CXX)
       return CXIdxEntity_CXXTypeAlias;

clang/unittests/Index/IndexTests.cpp

EXPECT_THAT(Index->Symbols, AllOf(Not(Contains(QName("Foo::T"))),
                                     Not(Contains(QName("Foo::I"))),
                                     Not(Contains(QName("Foo::C"))),
                                     Not(Contains(QName("Foo::NoRef")))));
 
   Opts.IndexTemplateParameters = true;
   Index->Symbols.clear();
   tooling::runToolOnCode(std::make_unique<IndexAction>(Index, Opts), Code);
   EXPECT_THAT(Index->Symbols,
-              AllOf(Contains(QName("Foo::T")), Contains(QName("Foo::I")),
-                    Contains(QName("Foo::C")), Contains(QName("Foo::NoRef"))));
+              AllOf(Contains(AllOf(QName("Foo::T"),
+                                   Kind(SymbolKind::TemplateTypeParm))),
+                    Contains(AllOf(QName("Foo::I"),
+                                   Kind(SymbolKind::NonTypeTemplateParm))),
+                    Contains(AllOf(QName("Foo::C"),
+                                   Kind(SymbolKind::TemplateTemplateParm))),
+                    Contains(QName("Foo::NoRef"))));
 }
 
 TEST(IndexTest, UsingDecls) {
   std::string Code = R"cpp(
     void foo(int bar);
     namespace std {
       using ::foo;
     }