[clangd] More sensible output for constructors/destructors in hover.

[sammccall]

Summary

Previously: both had type void() and return type void.
Now: neither have a type. Constructors return T, destructors return void.

[merge_guards_bot]

Build result: pass - 60137 tests passed, 0 failed and 729 were skipped.
Log files: console-log.txt, CMakeCache.txt

[hokein]

clang-tools-extra/clangd/XRefs.cpp

599

this looks reasonable though I'd prefer not setting the return type for ctor/dtor. If we decide to go down this path, we'd better make other "returnType" places (Symbol::ReturnType, CodeCompletion) align with this.

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

914

nit: add a test case for destructor?

[sammccall]

Update tests and rebase

[sammccall]

clang-tools-extra/clangd/XRefs.cpp

599

So code completion shows ~foo() as type void, and has no return type for constructors (including when it would matter for ranking!). Indexing code doesn't have a return type for either.
Per offline discussion, I'd rather fix those to include the type.

[merge_guards_bot]

Build result: pass - 60168 tests passed, 0 failed and 731 were skipped.
Log files: console-log.txt, CMakeCache.txt

clang-tools-extra/clangd/AST.h

 bool isExplicitTemplateSpecialization(const NamedDecl *D);
 
 /// Returns a nested name specifier loc of \p ND if it was present in the
 /// source, e.g.
 ///     void ns::something::foo() -> returns 'ns::something'
 ///     void foo() -> returns null
 NestedNameSpecifierLoc getQualifierLoc(const NamedDecl &ND);
 
+// Returns a type corresponding to a declaration of that type.
+// Unlike the method on ASTContext, attempts to preserve the type as-written
+// (i.e. vector<T*> rather than vector<type-parameter-0-0 *>.
+QualType declaredType(const TypeDecl *D);
+
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_

clang-tools-extra/clangd/AST.cpp

std::string printType(const QualType QT, const DeclContext & Context){
   PrintingPolicy PP(Context.getParentASTContext().getPrintingPolicy());
   PP.SuppressUnwrittenScope = 1;
   PP.SuppressTagKeyword = 1;
   return shortenNamespace(
       QT.getAsString(PP),
       printNamespaceScope(Context) );
 }
 
+QualType declaredType(const TypeDecl *D) {
+  if (const auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(D))
+    if (const auto *TSI = CTSD->getTypeAsWritten())
+      return TSI->getType();
+  return D->getASTContext().getTypeDeclType(D);
+}
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/XRefs.cpp

static void enhanceFromIndex(HoverInfo &Hover, const Decl *D,
   if (!ID)
     return;
   LookupRequest Req;
   Req.IDs.insert(*ID);
   Index->lookup(
       Req, [&](const Symbol &S) { Hover.Documentation = S.Documentation; });
 }
 
+// Populates Type, ReturnType, and Parameters for function-like decls.
+static void fillFunctionTypeAndParams(HoverInfo &HI, const Decl *D,
+                                      const FunctionDecl *FD,
+                                      const PrintingPolicy &Policy) {
+  HI.Parameters.emplace();
+  for (const ParmVarDecl *PVD : FD->parameters()) {
+    HI.Parameters->emplace_back();
+    auto &P = HI.Parameters->back();
+    if (!PVD->getType().isNull()) {
+      P.Type.emplace();
+      llvm::raw_string_ostream OS(*P.Type);
+      PVD->getType().print(OS, Policy);
+    } else {
+      std::string Param;
+      llvm::raw_string_ostream OS(Param);
+      PVD->dump(OS);
+      OS.flush();
+      elog("Got param with null type: {0}", Param);
+    }
+    if (!PVD->getName().empty())
+      P.Name = PVD->getNameAsString();
+    if (PVD->hasDefaultArg()) {
+      P.Default.emplace();
+      llvm::raw_string_ostream Out(*P.Default);
+      PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
+    }
+  }
+
+  if (const auto* CCD = llvm::dyn_cast<CXXConstructorDecl>(FD)) {
+    // Constructor's "return type" is the class type.
+    HI.ReturnType = declaredType(CCD->getParent()).getAsString(Policy);
+    // Don't provide any type for the constructor itself.
+  } else if (const auto* CDD = llvm::dyn_cast<CXXDestructorDecl>(FD)){
+    HI.ReturnType = "void";
+  } else {
+    HI.ReturnType = FD->getReturnType().getAsString(Policy);
+
+    QualType FunctionType = FD->getType();
+    if (const VarDecl *VD = llvm::dyn_cast<VarDecl>(D)) // Lambdas
+      FunctionType = VD->getType().getDesugaredType(D->getASTContext());
+    HI.Type = FunctionType.getAsString(Policy);
+  }
+  // FIXME: handle variadics.
+}
+
 /// Generate a \p Hover object given the declaration \p D.
 static HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
   HoverInfo HI;
   const ASTContext &Ctx = D->getASTContext();
 
   HI.NamespaceScope = getNamespaceScope(D);
   if (!HI.NamespaceScope->empty())
     HI.NamespaceScope->append("::");
