[PATCH] D50726: [clangd] Show function documentation in sigHelp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340004: [clangd] Show function documentation in signature
help (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D50726
Files:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
clang-tools-extra/trunk/clangd/CodeCompletionStrings.h
Index: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
===
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
@@ -51,44 +51,26 @@
// 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(Decl)) {
+ return Result.getDeclaration() ? getDeclComment(Ctx,
*Result.getDeclaration())
+ : "";
+}
+
+std::string getDeclComment(const ASTContext , const NamedDecl ) {
+ if (llvm::isa(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 ,
- const CodeCompleteConsumer::OverloadCandidate ,
- unsigned ArgIndex, bool CommentsFromHeaders) {
- auto *Func = Result.getFunction();
- if (!Func)
-return "";
- const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
+ const RawComment *RC = getCompletionComment(Ctx, );
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 , std::string *Signature,
Index: clang-tools-extra/trunk/clangd/CodeCompletionStrings.h
===
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.h
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.h
@@ -33,18 +33,8 @@
const CodeCompletionResult ,
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 ,
- const CodeCompleteConsumer::OverloadCandidate ,
- unsigned ArgIndex, bool CommentsFromHeaders);
+/// Similar to getDocComment, but returns the comment for a NamedDecl.
+std::string getDeclComment(const ASTContext , const NamedDecl );
/// Formats the signature for an item, as a display string and snippet.
/// e.g. for const_reference std::vector::at(size_type) const, this returns:
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -729,8 +729,9 @@
// 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())
+ : ""));
}
[PATCH] D50726: [clangd] Show function documentation in sigHelp
ioeric added inline comments. Comment at: clangd/CodeCompletionStrings.cpp:52 // get this declaration, so we don't show documentation in that case. if (Result.Kind != CodeCompletionResult::RK_Declaration) return ""; ilya-biryukov wrote: > ioeric wrote: > > `RK_Pattern` can also have declaration. Do we want to consider those? > 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. `getDeclaration()` has started supporting `RK_Pattern` recently, so this should be fixable now. But feel free to leave it to future. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50726: [clangd] Show function documentation in sigHelp
ilya-biryukov added inline comments. Comment at: clangd/CodeCompletionStrings.cpp:52 // get this declaration, so we don't show documentation in that case. if (Result.Kind != CodeCompletionResult::RK_Declaration) return ""; ioeric wrote: > `RK_Pattern` can also have declaration. Do we want to consider those? 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. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50726: [clangd] Show function documentation in sigHelp
ilya-biryukov updated this revision to Diff 161187.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.
- Turn ifs to ternary ops
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50726
Files:
clangd/CodeComplete.cpp
clangd/CodeCompletionStrings.cpp
clangd/CodeCompletionStrings.h
Index: clangd/CodeCompletionStrings.h
===
--- clangd/CodeCompletionStrings.h
+++ clangd/CodeCompletionStrings.h
@@ -32,18 +32,8 @@
const CodeCompletionResult ,
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 ,
- const CodeCompleteConsumer::OverloadCandidate ,
- unsigned ArgIndex, bool CommentsFromHeaders);
+/// Similar to getDocComment, but returns the comment for a NamedDecl.
+std::string getDeclComment(const ASTContext , const NamedDecl );
/// Formats the signature for an item, as a display string and snippet.
/// e.g. for const_reference std::vector::at(size_type) const, this returns:
Index: clangd/CodeCompletionStrings.cpp
===
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -51,44 +51,26 @@
// 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(Decl)) {
+ return Result.getDeclaration() ? getDeclComment(Ctx,
*Result.getDeclaration())
+ : "";
+}
+
+std::string getDeclComment(const ASTContext , const NamedDecl ) {
+ if (llvm::isa(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 ,
- const CodeCompleteConsumer::OverloadCandidate ,
- unsigned ArgIndex, bool CommentsFromHeaders) {
- auto *Func = Result.getFunction();
- if (!Func)
-return "";
- const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
+ const RawComment *RC = getCompletionComment(Ctx, );
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 , std::string *Signature,
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -729,8 +729,9 @@
// 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 , const ScoredSignature ) {
Index: clangd/CodeCompletionStrings.h
===
--- clangd/CodeCompletionStrings.h
+++ clangd/CodeCompletionStrings.h
@@ -32,18 +32,8 @@
const
[PATCH] D50726: [clangd] Show function documentation in sigHelp
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: clangd/CodeCompletionStrings.cpp:52 // get this declaration, so we don't show documentation in that case. if (Result.Kind != CodeCompletionResult::RK_Declaration) return ""; `RK_Pattern` can also have declaration. Do we want to consider those? Comment at: clangd/CodeCompletionStrings.cpp:57 +return ""; + return getDeclComment(Ctx, *Decl); +} nit: maybe `return Decl ? getDeclComment(...) : "";` Comment at: clangd/CodeCompletionStrings.cpp:77 return ""; return Doc; } nit: `return looksLikeDocComment(Doc) ? Doc : "";` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50726: [clangd] Show function documentation in sigHelp
ilya-biryukov updated this revision to Diff 161011.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- Expose getDeclComment instead of getDocComment
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50726
Files:
clangd/CodeComplete.cpp
clangd/CodeCompletionStrings.cpp
clangd/CodeCompletionStrings.h
Index: clangd/CodeCompletionStrings.h
===
--- clangd/CodeCompletionStrings.h
+++ clangd/CodeCompletionStrings.h
@@ -32,18 +32,8 @@
const CodeCompletionResult ,
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 ,
- const CodeCompleteConsumer::OverloadCandidate ,
- unsigned ArgIndex, bool CommentsFromHeaders);
+/// Similar to getDocComment, but returns the comment for a NamedDecl.
+std::string getDeclComment(const ASTContext , const NamedDecl );
/// Formats the signature for an item, as a display string and snippet.
/// e.g. for const_reference std::vector::at(size_type) const, this returns:
Index: clangd/CodeCompletionStrings.cpp
===
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -52,34 +52,20 @@
if (Result.Kind != CodeCompletionResult::RK_Declaration)
return "";
auto *Decl = Result.getDeclaration();
- if (!Decl || llvm::isa(Decl)) {
+ if (!Decl)
+return "";
+ return getDeclComment(Ctx, *Decl);
+}
+
+std::string getDeclComment(const ASTContext , const NamedDecl ) {
+ if (llvm::isa(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 ,
- const CodeCompleteConsumer::OverloadCandidate ,
- unsigned ArgIndex, bool CommentsFromHeaders) {
- auto *Func = Result.getFunction();
- if (!Func)
-return "";
- const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
+ const RawComment *RC = getCompletionComment(Ctx, );
if (!RC)
return "";
// Sanity check that the comment does not come from the PCH. We choose to not
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -729,8 +729,9 @@
// 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 , const ScoredSignature ) {
Index: clangd/CodeCompletionStrings.h
===
--- clangd/CodeCompletionStrings.h
+++ clangd/CodeCompletionStrings.h
@@ -32,18 +32,8 @@
const CodeCompletionResult ,
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
[PATCH] D50726: [clangd] Show function documentation in sigHelp
ioeric added inline comments. Comment at: clangd/CodeCompletionStrings.cpp:81 +std::string +getDocComment(const ASTContext , + const CodeCompleteConsumer::OverloadCandidate , 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? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50726: [clangd] Show function documentation in sigHelp
ilya-biryukov added a comment. There's a test for the new behavior in https://reviews.llvm.org/D50726 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50726: [clangd] Show function documentation in sigHelp
ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, ioeric, kadircet.
Herald added subscribers: arphaman, jkorous, MaskRay.
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.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50726
Files:
clangd/CodeComplete.cpp
clangd/CodeCompletionStrings.cpp
clangd/CodeCompletionStrings.h
Index: clangd/CodeCompletionStrings.h
===
--- clangd/CodeCompletionStrings.h
+++ clangd/CodeCompletionStrings.h
@@ -32,18 +32,10 @@
const CodeCompletionResult ,
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 ,
- const CodeCompleteConsumer::OverloadCandidate ,
- unsigned ArgIndex, bool CommentsFromHeaders);
+getDocComment(const ASTContext ,
+ const CodeCompleteConsumer::OverloadCandidate ,
+ bool CommentsFromHeaders);
/// Formats the signature for an item, as a display string and snippet.
/// e.g. for const_reference std::vector::at(size_type) const, this returns:
Index: clangd/CodeCompletionStrings.cpp
===
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -41,28 +41,17 @@
return CommentText.find_first_not_of("/*-= \t\r\n") != llvm::StringRef::npos;
}
-} // namespace
-
-std::string getDocComment(const ASTContext ,
- const CodeCompletionResult ,
- 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(Decl)) {
+std::string getDeclComment(const ASTContext , const NamedDecl ) {
+ if (llvm::isa(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);
+ const RawComment *RC = getCompletionComment(Ctx, );
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()));
@@ -72,23 +61,30 @@
return Doc;
}
-std::string
-getParameterDocComment(const ASTContext ,
- const CodeCompleteConsumer::OverloadCandidate ,
- unsigned ArgIndex, bool CommentsFromHeaders) {
- auto *Func = Result.getFunction();
- if (!Func)
+} // namespace
+
+std::string getDocComment(const ASTContext ,
+ const CodeCompletionResult ,
+ 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 "";
- const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
- if (!RC)
+ auto *Decl = Result.getDeclaration();
+ if (!Decl)
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 getDeclComment(Ctx, *Decl);
+}
+
