[clang-tools-extra] 713c30b - [clangd] Don't consider class template params part of constructor name.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2019-11-15T17:32:55+01:00
New Revision: 713c30b389602eda5c70b696e8c640487cc8b2cb

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

LOG: [clangd] Don't consider class template params part of constructor name.

Summary:
This is shorter and usually the extra info is noise.
There are cases where the params become type-parameter-0-0 that are hard to fix.

This affects a few features:
 - 'name' field in structured hover API (not exposed yet)
 - 'name' field in locateSymbolAt (not exposed in LSP)
 - 'document/symbol' - the symbol is hierarchically nested in the class
   template, or written as foo<t>::foo when defined out-of-line.

Added a test case for hover from https://github.com/clangd/clangd/issues/76.
This patch fixes one field, but no fewer than four others are wrong!
I'll fix them...

Reviewers: hokein

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/AST.cpp
    clang-tools-extra/clangd/XRefs.cpp
    clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index 1958ebf80c4c..af04fbd0d4d4 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -127,6 +127,8 @@ std::string printName(const ASTContext &Ctx, const NamedDecl &ND) {
   std::string Name;
   llvm::raw_string_ostream Out(Name);
   PrintingPolicy PP(Ctx.getLangOpts());
+  // We don't consider a class template's args part of the constructor name.
+  PP.SuppressTemplateArgsInCXXConstructors = true;
 
   // Handle 'using namespace'. They all have the same name - <using-directive>.
   if (auto *UD = llvm::dyn_cast<UsingDirectiveDecl>(&ND)) {

diff  --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index dc5d5db978fc..2f4cfc2608e5 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -417,6 +417,7 @@ static std::string getLocalScope(const Decl *D) {
   auto GetName = [](const Decl *D) {
     const NamedDecl *ND = dyn_cast<NamedDecl>(D);
     std::string Name = ND->getNameAsString();
+    // FIXME(sammccall): include template params/specialization args?.
     if (!Name.empty())
       return Name;
     if (auto RD = dyn_cast<RecordDecl>(D))

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 0de112735566..e896096b7ef4 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -897,7 +897,7 @@ void foo())cpp";
          HI.Definition = "int test";
          HI.Type = "int";
        }},
-      // Partially-specialized class decl. (formerly type-parameter-0-0)
+      // Partially-specialized class template. (formerly type-parameter-0-0)
       {R"cpp(
         template <typename T> class X;
         template <typename T> class [[^X]]<T*> {};
@@ -908,6 +908,21 @@ void foo())cpp";
          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::Method; // FIXME: Should be Constructor
+         HI.Type = "void ()";          // FIXME: Should be None
+         HI.ReturnType = "void";       // FIXME: Should be None or X<T*>
+         HI.Definition = "X<type - parameter - 0 - 0 *>()"; // FIXME: --> X()
+         HI.Parameters.emplace();
+       }},
 
       // auto on lambda
       {R"cpp(