ilya-biryukov updated this revision to Diff 145883.
ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added a comment.
- Renames and other comments
- Don't include brief comments in signature help either, comments there are
also handled by the code completion code now.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45999
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/CodeCompletionStrings.cpp
clangd/CodeCompletionStrings.h
clangd/index/SymbolCollector.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/CodeCompletionStringsTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -490,11 +490,11 @@
}
)";
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAre(QName("nx"),
- AllOf(QName("nx::ff"),
- Labeled("ff(int x, double y)"),
- Detail("int"), Doc("Foo comment."))));
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"), AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment."))));
}
TEST_F(SymbolCollectorTest, PlainAndSnippet) {
Index: unittests/clangd/CodeCompletionStringsTests.cpp
===================================================================
--- unittests/clangd/CodeCompletionStringsTests.cpp
+++ unittests/clangd/CodeCompletionStringsTests.cpp
@@ -51,23 +51,24 @@
}
TEST_F(CompletionStringTest, Documentation) {
- Builder.addBriefComment("Is this brief?");
- EXPECT_EQ(getDocumentation(*Builder.TakeString()), "Is this brief?");
+ Builder.addBriefComment("This is ignored");
+ EXPECT_EQ(formatDocumentation(*Builder.TakeString(), "Is this brief?"),
+ "Is this brief?");
}
TEST_F(CompletionStringTest, DocumentationWithAnnotation) {
- Builder.addBriefComment("Is this brief?");
+ Builder.addBriefComment("This is ignored");
Builder.AddAnnotation("Ano");
- EXPECT_EQ(getDocumentation(*Builder.TakeString()),
+ EXPECT_EQ(formatDocumentation(*Builder.TakeString(), "Is this brief?"),
"Annotation: Ano\n\nIs this brief?");
}
TEST_F(CompletionStringTest, MultipleAnnotations) {
Builder.AddAnnotation("Ano1");
Builder.AddAnnotation("Ano2");
Builder.AddAnnotation("Ano3");
- EXPECT_EQ(getDocumentation(*Builder.TakeString()),
+ EXPECT_EQ(formatDocumentation(*Builder.TakeString(), ""),
"Annotations: Ano1 Ano2 Ano3\n");
}
@@ -97,7 +98,7 @@
TEST_F(CompletionStringTest, FunctionSnippet) {
Builder.AddResultTypeChunk("result no no");
- Builder.addBriefComment("Foo's comment");
+ Builder.addBriefComment("This comment is ignored");
Builder.AddTypedTextChunk("Foo");
Builder.AddChunk(CodeCompletionString::CK_LeftParen);
Builder.AddPlaceholderChunk("p1");
@@ -113,7 +114,7 @@
labelAndInsertText(*CCS, /*EnableSnippets=*/true);
EXPECT_EQ(Label, "Foo(p1, p2)");
EXPECT_EQ(InsertText, "Foo(${1:p1}, ${2:p2})");
- EXPECT_EQ(getDocumentation(*CCS), "Foo's comment");
+ EXPECT_EQ(formatDocumentation(*CCS, "Foo's comment"), "Foo's comment");
EXPECT_EQ(getFilterText(*CCS), "Foo");
}
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -243,8 +243,7 @@
// completion. At least there aren't any we're aware of.
EXPECT_THAT(Results.items, Not(Contains(Kind(CompletionItemKind::Snippet))));
// Check documentation.
- EXPECT_IFF(Opts.IncludeBriefComments, Results.items,
- Contains(IsDocumented()));
+ EXPECT_IFF(Opts.IncludeComments, Results.items, Contains(IsDocumented()));
}
void TestGlobalScopeCompletion(clangd::CodeCompleteOptions Opts) {
@@ -289,8 +288,7 @@
AllOf(Has("local_var"), Has("LocalClass"),
Contains(Kind(CompletionItemKind::Snippet))));
// Check documentation.
- EXPECT_IFF(Opts.IncludeBriefComments, Results.items,
- Contains(IsDocumented()));
+ EXPECT_IFF(Opts.IncludeComments, Results.items, Contains(IsDocumented()));
}
TEST(CompletionTest, CompletionOptions) {
@@ -301,7 +299,7 @@
// We used to test every combination of options, but that got too slow (2^N).
auto Flags = {
&clangd::CodeCompleteOptions::IncludeMacros,
- &clangd::CodeCompleteOptions::IncludeBriefComments,
+ &clangd::CodeCompleteOptions::IncludeComments,
&clangd::CodeCompleteOptions::EnableSnippets,
&clangd::CodeCompleteOptions::IncludeCodePatterns,
&clangd::CodeCompleteOptions::IncludeIneligibleResults,
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -296,7 +296,8 @@
const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
SymbolID ID) {
- auto &SM = ND.getASTContext().getSourceManager();
+ auto &Ctx = ND.getASTContext();
+ auto &SM = Ctx.getSourceManager();
std::string QName;
llvm::raw_string_ostream OS(QName);
@@ -327,7 +328,7 @@
const auto *CCS = SymbolCompletion.CreateCodeCompletionString(
*ASTCtx, *PP, CodeCompletionContext::CCC_Name, *CompletionAllocator,
*CompletionTUInfo,
- /*IncludeBriefComments*/ true);
+ /*IncludeBriefComments*/ false);
std::string Label;
std::string SnippetInsertText;
std::string IgnoredLabel;
@@ -337,7 +338,8 @@
getLabelAndInsertText(*CCS, &IgnoredLabel, &PlainInsertText,
/*EnableSnippets=*/false);
std::string FilterText = getFilterText(*CCS);
- std::string Documentation = getDocumentation(*CCS);
+ std::string Documentation =
+ formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion));
std::string CompletionDetail = getDetail(*CCS);
std::string Include;
Index: clangd/CodeCompletionStrings.h
===================================================================
--- clangd/CodeCompletionStrings.h
+++ clangd/CodeCompletionStrings.h
@@ -17,19 +17,40 @@
#include "clang/Sema/CodeCompleteConsumer.h"
namespace clang {
+class ASTContext;
+
namespace clangd {
+/// Gets a minimally formatted documentation comment of \p Result, with comment
+/// markers stripped. See clang::RawComment::getFormattedText() for the detailed
+/// explanation of how the comment text is transformed.
+/// Returns empty string when no comment is available.
+std::string getDocComment(const ASTContext &Ctx,
+ const CodeCompletionResult &Result);
+
+/// Gets a minimally formatted documentation for parameter of \p Result,
+/// corresponding to argument number \p CurrentArg.
+/// Returns empty string when no comment is available.
+std::string
+getParameterDocComment(const ASTContext &Ctx,
+ const CodeCompleteConsumer::OverloadCandidate &Result,
+ unsigned CurrentArg);
+
/// Gets label and insert text for a completion item. For example, for function
/// `Foo`, this returns <"Foo(int x, int y)", "Foo"> without snippts enabled.
///
/// If \p EnableSnippets is true, this will try to use snippet for the insert
/// text. Otherwise, the insert text will always be plain text.
void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label,
std::string *InsertText, bool EnableSnippets);
-/// Gets the documentation for a completion item. For example, comment for the
-/// a class declaration.
-std::string getDocumentation(const CodeCompletionString &CCS);
+/// Assembles formatted documentation for a completion result. This includes
+/// documentation comments and other relevant information like annotations.
+///
+/// \param DocComment is a documentation comment for the original declaration,
+/// it should be obtained via getDocComment or getParameterDocComment.
+std::string formatDocumentation(const CodeCompletionString &CCS,
+ llvm::StringRef DocComment);
/// Gets detail to be used as the detail field in an LSP completion item. This
/// is usually the return type of a function.
Index: clangd/CodeCompletionStrings.cpp
===================================================================
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -8,6 +8,7 @@
//===---------------------------------------------------------------------===//
#include "CodeCompletionStrings.h"
+#include "clang/AST/RawCommentList.h"
#include <utility>
namespace clang {
@@ -122,13 +123,43 @@
} // namespace
+std::string getDocComment(const ASTContext &Ctx,
+ const CodeCompletionResult &Result) {
+ // 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)
+ return "";
+ const RawComment *RC = getCompletionComment(Ctx, Decl);
+ if (!RC)
+ return "";
+ return RC->getFormattedText(Ctx);
+}
+
+std::string
+getParameterDocComment(const ASTContext &Ctx,
+ const CodeCompleteConsumer::OverloadCandidate &Result,
+ unsigned CurrentArg) {
+ auto Func = Result.getFunction();
+ if (!Func)
+ return "";
+ const RawComment *RC = getParameterComment(Ctx, Result, CurrentArg);
+ if (!RC)
+ return "";
+ return RC->getFormattedText(Ctx);
+}
+
void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label,
std::string *InsertText, bool EnableSnippets) {
return EnableSnippets ? processSnippetChunks(CCS, Label, InsertText)
: processPlainTextChunks(CCS, Label, InsertText);
}
-std::string getDocumentation(const CodeCompletionString &CCS) {
+std::string formatDocumentation(const CodeCompletionString &CCS,
+ llvm::StringRef DocComment) {
// Things like __attribute__((nonnull(1,3))) and [[noreturn]]. Present this
// information in the documentation field.
std::string Result;
@@ -146,13 +177,13 @@
}
}
// Add brief documentation (if there is any).
- if (CCS.getBriefComment() != nullptr) {
+ if (!DocComment.empty()) {
if (!Result.empty()) {
// This means we previously added annotations. Add an extra newline
// character to make the annotations stand out.
Result.push_back('\n');
}
- Result += CCS.getBriefComment();
+ Result += DocComment;
}
return Result;
}
Index: clangd/CodeComplete.h
===================================================================
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -44,10 +44,8 @@
/// Add macros to code completion results.
bool IncludeMacros = true;
- /// Add brief comments to completion items, if available.
- /// FIXME(ibiryukov): it looks like turning this option on significantly slows
- /// down completion, investigate if it can be made faster.
- bool IncludeBriefComments = true;
+ /// Add comments to code completion results, if available.
+ bool IncludeComments = true;
/// Include results that are not legal completions in the current context.
/// For example, private members are usually inaccessible.
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -258,15 +258,16 @@
CompletionItem build(llvm::StringRef FileName,
const CompletionItemScores &Scores,
const CodeCompleteOptions &Opts,
- CodeCompletionString *SemaCCS) const {
+ CodeCompletionString *SemaCCS,
+ llvm::StringRef SemaDocComment) const {
assert(bool(SemaResult) == bool(SemaCCS));
CompletionItem I;
if (SemaResult) {
I.kind = toCompletionItemKind(SemaResult->Kind, SemaResult->CursorKind);
getLabelAndInsertText(*SemaCCS, &I.label, &I.insertText,
Opts.EnableSnippets);
I.filterText = getFilterText(*SemaCCS);
- I.documentation = getDocumentation(*SemaCCS);
+ I.documentation = formatDocumentation(*SemaCCS, SemaDocComment);
I.detail = getDetail(*SemaCCS);
}
if (IndexResult) {
@@ -505,17 +506,17 @@
case CodeCompletionResult::RK_Pattern:
return Result.Pattern->getTypedText();
}
- auto *CCS = codeCompletionString(Result, /*IncludeBriefComments=*/false);
+ auto *CCS = codeCompletionString(Result);
return CCS->getTypedText();
}
// Build a CodeCompletion string for R, which must be from Results.
// The CCS will be owned by this recorder.
- CodeCompletionString *codeCompletionString(const CodeCompletionResult &R,
- bool IncludeBriefComments) {
+ CodeCompletionString *codeCompletionString(const CodeCompletionResult &R) {
// CodeCompletionResult doesn't seem to be const-correct. We own it, anyway.
return const_cast<CodeCompletionResult &>(R).CreateCodeCompletionString(
- *CCSema, CCContext, *CCAllocator, CCTUInfo, IncludeBriefComments);
+ *CCSema, CCContext, *CCAllocator, CCTUInfo,
+ /*IncludeBriefComments=*/false);
}
private:
@@ -597,7 +598,9 @@
const auto *CCS = Candidate.CreateSignatureString(
CurrentArg, S, *Allocator, CCTUInfo, true);
assert(CCS && "Expected the CodeCompletionString to be non-null");
- SigHelp.signatures.push_back(ProcessOverloadCandidate(Candidate, *CCS));
+ SigHelp.signatures.push_back(ProcessOverloadCandidate(
+ Candidate, *CCS,
+ getParameterDocComment(S.getASTContext(), Candidate, CurrentArg)));
}
}
@@ -610,11 +613,12 @@
// CompletionString.h.
SignatureInformation
ProcessOverloadCandidate(const OverloadCandidate &Candidate,
- const CodeCompletionString &CCS) const {
+ const CodeCompletionString &CCS,
+ llvm::StringRef DocComment) const {
SignatureInformation Result;
const char *ReturnType = nullptr;
- Result.documentation = getDocumentation(CCS);
+ Result.documentation = formatDocumentation(CCS, DocComment);
for (const auto &Chunk : CCS) {
switch (Chunk.Kind) {
@@ -810,7 +814,9 @@
Result.IncludeCodePatterns = EnableSnippets && IncludeCodePatterns;
Result.IncludeMacros = IncludeMacros;
Result.IncludeGlobals = true;
- Result.IncludeBriefComments = IncludeBriefComments;
+ // We never include brief comments, they are slow and don't provide useful
+ // results for non-doxygen comments.
+ Result.IncludeBriefComments = false;
// When an is used, Sema is responsible for completing the main file,
// the index can provide results from the preamble.
@@ -1002,9 +1008,15 @@
CompletionItem toCompletionItem(const CompletionCandidate &Candidate,
const CompletionItemScores &Scores) {
CodeCompletionString *SemaCCS = nullptr;
- if (auto *SR = Candidate.SemaResult)
- SemaCCS = Recorder->codeCompletionString(*SR, Opts.IncludeBriefComments);
- return Candidate.build(FileName, Scores, Opts, SemaCCS);
+ std::string DocComment;
+ if (auto *SR = Candidate.SemaResult) {
+ SemaCCS = Recorder->codeCompletionString(*SR);
+ if (Opts.IncludeComments) {
+ assert(Recorder->CCSema);
+ DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR);
+ }
+ }
+ return Candidate.build(FileName, Scores, Opts, SemaCCS, DocComment);
}
};
@@ -1030,7 +1042,7 @@
Options.IncludeGlobals = false;
Options.IncludeMacros = false;
Options.IncludeCodePatterns = false;
- Options.IncludeBriefComments = true;
+ Options.IncludeBriefComments = false;
semaCodeComplete(llvm::make_unique<SignatureHelpCollector>(Options, Result),
Options,
{FileName, Command, Preamble, Contents, Pos, std::move(VFS),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
