[clangd] Show function documentation in sigHelp

[ilya-biryukov]

Summary

Previously, clangd was trying to show documentation for the active
parameter instead, which is wrong per LSP specification.

Moreover, the code path that attempts to get parameter comments never
succeds, because no attempt is made to parse function doc comment and
extract parameter-specific parts out of it. So we also remove the code
that claims to fetch parameter comments: it is not used anymore and is
incorrect.

[ilya-biryukov]

There's a test for the new behavior in D50726

[ioeric]

clangd/CodeCompletionStrings.cpp

81 ↗

(On Diff #160639)

The function doesn't seem to carry its weight, and the difference from the other overload is a bit subtle. Maybe we could expose getDeclComent instead?

[ilya-biryukov]

• Expose getDeclComment instead of getDocComment

[ioeric]

lgtm

clangd/CodeCompletionStrings.cpp

52 ↗

(On Diff #161011)

RK_Pattern can also have declaration. Do we want to consider those?

57 ↗

(On Diff #161011)

nit: maybe return Decl ? getDeclComment(...) : "";

77 ↗

(On Diff #161011)

nit: return looksLikeDocComment(Doc) ? Doc : "";

[ilya-biryukov]

• Turn ifs to ternary ops

[ilya-biryukov]

clangd/CodeCompletionStrings.cpp

52 ↗

(On Diff #161011)

See the FIXME at the top, we don't have the API exposed from clang to get the proper declarations (IIRC, this only shows up for ObjC properties and we need to look at both getters and setters when looking for a comment, and the clang code does not expose the API to do that).

We should probably fix this, but that's a separate issue.

[ioeric]

clangd/CodeCompletionStrings.cpp

52 ↗

(On Diff #161011)

getDeclaration() has started supporting RK_Pattern recently, so this should be fixable now. But feel free to leave it to future.

clang-tools-extra/trunk/clangd/CodeComplete.cpp

for (unsigned I = 0; I < NumCandidates; ++I) {
       }
 
       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)));
+          Candidate.getFunction()
+              ? getDeclComment(S.getASTContext(), *Candidate.getFunction())
+              : ""));
     }
     std::sort(ScoredSignatures.begin(), ScoredSignatures.end(),
               [](const ScoredSignature &L, const ScoredSignature &R) {
                 // Ordering follows:
                 // - Less number of parameters is better.
                 // - Function is better than FunctionType which is better than
                 // Function Template.
                 // - High score is better.

clang-tools-extra/trunk/clangd/CodeCompletionStrings.h

 /// Returns empty string when no comment is available.
 /// If \p CommentsFromHeaders parameter is set, only comments from the main
 /// file will be returned. It is used to workaround crashes when parsing
 /// comments in the stale headers, coming from completion preamble.
 std::string getDocComment(const ASTContext &Ctx,
                           const CodeCompletionResult &Result,
                           bool CommentsFromHeaders);
 
-/// Gets a minimally formatted documentation for parameter of \p Result,
-/// corresponding to argument number \p ArgIndex.
-/// This currently looks for comments attached to the parameter itself, and
-/// doesn't extract them from function documentation.
-/// Returns empty string when no comment is available.
-/// If \p CommentsFromHeaders parameter is set, only comments from the main
-/// file will be returned. It is used to workaround crashes when parsing
-/// comments in the stale headers, coming from completion preamble.
-std::string
-getParameterDocComment(const ASTContext &Ctx,
-                       const CodeCompleteConsumer::OverloadCandidate &Result,
-                       unsigned ArgIndex, bool CommentsFromHeaders);
+/// Similar to getDocComment, but returns the comment for a NamedDecl.
+std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &D);
 
 /// Formats the signature for an item, as a display string and snippet.
 /// e.g. for const_reference std::vector<T>::at(size_type) const, this returns:
 ///   *Signature = "(size_type) const"
 ///   *Snippet = "(${0:size_type})"
 /// If set, RequiredQualifiers is the text that must be typed before the name.
 /// e.g "Base::" when calling a base class member function that's hidden.
 void getSignature(const CodeCompletionString &CCS, std::string *Signature,

clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp

 std::string getDocComment(const ASTContext &Ctx,
                           const CodeCompletionResult &Result,
                           bool CommentsFromHeaders) {
   // FIXME: clang's completion also returns documentation for RK_Pattern if they
   // contain a pattern for ObjC properties. Unfortunately, there is no API to
   // get this declaration, so we don't show documentation in that case.
   if (Result.Kind != CodeCompletionResult::RK_Declaration)
     return "";
-  auto *Decl = Result.getDeclaration();
-  if (!Decl || llvm::isa<NamespaceDecl>(Decl)) {
+  return Result.getDeclaration() ? getDeclComment(Ctx, *Result.getDeclaration())
+                                 : "";
+}
+
+std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &Decl) {
+  if (llvm::isa<NamespaceDecl>(Decl)) {
     // Namespaces often have too many redecls for any particular redecl comment
     // to be useful. Moreover, we often confuse file headers or generated
     // comments with namespace comments. Therefore we choose to just ignore
     // the comments for namespaces.
     return "";
   }
-  const RawComment *RC = getCompletionComment(Ctx, Decl);
-  if (!RC)
-    return "";
-
-  // Sanity check that the comment does not come from the PCH. We choose to not
-  // write them into PCH, because they are racy and slow to load.
-  assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getBeginLoc()));
-  std::string Doc = RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
-  if (!looksLikeDocComment(Doc))
-    return "";
-  return Doc;
-}
-
-std::string
-getParameterDocComment(const ASTContext &Ctx,
-                       const CodeCompleteConsumer::OverloadCandidate &Result,
-                       unsigned ArgIndex, bool CommentsFromHeaders) {
-  auto *Func = Result.getFunction();
-  if (!Func)
-    return "";
-  const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
+  const RawComment *RC = getCompletionComment(Ctx, &Decl);
   if (!RC)
     return "";
   // Sanity check that the comment does not come from the PCH. We choose to not
   // write them into PCH, because they are racy and slow to load.
   assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getBeginLoc()));
   std::string Doc = RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
-  if (!looksLikeDocComment(Doc))
-    return "";
-  return Doc;
+  return looksLikeDocComment(Doc) ? Doc : "";
 }
 
 void getSignature(const CodeCompletionString &CCS, std::string *Signature,
                   std::string *Snippet, std::string *RequiredQualifiers) {
   unsigned ArgCount = 0;
   for (const auto &Chunk : CCS) {
     // Informative qualifier chunks only clutter completion results, skip
     // them.