if (const auto FTD = FD->getDescribedTemplate()) {
       HI.TemplateParameters =
           fetchTemplateParameters(FTD->getTemplateParameters(), Policy);
       D = FTD;
     }
   }
 
   // Fill in types and params.
   if (const FunctionDecl *FD = getUnderlyingFunction(D)) {
-    HI.ReturnType.emplace();
-    {
-      llvm::raw_string_ostream OS(*HI.ReturnType);
-      FD->getReturnType().print(OS, Policy);
-    }
-
-    HI.Parameters.emplace();
-    for (const ParmVarDecl *PVD : FD->parameters()) {
-      HI.Parameters->emplace_back();
-      auto &P = HI.Parameters->back();
-      if (!PVD->getType().isNull()) {
-        P.Type.emplace();
-        llvm::raw_string_ostream OS(*P.Type);
-        PVD->getType().print(OS, Policy);
-      } else {
-        std::string Param;
-        llvm::raw_string_ostream OS(Param);
-        PVD->dump(OS);
-        OS.flush();
-        elog("Got param with null type: {0}", Param);
-      }
-      if (!PVD->getName().empty())
-        P.Name = PVD->getNameAsString();
-      if (PVD->hasDefaultArg()) {
-        P.Default.emplace();
-        llvm::raw_string_ostream Out(*P.Default);
-        PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
-      }
-    }
-
-    HI.Type.emplace();
-    llvm::raw_string_ostream TypeOS(*HI.Type);
-    // Lambdas
-    if (const VarDecl *VD = llvm::dyn_cast<VarDecl>(D))
-      VD->getType().getDesugaredType(D->getASTContext()).print(TypeOS, Policy);
-    // Functions
-    else
-      FD->getType().print(TypeOS, Policy);
-    // FIXME: handle variadics.
+    fillFunctionTypeAndParams(HI, D, FD, Policy);
   } else if (const auto *VD = dyn_cast<ValueDecl>(D)) {
     HI.Type.emplace();
     llvm::raw_string_ostream OS(*HI.Type);
     VD->getType().print(OS, Policy);
   }
 
   // Fill in value with evaluated initializer if possible.
   // FIXME(kadircet): Also set Value field for expressions like "sizeof" and

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

void foo())cpp";
          HI.Name = "X<T *>";
          HI.NamespaceScope = "";
          HI.Kind = SymbolKind::Class;
          HI.Definition = "template <typename T> class X<T *> {}";
        }},
       // Constructor of partially-specialized class template
       {R"cpp(
           template<typename> struct X;
           template<typename T> struct X<T*>{ [[^X]](); };
           )cpp",
        [](HoverInfo &HI) {
          HI.NamespaceScope = "";
          HI.Name = "X";
          HI.LocalScope = "X::";        // FIXME: Should be X<T *>::
          HI.Kind = SymbolKind::Constructor;
-         HI.Type = "void ()";          // FIXME: Should be None
-         HI.ReturnType = "void";       // FIXME: Should be None or X<T*>
+         HI.ReturnType = "X<T *>";
          HI.Definition = "X()";
          HI.Parameters.emplace();
        }},
+      {"class X { [[^~]]X(); };", // FIXME: Should be [[~X]]()
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.Name = "~X";
+         HI.LocalScope = "X::";
+         HI.Kind = SymbolKind::Constructor;
+         HI.ReturnType = "void";
+         HI.Definition = "~X()";
+         HI.Parameters.emplace();
+       }},
 
       // auto on lambda
       {R"cpp(
         void foo() {
           [[au^to]] lamb = []{};
         }
         )cpp",
        [](HoverInfo &HI) {