[clangd] Show non-instantiated decls in signatureHelp

[ilya-biryukov]

Summary

To avoid producing very verbose output in substitutions involving
typedefs, e.g.

T -> std::vector<std::string>::iterator

gets turned into an unreadable mess when printed out for libstdc++,
result contains internal types (std::__Vector_iterator<...>) and
expanded well-defined typedefs (std::basic_string<char>).

Until we improve the presentation code in clang, going with
non-instantiated decls looks like a better UX trade-off.

[hokein]

looks good.

clangd/CodeComplete.cpp

721

nit: auto*, the same below.

[ilya-biryukov]

• Use 'auto*' instead of 'auto'

[ilya-biryukov]

clangd/CodeComplete.cpp

721

Good catch, thanks!

[yroux]

Hi Ilya,

This commit broke some of ARM's bots, log are available here:

http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/3120/steps/ninja%20check%201/logs/stdio

Thanks
Yvan

clangd/CodeComplete.cpp

void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
     // FIXME(rwols): How can we determine the "active overload candidate"?
     // Right now the overloaded candidates seem to be provided in a "best fit"
     // order, so I'm not too worried about this.
     SigHelp.activeSignature = 0;
     assert(CurrentArg <= (unsigned)std::numeric_limits<int>::max() &&
            "too many arguments");
     SigHelp.activeParameter = static_cast<int>(CurrentArg);
     for (unsigned I = 0; I < NumCandidates; ++I) {
-      const auto &Candidate = Candidates[I];
+      OverloadCandidate Candidate = Candidates[I];
+      // We want to avoid showing instantiated signatures, because they may be
+      // long in some cases (e.g. when 'T' is substituted with 'std::string', we
+      // would get 'std::basic_string<char>').
+      if (auto Func = Candidate.getFunction()) {
+        if (auto Pattern = Func->getTemplateInstantiationPattern())
+          Candidate = OverloadCandidate(Pattern);
+      }
+
       const auto *CCS = Candidate.CreateSignatureString(
           CurrentArg, S, *Allocator, CCTUInfo, true);
       assert(CCS && "Expected the CodeCompletionString to be non-null");
       // FIXME: for headers, we need to get a comment from the index.
       ScoredSignatures.push_back(processOverloadCandidate(
           Candidate, *CCS,
           getParameterDocComment(S.getASTContext(), Candidate, CurrentArg,
                                  /*CommentsFromHeaders=*/false)));

unittests/clangd/CodeCompleteTests.cpp

EXPECT_THAT(
           Sig("foo(float x, int y) -> void", {"float x", "int y"}),
           Sig("foo(int x, float y) -> void", {"int x", "float y"}),
           Sig("foo(float x, float y) -> void", {"float x", "float y"})));
   // We always prefer the first signature.
   EXPECT_EQ(0, Results.activeSignature);
   EXPECT_EQ(0, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, InstantiatedSignatures) {
+  EXPECT_THAT(signatures(R"cpp(
+    template <class T>
+    void foo(T, T, T);
+
+    int main() {
+      foo<int>(^);
+    }
+  )cpp")
+                  .signatures,
+              ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+    template <class T>
+    void foo(T, T, T);
+
+    int main() {
+      foo(10, ^);
+    })cpp")
+                  .signatures,
+              ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+    template <class ...T>
+    void foo(T...);
+
+    int main() {
+      foo<int>(^);
+    }
+  )cpp")
+                  .signatures,
+              ElementsAre(Sig("foo(T...) -> void", {"T..."})));
+
+  // It is debatable whether we should substitute the outer template parameter
+  // ('T') in that case. Currently we don't substitute it in signature help, but
+  // do substitute in code complete.
+  // FIXME: make code complete and signature help consistent, figure out which
+  // way is better.
+  EXPECT_THAT(signatures(R"cpp(
+    template <class T>
+    struct X {
+      template <class U>
+      void foo(T, U);
+    };
+
+    int main() {
+      X<int>().foo<double>(^)
+    }
+  )cpp")
+                  .signatures,
+              ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang