tom-anders created this revision.
Herald added subscribers: wenlei, kadircet, arphaman.
Herald added a project: All.
tom-anders added reviewers: nridge, sammccall, kadircet.
tom-anders published this revision for review.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
This stores SymbolDocumentation in the index and uses it in Hover.
CodeCompletion and SignatureHelp still use the unparsed comment for now.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D134132
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/index/Merge.cpp
clang-tools-extra/clangd/index/Serialization.cpp
clang-tools-extra/clangd/index/Symbol.h
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/YAMLSerialization.cpp
clang-tools-extra/clangd/test/index-serialization/Inputs/sample.cpp
clang-tools-extra/clangd/test/index-serialization/Inputs/sample.idx
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang-tools-extra/clangd/unittests/IndexTests.cpp
clang-tools-extra/clangd/unittests/SerializationTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -50,7 +50,7 @@
return (arg.Name + arg.Signature).str() == Label;
}
MATCHER_P(returnType, D, "") { return arg.ReturnType == D; }
-MATCHER_P(doc, D, "") { return arg.Documentation == D; }
+MATCHER_P(doc, D, "") { return arg.Documentation.CommentText == D; }
MATCHER_P(snippet, S, "") {
return (arg.Name + arg.CompletionSnippetSuffix).str() == S;
}
Index: clang-tools-extra/clangd/unittests/SerializationTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SerializationTests.cpp
+++ clang-tools-extra/clangd/unittests/SerializationTests.cpp
@@ -8,6 +8,7 @@
#include "Headers.h"
#include "RIFF.h"
+#include "SymbolDocumentationMatchers.h"
#include "index/Serialization.h"
#include "support/Logger.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -48,7 +49,22 @@
Line: 1
Column: 1
Flags: 129
-Documentation: 'Foo doc'
+Documentation:
+ Brief: 'Foo brief'
+ Returns: 'Foo returns'
+ Description: 'Foo description'
+ Notes:
+ - 'Foo note 1'
+ - 'Foo note 2'
+ Warnings:
+ - 'Foo warning 1'
+ - 'Foo warning 2'
+ Parameters:
+ - Name: 'param1'
+ Description: 'Foo param 1'
+ - Name: 'param2'
+ Description: 'Foo param 2'
+ CommentText: 'Full text would be here'
ReturnType: 'int'
IncludeHeaders:
- Header: 'include1'
@@ -141,7 +157,20 @@
EXPECT_THAT(Sym1, qName("clang::Foo1"));
EXPECT_EQ(Sym1.Signature, "");
- EXPECT_EQ(Sym1.Documentation, "Foo doc");
+
+ SymbolDocumentationRef ExpectedDocumentation;
+ ExpectedDocumentation.Brief = "Foo brief";
+ ExpectedDocumentation.Returns = "Foo returns";
+ ExpectedDocumentation.Description = "Foo description";
+ ExpectedDocumentation.Notes = {"Foo note 1", "Foo note 2"};
+ ExpectedDocumentation.Warnings = {"Foo warning 1", "Foo warning 2"};
+ ExpectedDocumentation.Parameters = {
+ {"param1", "Foo param 1"},
+ {"param2", "Foo param 2"},
+ };
+ ExpectedDocumentation.CommentText = "Full text would be here";
+ EXPECT_THAT(Sym1.Documentation, matchesDoc(ExpectedDocumentation));
+
EXPECT_EQ(Sym1.ReturnType, "int");
EXPECT_EQ(StringRef(Sym1.CanonicalDeclaration.FileURI), "file:///path/foo.h");
EXPECT_EQ(Sym1.Origin, SymbolOrigin::Static);
Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "Annotations.h"
+#include "SymbolDocumentationMatchers.h"
#include "SyncAPI.h"
#include "TestIndex.h"
#include "TestTU.h"
@@ -391,7 +392,7 @@
R.References = 2;
L.Signature = "()"; // present in left only
R.CompletionSnippetSuffix = "{$1:0}"; // present in right only
- R.Documentation = "--doc--";
+ R.Documentation = SymbolDocumentationRef::descriptionOnly("--doc--");
L.Origin = SymbolOrigin::Preamble;
R.Origin = SymbolOrigin::Static;
R.Type = "expectedType";
@@ -402,7 +403,8 @@
EXPECT_EQ(M.References, 3u);
EXPECT_EQ(M.Signature, "()");
EXPECT_EQ(M.CompletionSnippetSuffix, "{$1:0}");
- EXPECT_EQ(M.Documentation, "--doc--");
+ EXPECT_THAT(M.Documentation,
+ matchesDoc(SymbolDocumentationRef::descriptionOnly("--doc--")));
EXPECT_EQ(M.Type, "expectedType");
EXPECT_EQ(M.Origin, SymbolOrigin::Preamble | SymbolOrigin::Static |
SymbolOrigin::Merge);
@@ -546,16 +548,18 @@
Symbol L, R;
L.ID = R.ID = SymbolID("x");
L.Definition.FileURI = "file:/x.h";
- R.Documentation = "Forward declarations because x.h is too big to include";
+ R.Documentation = SymbolDocumentationRef::descriptionOnly(
+ "Forward declarations because x.h is too big to include");
for (auto ClassLikeKind :
{SymbolKind::Class, SymbolKind::Struct, SymbolKind::Union}) {
L.SymInfo.Kind = ClassLikeKind;
- EXPECT_EQ(mergeSymbol(L, R).Documentation, "");
+ ASSERT_TRUE(mergeSymbol(L, R).Documentation.empty());
}
L.SymInfo.Kind = SymbolKind::Function;
- R.Documentation = "Documentation from non-class symbols should be included";
- EXPECT_EQ(mergeSymbol(L, R).Documentation, R.Documentation);
+ R.Documentation = SymbolDocumentationRef::descriptionOnly(
+ "Documentation from non-class symbols should be included");
+ EXPECT_THAT(mergeSymbol(L, R).Documentation, matchesDoc(R.Documentation));
}
MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2770,7 +2770,8 @@
// Create a tiny index, so tests above can verify documentation is fetched.
Symbol IndexSym = func("indexSymbol");
- IndexSym.Documentation = "comment from index";
+ IndexSym.Documentation =
+ SymbolDocumentationRef::descriptionOnly("comment from index");
SymbolSlab::Builder Symbols;
Symbols.insert(IndexSym);
auto Index =
@@ -2827,7 +2828,8 @@
auto AST = TU.build();
Symbol IndexSym;
IndexSym.ID = getSymbolID(&findDecl(AST, "X"));
- IndexSym.Documentation = "comment from index";
+ IndexSym.Documentation =
+ SymbolDocumentationRef::descriptionOnly("comment from index");
SymbolSlab::Builder Symbols;
Symbols.insert(IndexSym);
auto Index =
@@ -2836,9 +2838,7 @@
for (const auto &P : T.points()) {
auto H = getHover(AST, P, format::getLLVMStyle(), Index.get());
ASSERT_TRUE(H);
- ASSERT_THAT(H->Documentation,
- matchesDoc(SymbolDocumentationOwned::descriptionOnly(
- std::string(IndexSym.Documentation))));
+ ASSERT_THAT(H->Documentation.toRef(), matchesDoc(IndexSym.Documentation));
}
}
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2335,9 +2335,9 @@
TEST(SignatureHelpTest, IndexDocumentation) {
Symbol Foo0 = sym("foo", index::SymbolKind::Function, "@F@\\0#");
- Foo0.Documentation = "doc from the index";
+ Foo0.Documentation.CommentText = "doc from the index";
Symbol Foo1 = sym("foo", index::SymbolKind::Function, "@F@\\0#I#");
- Foo1.Documentation = "doc from the index";
+ Foo1.Documentation.CommentText = "doc from the index";
Symbol Foo2 = sym("foo", index::SymbolKind::Function, "@F@\\0#I#I#");
StringRef Sig0 = R"cpp(
Index: clang-tools-extra/clangd/test/index-serialization/Inputs/sample.cpp
===================================================================
--- clang-tools-extra/clangd/test/index-serialization/Inputs/sample.cpp
+++ clang-tools-extra/clangd/test/index-serialization/Inputs/sample.cpp
@@ -3,6 +3,11 @@
// This introduces a symbol, a reference and a relation.
struct Bar : public Foo {
- // This introduces an OverriddenBy relation by implementing Foo::Func.
+ /// \brief This introduces an OverriddenBy relation by implementing Foo::Func.
+ /// \details And it also introduces some doxygen!
+ /// \param foo bar
+ /// \warning !!!
+ /// \note a note
+ /// \return nothing
void Func() override {}
};
Index: clang-tools-extra/clangd/index/YAMLSerialization.cpp
===================================================================
--- clang-tools-extra/clangd/index/YAMLSerialization.cpp
+++ clang-tools-extra/clangd/index/YAMLSerialization.cpp
@@ -30,6 +30,7 @@
LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::Symbol::IncludeHeaderWithReferences)
LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::Ref)
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::ParameterDocumentationRef)
namespace {
using RefBundle =
@@ -59,11 +60,13 @@
using clang::clangd::FileDigest;
using clang::clangd::IncludeGraph;
using clang::clangd::IncludeGraphNode;
+using clang::clangd::ParameterDocumentationRef;
using clang::clangd::Ref;
using clang::clangd::RefKind;
using clang::clangd::Relation;
using clang::clangd::RelationKind;
using clang::clangd::Symbol;
+using clang::clangd::SymbolDocumentationRef;
using clang::clangd::SymbolID;
using clang::clangd::SymbolLocation;
using clang::index::SymbolInfo;
@@ -174,6 +177,28 @@
}
};
+template <> struct MappingTraits<ParameterDocumentationRef> {
+ static void mapping(IO &IO, ParameterDocumentationRef &P) {
+ IO.mapRequired("Name", P.Name);
+ IO.mapRequired("Description", P.Description);
+ }
+};
+
+template <> struct MappingTraits<SymbolDocumentationRef> {
+ static void mapping(IO &IO, SymbolDocumentationRef &Doc) {
+ IO.mapOptional("Brief", Doc.Brief);
+ IO.mapOptional("Returns", Doc.Returns);
+
+ IO.mapOptional("Notes", Doc.Notes);
+ IO.mapOptional("Warnings", Doc.Warnings);
+
+ IO.mapOptional("Parameters", Doc.Parameters);
+
+ IO.mapOptional("Description", Doc.Description);
+ IO.mapOptional("CommentText", Doc.CommentText);
+ }
+};
+
template <> struct MappingTraits<Symbol> {
static void mapping(IO &IO, Symbol &Sym) {
MappingNormalization<NormalizedSymbolID, SymbolID> NSymbolID(IO, Sym.ID);
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -912,17 +912,17 @@
*ASTCtx, *PP, CodeCompletionContext::CCC_Symbol, *CompletionAllocator,
*CompletionTUInfo,
/*IncludeBriefComments*/ false);
- std::string Documentation =
- formatDocumentation(*CCS, getDocumentation(Ctx, SymbolCompletion,
- /*CommentsFromHeaders=*/true)
- .CommentText);
+
+ const SymbolDocumentationOwned Documentation =
+ getDocumentation(Ctx, SymbolCompletion, /*CommentsFromHeaders=*/true);
+
if (!(S.Flags & Symbol::IndexedForCodeCompletion)) {
if (Opts.StoreAllDocumentation)
- S.Documentation = Documentation;
+ S.Documentation = Documentation.toRef();
Symbols.insert(S);
return Symbols.find(S.ID);
}
- S.Documentation = Documentation;
+ S.Documentation = Documentation.toRef();
std::string Signature;
std::string SnippetSuffix;
getSignature(*CCS, &Signature, &SnippetSuffix);
Index: clang-tools-extra/clangd/index/Symbol.h
===================================================================
--- clang-tools-extra/clangd/index/Symbol.h
+++ clang-tools-extra/clangd/index/Symbol.h
@@ -9,6 +9,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_H
+#include "SymbolDocumentation.h"
#include "index/SymbolID.h"
#include "index/SymbolLocation.h"
#include "index/SymbolOrigin.h"
@@ -73,7 +74,7 @@
/// Only set when the symbol is indexed for completion.
llvm::StringRef CompletionSnippetSuffix;
/// Documentation including comment for the symbol declaration.
- llvm::StringRef Documentation;
+ SymbolDocumentationRef Documentation;
/// Type when this symbol is used in an expression. (Short display form).
/// e.g. return type of a function, or type of a variable.
/// Only set when the symbol is indexed for completion.
@@ -150,7 +151,20 @@
CB(S.TemplateSpecializationArgs);
CB(S.Signature);
CB(S.CompletionSnippetSuffix);
- CB(S.Documentation);
+
+ CB(S.Documentation.Brief);
+ CB(S.Documentation.Returns);
+ for (auto &Note : S.Documentation.Notes)
+ CB(Note);
+ for (auto &Warning : S.Documentation.Warnings)
+ CB(Warning);
+ for (auto &ParamDoc : S.Documentation.Parameters) {
+ CB(ParamDoc.Name);
+ CB(ParamDoc.Description);
+ }
+ CB(S.Documentation.Description);
+ CB(S.Documentation.CommentText);
+
CB(S.ReturnType);
CB(S.Type);
auto RawCharPointerCB = [&CB](const char *&P) {
Index: clang-tools-extra/clangd/index/Serialization.cpp
===================================================================
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serialization.cpp
@@ -283,6 +283,57 @@
return Loc;
}
+void writeSymbolDocumentation(const SymbolDocumentationRef &Doc,
+ const StringTableOut &Strings,
+ llvm::raw_ostream &OS) {
+ writeVar(Strings.index(Doc.Brief), OS);
+ writeVar(Strings.index(Doc.Returns), OS);
+
+ writeVar(Doc.Notes.size(), OS);
+ for (const auto &Note : Doc.Notes)
+ writeVar(Strings.index(Note), OS);
+
+ writeVar(Doc.Warnings.size(), OS);
+ for (const auto &Warning : Doc.Warnings)
+ writeVar(Strings.index(Warning), OS);
+
+ writeVar(Doc.Parameters.size(), OS);
+ for (const auto &ParamDoc : Doc.Parameters) {
+ writeVar(Strings.index(ParamDoc.Name), OS);
+ writeVar(Strings.index(ParamDoc.Description), OS);
+ }
+
+ writeVar(Strings.index(Doc.Description), OS);
+ writeVar(Strings.index(Doc.CommentText), OS);
+}
+
+SymbolDocumentationRef
+readSymbolDocumentation(Reader &Data, llvm::ArrayRef<llvm::StringRef> Strings) {
+ SymbolDocumentationRef Doc;
+ Doc.Brief = Data.consumeString(Strings);
+ Doc.Returns = Data.consumeString(Strings);
+
+ if (!Data.consumeSize(Doc.Notes))
+ return Doc;
+ for (auto &Note : Doc.Notes)
+ Note = Data.consumeString(Strings);
+
+ if (!Data.consumeSize(Doc.Warnings))
+ return Doc;
+ for (auto &Warning : Doc.Warnings)
+ Warning = Data.consumeString(Strings);
+
+ if (!Data.consumeSize(Doc.Parameters))
+ return Doc;
+ for (auto &ParamDoc : Doc.Parameters)
+ ParamDoc = {Data.consumeString(Strings), Data.consumeString(Strings)};
+
+ Doc.Description = Data.consumeString(Strings);
+ Doc.CommentText = Data.consumeString(Strings);
+
+ return Doc;
+}
+
IncludeGraphNode readIncludeGraphNode(Reader &Data,
llvm::ArrayRef<llvm::StringRef> Strings) {
IncludeGraphNode IGN;
@@ -325,7 +376,7 @@
OS.write(static_cast<uint8_t>(Sym.Flags));
writeVar(Strings.index(Sym.Signature), OS);
writeVar(Strings.index(Sym.CompletionSnippetSuffix), OS);
- writeVar(Strings.index(Sym.Documentation), OS);
+ writeSymbolDocumentation(Sym.Documentation, Strings, OS);
writeVar(Strings.index(Sym.ReturnType), OS);
writeVar(Strings.index(Sym.Type), OS);
@@ -354,7 +405,7 @@
Sym.Origin = Origin;
Sym.Signature = Data.consumeString(Strings);
Sym.CompletionSnippetSuffix = Data.consumeString(Strings);
- Sym.Documentation = Data.consumeString(Strings);
+ Sym.Documentation = readSymbolDocumentation(Data, Strings);
Sym.ReturnType = Data.consumeString(Strings);
Sym.Type = Data.consumeString(Strings);
if (!Data.consumeSize(Sym.IncludeHeaders))
@@ -455,7 +506,7 @@
// The current versioning scheme is simple - non-current versions are rejected.
// If you make a breaking change, bump this version number to invalidate stored
// data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 17;
+constexpr static uint32_t Version = 18;
llvm::Expected<IndexFileIn> readRIFF(llvm::StringRef Data,
SymbolOrigin Origin) {
Index: clang-tools-extra/clangd/index/Merge.cpp
===================================================================
--- clang-tools-extra/clangd/index/Merge.cpp
+++ clang-tools-extra/clangd/index/Merge.cpp
@@ -227,7 +227,7 @@
S.Signature = O.Signature;
if (S.CompletionSnippetSuffix == "")
S.CompletionSnippetSuffix = O.CompletionSnippetSuffix;
- if (S.Documentation == "") {
+ if (S.Documentation.empty()) {
// Don't accept documentation from bare forward class declarations, if there
// is a definition and it didn't provide one. S is often an undocumented
// class, and O is a non-canonical forward decl preceded by an irrelevant
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -333,8 +333,7 @@
LookupRequest Req;
Req.IDs.insert(ID);
Index->lookup(Req, [&](const Symbol &S) {
- Hover.Documentation =
- SymbolDocumentationOwned::descriptionOnly(std::string(S.Documentation));
+ Hover.Documentation = S.Documentation.toOwned();
});
}
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -425,12 +425,11 @@
}
};
if (C.IndexResult) {
- SetDoc(C.IndexResult->Documentation);
+ SetDoc(C.IndexResult->Documentation.CommentText);
} else if (C.SemaResult) {
const auto DocComment = getDocumentation(*ASTCtx, *C.SemaResult,
- /*CommentsFromHeaders=*/false)
- .CommentText;
- SetDoc(formatDocumentation(*SemaCCS, DocComment));
+ /*CommentsFromHeaders=*/false);
+ SetDoc(formatDocumentation(*SemaCCS, DocComment.CommentText));
}
}
if (Completion.Deprecated) {
@@ -985,8 +984,7 @@
Candidate.getFunction()
? getDeclDocumentation(S.getASTContext(),
*Candidate.getFunction())
- .CommentText
- : ""));
+ : SymbolDocumentationOwned{}));
}
// Sema does not load the docs from the preamble, so we need to fetch extra
@@ -1001,7 +999,7 @@
}
Index->lookup(IndexRequest, [&](const Symbol &S) {
if (!S.Documentation.empty())
- FetchedDocs[S.ID] = std::string(S.Documentation);
+ FetchedDocs[S.ID] = std::string(S.Documentation.CommentText);
});
vlog("SigHelp: requested docs for {0} symbols from the index, got {1} "
"symbols with non-empty docs in the response",
@@ -1110,15 +1108,17 @@
// FIXME(ioeric): consider moving CodeCompletionString logic here to
// CompletionString.h.
- ScoredSignature processOverloadCandidate(const OverloadCandidate &Candidate,
- const CodeCompletionString &CCS,
- llvm::StringRef DocComment) const {
+ ScoredSignature
+ processOverloadCandidate(const OverloadCandidate &Candidate,
+ const CodeCompletionString &CCS,
+ const SymbolDocumentationOwned &DocComment) const {
SignatureInformation Signature;
SignatureQualitySignals Signal;
const char *ReturnType = nullptr;
markup::Document OverloadComment;
- parseDocumentation(formatDocumentation(CCS, DocComment), OverloadComment);
+ parseDocumentation(formatDocumentation(CCS, DocComment.CommentText),
+ OverloadComment);
Signature.documentation = renderDoc(OverloadComment, DocumentationFormat);
Signal.Kind = Candidate.getKind();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
