[clang-doc] Read docstrings for record members

[brettw]

Summary

Struct/class data members did not have the comments associated with them. This adds that information to the MemberTypeInfo class and emits it in the YAML. This does not update the frontends yet.

[phosek]

Would it be possible to extend the existing unit tests to cover this:

https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

When uploading patches through web interface, it's useful to include full context, see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

[paulkirth]

I agree w/ @phosek on unit testing.

Additionally, while we don't have many of them in clang-doc, an end-to-end test would also be welcome. In this case, it will also probably help w/ review, since we can see how clang-doc output is changing as part of the review process. To be clear: I'm not going to block acceptance on the end-to-end tests, since that may be asking too much. It would just be helpful.

I've left more direct feedback inline.

clang-tools-extra/clang-doc/BitcodeReader.cpp

354

So, I see that this uses the same pattern as in other getCommentInfo(...) API's, but I'm a bit confused by this.

Do we always grow the vector whenever getCommentInfo(...) is called? I would expect a get... function to be const and just return data. This grows the Description vector on every call, which seems like an odd choice. The pattern is also pervasive in BitcodeReader.cpp.

@phosek is this intentional? If Description exceeds capacity and reallocs on emplace_back, then any reference it had returned would be invalidated, right? Or am I missing something here re: the various *TypeInfos and how clang-doc uses them?

clang-tools-extra/clang-doc/Serialize.cpp

439

I think we need to at least assert that D != nullptr. While there is only one caller now, its likely there may be more in the future. Plus nullptr is deref is never fun to debug.

If getting a nullptr is expected to be a common input, then an early return is also fine.

If you prefer, a few places in the codebase do both to maintain correct behavior when asserts are disabled. Often they have a form along these lines:

if(!D){
  assert(false && "Some error message");
  return;
}

Any of these would be fine, and I don't have a preference between them.

440

If the goal is to get the FullComment, then doesn't getCommentForDecl(...) handle that? This also seems like an area where we'd want to use caching if we could.

clang-tools-extra/clang-doc/YAMLGenerator.cpp

195

Can we get a comment describing this?

[brettw]

Added some tests (with one open question).

[brettw]

clang-tools-extra/clang-doc/BitcodeReader.cpp

354

You are correct that this always grows. These functions are only called once and that's when readSubBlock gets a block ID. The comment is filled into the returned array.

My question is why the comments on the info structs are arrays. I have only ever seen one top-level comment of type "FullComment" which can have many children.

clang-tools-extra/clang-doc/Serialize.cpp

439

I'm fine adding an assert to unblock the review.

But clang-doc is full of pointers and none of them are asserted on. Is this one special in some way? To me, it seems weird to assert on this one random pointer. Is there some policy about this?

I follow the opposite theory that asserts on null pointers all over the place clutter the code and random null parameters are one of the easiest types of crashes to debug.

440

I have no idea how this works, I copied this snipped from MapASTVisitor::getComment in Mapper.cpp

clang-tools-extra/clang-doc/YAMLGenerator.cpp

195

I don't think this needs a comment, a substantial part of this file is just adding these mappings between YAML keys and field names. The one above it has a comment only because of the non-obvious default value. If you feel strongly, please let me know what the comment should say.

clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp

291

Whoops, I'll delete this.

clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

169

I spent quite some time trying to get this to work and I'm stuck. If I copy the contents of the raw string above into a file and run clang-doc on it, I get a comment in the YAML output. But in this test I never get a comment and I don't know how this parsing environment is different than the "real" one.

I'm very confused by the result of ExtractInfosFromCode function in the first place. What are the 1st and 6th item which are never checked? Why does it emit two duplicate records for the E struct, one (Infos[2]) with a data member and no function and one (Infos[3]) with no data member and a function in it?

Any suggestions here would be appreciated.

[paulkirth]

clang-tools-extra/clang-doc/Serialize.cpp

439

There's nothing special regarding this particular pointer, but such checks are common in other parts of LLVM.

We use asserts liberally throughout the LLVM codebase (https://llvm.org/docs/CodingStandards.html#assert-liberally) and asserting that a pointer is valid is common.

The other thing to keep in mind is that debug builds of LLVM are > 20GB, and enabling asserts is a more desirable choice for most of our developers. That probably isn't as bad for clang-doc, but I'd rather err on the side of caution.

While I agree that right now it may be a bit strange to have a single assert, hopefully overtime the surrounding code will start to use such checks in a way that is more consistent with the rest of the codebase.

But like I said, an early return is also fine IMO.

clang-tools-extra/clang-doc/YAMLGenerator.cpp

195

Now that I see this in context, I agree that it doesn't need a comment.

clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

201

?

[brettw]

clang-tools-extra/clang-doc/Serialize.cpp

439

Done

[brettw]

I updated the comment test that's not currently working with a TODO.

There are some open questions in the code review about some bigger questions about this tool. Since this has been virtually abandoned for several years, I don't think anybody knows these answers and we're not going to figure out everything for this patch.

I think this patch makes things incrementally better and I'd like to land something soon so I can work on the next missing feature. I think over time we can build up knowledge about this tool and make it better on a wider scale. Can you please talk another look?

[paulkirth]

LGTM, modulo the small fixes I've left inline

clang-tools-extra/clang-doc/Serialize.cpp

439

Can you add a String message to the assert?

This is a fairly standard example of the usage w/in LLVM:

assert(LCS && RCS && "Expect non-null FunctionSamples");

Taken from https://github.com/llvm/llvm-project/blob/30bbb73bb448910f791088bfc3154e752d42241a/llvm/lib/Transforms/IPO/SampleProfile.cpp#L400

440

That's fine, but lets add a TODO here to consider the other API, so we track this.

clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

169

So I also wanted to understand why. MakeOneLinecCmmentIInfo() is not used elsewhere in clang-doc, and I don't think it was ever tested.

Looking at the body it doesn't use it's input parameter at all.

There may also be an issue w/ how it constructs the CommentInfo.

Leaving this as a TODO is fine. I don't think this is something we're going to solve in this patch, but if you can also add a TODO to MakeOneLine CommentInfo, it would be appreciated.

[brettw]

New patch up.

[paulkirth]

Thanks. Can we get the small changes to the assert + the TODO in Serialize.cpp? I'll be happy to land this for you after that.

[brettw]

clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

169

The problem isn't with MakeOneLineCommentInfo (I fixed the parameter usage) but rather that the records generated in the test has no comments at all. The current test code (with the line commented out) expects that the comment is completely missing, which is the only way the test will pass now.

Since nobody else uses it, I commented it out and referenced the calling code.

[brettw]

New patch up with requested changes.

clang-tools-extra/clang-doc/BitcodeReader.cpp

template <> llvm::Expected<CommentInfo *> getCommentInfo(NamespaceInfo *I) {
   return &I->Description.back();
 }
 
 template <> llvm::Expected<CommentInfo *> getCommentInfo(RecordInfo *I) {
   I->Description.emplace_back();
   return &I->Description.back();
 }
 
+template <> llvm::Expected<CommentInfo *> getCommentInfo(MemberTypeInfo *I) {
+  I->Description.emplace_back();
+  return &I->Description.back();
+}
+
 template <> llvm::Expected<CommentInfo *> getCommentInfo(EnumInfo *I) {
   I->Description.emplace_back();
   return &I->Description.back();
 }
 
 template <> llvm::Expected<CommentInfo *> getCommentInfo(CommentInfo *I) {
   I->Children.emplace_back(std::make_unique<CommentInfo>());
   return I->Children.back().get();

clang-tools-extra/clang-doc/BitcodeWriter.cpp

void ClangDocBitcodeWriter::emitBlock(const FieldTypeInfo &T) {
   emitRecord(T.Name, FIELD_TYPE_NAME);
 }
 
 void ClangDocBitcodeWriter::emitBlock(const MemberTypeInfo &T) {
   StreamSubBlockGuard Block(Stream, BI_MEMBER_TYPE_BLOCK_ID);
   emitBlock(T.Type, FieldId::F_type);
   emitRecord(T.Name, MEMBER_TYPE_NAME);
   emitRecord(T.Access, MEMBER_TYPE_ACCESS);
+  for (const auto &CI : T.Description)
+    emitBlock(CI);
 }
 
 void ClangDocBitcodeWriter::emitBlock(const CommentInfo &I) {
   StreamSubBlockGuard Block(Stream, BI_COMMENT_BLOCK_ID);
   for (const auto &L : std::vector<std::pair<llvm::StringRef, RecordId>>{
            {I.Kind, COMMENT_KIND},
            {I.Text, COMMENT_TEXT},
            {I.Name, COMMENT_NAME},

clang-tools-extra/clang-doc/Representation.h

struct MemberTypeInfo : public FieldTypeInfo {
   MemberTypeInfo(llvm::StringRef RefName, llvm::StringRef Name,
                  AccessSpecifier Access)
       : FieldTypeInfo(RefName, Name), Access(Access) {}
   MemberTypeInfo(llvm::StringRef RefName, StringRef Path, llvm::StringRef Name,
                  AccessSpecifier Access)
       : FieldTypeInfo(RefName, Path, Name), Access(Access) {}
 
   bool operator==(const MemberTypeInfo &Other) const {
-    return std::tie(Type, Name, Access) ==
-           std::tie(Other.Type, Other.Name, Other.Access);
+    return std::tie(Type, Name, Access, Description) ==
+           std::tie(Other.Type, Other.Name, Other.Access, Other.Description);
   }
 
   // Access level associated with this info (public, protected, private, none).
   // AS_public is set as default because the bitcode writer requires the enum
   // with value 0 to be used as the default.
   // (AS_public = 0, AS_protected = 1, AS_private = 2, AS_none = 3)
   AccessSpecifier Access = AccessSpecifier::AS_public;
+
+  std::vector<CommentInfo> Description; // Comment description of this field.
 };
 
 struct Location {
   Location() = default;
   Location(int LineNumber, SmallString<16> Filename)
       : LineNumber(LineNumber), Filename(std::move(Filename)) {}
   Location(int LineNumber, SmallString<16> Filename, bool IsFileInRootDir)
       : LineNumber(LineNumber), Filename(std::move(Filename)),

clang-tools-extra/clang-doc/Serialize.cpp

SymbolID hashUSR(llvm::StringRef USR) {
   return llvm::SHA1::hash(arrayRefFromStringRef(USR));
 }
 
 template <typename T>
 static void
 populateParentNamespaces(llvm::SmallVector<Reference, 4> &Namespaces,
                          const T *D, bool &IsAnonymousNamespace);
 
+static void populateMemberTypeInfo(MemberTypeInfo &I, const FieldDecl *D);
+
 // A function to extract the appropriate relative path for a given info's
 // documentation. The path returned is a composite of the parent namespaces.
 //
 // Example: Given the below, the directory path for class C info will be
 // <root>/A/B
 //
 // namespace A {
 // namespace B {
if (const auto *T = getDeclForType(F->getTypeSourceInfo()->getType())) {
       } else if (const auto *N = dyn_cast<RecordDecl>(T)) {
         I.Members.emplace_back(
             getUSRForDecl(T), N->getNameAsString(), InfoType::IT_record,
             getInfoRelativePath(N), F->getNameAsString(),
             getFinalAccessSpecifier(Access, N->getAccessUnsafe()));
         continue;
       }
     }
-    I.Members.emplace_back(
+
+    auto& member = I.Members.emplace_back(
         F->getTypeSourceInfo()->getType().getAsString(), F->getNameAsString(),
         getFinalAccessSpecifier(Access, F->getAccessUnsafe()));
+    populateMemberTypeInfo(member, F);
   }
 }
 
 static void parseEnumerators(EnumInfo &I, const EnumDecl *D) {
   for (const EnumConstantDecl *E : D->enumerators())
     I.Members.emplace_back(E->getNameAsString());
 }
 
else if (isa<RecordDecl>(T))
       I.ReturnType = TypeInfo(getUSRForDecl(T), T->getNameAsString(),
                               InfoType::IT_record, getInfoRelativePath(T));
   } else {
     I.ReturnType = TypeInfo(D->getReturnType().getAsString());
   }
   parseParameters(I, D);
 }
 
+static void populateMemberTypeInfo(MemberTypeInfo &I, const FieldDecl *D) {
+  assert(D);
+  ASTContext& Context = D->getASTContext();
+  RawComment *Comment = Context.getRawCommentForDeclNoCache(D);
+  if (!Comment)
+    return;
+
+  Comment->setAttached();
+  if (comments::FullComment* fc = Comment->parse(Context, nullptr, D)) {
+    I.Description.emplace_back();
+    parseFullComment(fc, I.Description.back());
+  }
+}
+
 static void
 parseBases(RecordInfo &I, const CXXRecordDecl *D, bool IsFileInRootDir,
            bool PublicOnly, bool IsParent,
            AccessSpecifier ParentAccess = AccessSpecifier::AS_public) {
   // Don't parse bases if this isn't a definition.
   if (!D->isThisDeclarationADefinition())
     return;
   for (const CXXBaseSpecifier &B : D->bases()) {

clang-tools-extra/clang-doc/YAMLGenerator.cpp

 
 template <> struct MappingTraits<MemberTypeInfo> {
   static void mapping(IO &IO, MemberTypeInfo &I) {
     FieldTypeInfoMapping(IO, I);
     // clang::AccessSpecifier::AS_none is used as the default here because it's
     // the AS that shouldn't be part of the output. Even though AS_public is the
     // default in the struct, it should be displayed in the YAML output.
     IO.mapOptional("Access", I.Access, clang::AccessSpecifier::AS_none);
+    IO.mapOptional("Description", I.Description);
   }
 };
 
 template <> struct MappingTraits<NamespaceInfo> {
   static void mapping(IO &IO, NamespaceInfo &I) {
     InfoMapping(IO, I);
     IO.mapOptional("ChildNamespaces", I.ChildNamespaces,
                    std::vector<Reference>());

clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp

TEST(BitcodeTest, emitRecordInfoBitcode) {
   I.IsTypeDef = true;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
                        AccessSpecifier::AS_public, true);
   I.Bases.back().ChildFunctions.emplace_back();
   I.Bases.back().Members.emplace_back("int", "X", AccessSpecifier::AS_private);
   I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
 
+  // Documentation for the data member.
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique<CommentInfo>());
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+  Brief->Children.emplace_back(std::make_unique<CommentInfo>());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+  I.Bases.back().Members.back().Description.emplace_back(std::move(TopComment));
+
   I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
   I.ChildFunctions.emplace_back();
   I.ChildEnums.emplace_back();
 
   std::string WriteResult = writeInfo(&I);
   EXPECT_TRUE(WriteResult.size() > 0);
   std::vector<std::unique_ptr<Info>> ReadResults = readInfo(WriteResult, 1);
 
TEST(SerializeTest, emitInfoWithCommentBitcode) {
   std::string WriteResult = writeInfo(&F);
   EXPECT_TRUE(WriteResult.size() > 0);
   std::vector<std::unique_ptr<Info>> ReadResults = readInfo(WriteResult, 1);
 
   CheckFunctionInfo(&F, InfoAsFunction(ReadResults[0].get()));
 }
 
 } // namespace doc
 } // namespace clang

clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp

FunctionInfo *InfoAsFunction(Info *I) {
   return static_cast<FunctionInfo *>(I);
 }
 
 EnumInfo *InfoAsEnum(Info *I) {
   assert(I->IT == InfoType::IT_enum);
   return static_cast<EnumInfo *>(I);
 }
 
-void CheckCommentInfo(CommentInfo &Expected, CommentInfo &Actual) {
+void CheckCommentInfo(const std::vector<CommentInfo> &Expected,
+                      const std::vector<CommentInfo> &Actual);
+void CheckCommentInfo(const std::vector<std::unique_ptr<CommentInfo>> &Expected,
+                      const std::vector<std::unique_ptr<CommentInfo>> &Actual);
+
+void CheckCommentInfo(const CommentInfo &Expected, const CommentInfo &Actual) {
   EXPECT_EQ(Expected.Kind, Actual.Kind);
   EXPECT_EQ(Expected.Text, Actual.Text);
   EXPECT_EQ(Expected.Name, Actual.Name);
   EXPECT_EQ(Expected.Direction, Actual.Direction);
   EXPECT_EQ(Expected.ParamName, Actual.ParamName);
   EXPECT_EQ(Expected.CloseName, Actual.CloseName);
   EXPECT_EQ(Expected.SelfClosing, Actual.SelfClosing);
   EXPECT_EQ(Expected.Explicit, Actual.Explicit);
 
   ASSERT_EQ(Expected.AttrKeys.size(), Actual.AttrKeys.size());
   for (size_t Idx = 0; Idx < Actual.AttrKeys.size(); ++Idx)
     EXPECT_EQ(Expected.AttrKeys[Idx], Actual.AttrKeys[Idx]);
 
   ASSERT_EQ(Expected.AttrValues.size(), Actual.AttrValues.size());
   for (size_t Idx = 0; Idx < Actual.AttrValues.size(); ++Idx)
     EXPECT_EQ(Expected.AttrValues[Idx], Actual.AttrValues[Idx]);
 
   ASSERT_EQ(Expected.Args.size(), Actual.Args.size());
   for (size_t Idx = 0; Idx < Actual.Args.size(); ++Idx)
     EXPECT_EQ(Expected.Args[Idx], Actual.Args[Idx]);
 
-  ASSERT_EQ(Expected.Children.size(), Actual.Children.size());
-  for (size_t Idx = 0; Idx < Actual.Children.size(); ++Idx)
-    CheckCommentInfo(*Expected.Children[Idx], *Actual.Children[Idx]);
+  CheckCommentInfo(Expected.Children, Actual.Children);
+}
+
+void CheckCommentInfo(const std::vector<CommentInfo> &Expected,
+                      const std::vector<CommentInfo> &Actual) {
+  ASSERT_EQ(Expected.size(), Actual.size());
+  for (size_t Idx = 0; Idx < Actual.size(); ++Idx)
+    CheckCommentInfo(Expected[Idx], Actual[Idx]);
+}
+
+void CheckCommentInfo(const std::vector<std::unique_ptr<CommentInfo>> &Expected,
+                      const std::vector<std::unique_ptr<CommentInfo>> &Actual) {
+  ASSERT_EQ(Expected.size(), Actual.size());
+  for (size_t Idx = 0; Idx < Actual.size(); ++Idx)
+    CheckCommentInfo(*Expected[Idx], *Actual[Idx]);
 }
 
 void CheckReference(Reference &Expected, Reference &Actual) {
   EXPECT_EQ(Expected.Name, Actual.Name);
   EXPECT_EQ(Expected.RefType, Actual.RefType);
   EXPECT_EQ(Expected.Path, Actual.Path);
 }
 
 void CheckTypeInfo(TypeInfo *Expected, TypeInfo *Actual) {
   CheckReference(Expected->Type, Actual->Type);
 }
 
 void CheckFieldTypeInfo(FieldTypeInfo *Expected, FieldTypeInfo *Actual) {
   CheckTypeInfo(Expected, Actual);
   EXPECT_EQ(Expected->Name, Actual->Name);
 }
 
 void CheckMemberTypeInfo(MemberTypeInfo *Expected, MemberTypeInfo *Actual) {
   CheckFieldTypeInfo(Expected, Actual);
   EXPECT_EQ(Expected->Access, Actual->Access);
+  CheckCommentInfo(Expected->Description, Actual->Description);
 }
 
 void CheckBaseInfo(Info *Expected, Info *Actual) {
   EXPECT_EQ(size_t(20), Actual->USR.size());
   EXPECT_EQ(Expected->Name, Actual->Name);
   EXPECT_EQ(Expected->Path, Actual->Path);
   ASSERT_EQ(Expected->Namespace.size(), Actual->Namespace.size());
   for (size_t Idx = 0; Idx < Actual->Namespace.size(); ++Idx)
     CheckReference(Expected->Namespace[Idx], Actual->Namespace[Idx]);
-  ASSERT_EQ(Expected->Description.size(), Actual->Description.size());
-  for (size_t Idx = 0; Idx < Actual->Description.size(); ++Idx)
-    CheckCommentInfo(Expected->Description[Idx], Actual->Description[Idx]);
+  CheckCommentInfo(Expected->Description, Actual->Description);
 }
 
 void CheckSymbolInfo(SymbolInfo *Expected, SymbolInfo *Actual) {
   CheckBaseInfo(Expected, Actual);
   EXPECT_EQ(Expected->DefLoc.has_value(), Actual->DefLoc.has_value());
   if (Expected->DefLoc && Actual->DefLoc.has_value()) {
     EXPECT_EQ(Expected->DefLoc->LineNumber, Actual->DefLoc->LineNumber);
     EXPECT_EQ(Expected->DefLoc->Filename, Actual->DefLoc->Filename);

clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

void ExtractInfosFromCodeWithArgs(StringRef Code, size_t NumExpectedInfos,
                                   std::vector<std::string> &Args) {
   auto ASTUnit = clang::tooling::buildASTFromCodeWithArgs(Code, Args);
   auto TU = ASTUnit->getASTContext().getTranslationUnitDecl();
   ClangDocSerializeTestVisitor Visitor(EmittedInfos, Public);
   Visitor.TraverseTranslationUnitDecl(TU);
   ASSERT_EQ(NumExpectedInfos, EmittedInfos.size());
 }
 
+// Constructs a comment definition as the parser would for one comment line.
+/* TODO uncomment this when the missing comment is fixed in emitRecordInfo and
+   the code that calls this is re-enabled.
+CommentInfo MakeOneLineCommentInfo(const std::string &Text) {
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique<CommentInfo>());
+
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+
+  Brief->Children.emplace_back(std::make_unique<CommentInfo>());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = Text;
+
+  return TopComment;
+}
+*/
+
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
   ExtractInfosFromCode("namespace A { namespace B { void f() {} } }", 5,
                        /*Public=*/false, Infos);
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID, "A");

 }
 
 // Test serialization of record declarations.
 TEST(SerializeTest, emitRecordInfo) {
   EmittedInfoList Infos;
   ExtractInfosFromCode(R"raw(class E {
 public:
   E() {}
+
+  // Some docs.
+  int value;
 protected:
   void ProtectedMethod();
 };
 template <typename T>
 struct F {
   void TemplateMethod();
 };
 template <>
 void F<int>::TemplateMethod();
 typedef struct {} G;)raw",
                        10, /*Public=*/false, Infos);
 
   RecordInfo *E = InfoAsRecord(Infos[0].get());
   RecordInfo ExpectedE(EmptySID, /*Name=*/"E", /*Path=*/"GlobalNamespace");
   ExpectedE.Namespace.emplace_back(EmptySID, "GlobalNamespace",
                                    InfoType::IT_namespace);
   ExpectedE.TagType = TagTypeKind::TTK_Class;
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  ExpectedE.Members.emplace_back("int", "value", AccessSpecifier::AS_public);
+  // TODO the data member should have the docstring on it:
+  //ExpectedE.Members.back().Description.push_back(MakeOneLineCommentInfo(" Some docs"));
   CheckRecordInfo(&ExpectedE, E);
 
   RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[2].get());
   RecordInfo ExpectedRecordWithEConstructor(EmptySID);
   FunctionInfo EConstructor;
   EConstructor.Name = "E";
   EConstructor.Parent = Reference(EmptySID, "E", InfoType::IT_record);
   EConstructor.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
typedef struct {} G;)raw",
   Method.Parent = Reference(EmptySID, "E", InfoType::IT_record);
   Method.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   Method.Loc.emplace_back(0, llvm::SmallString<16>{"test.cpp"});
   Method.Namespace.emplace_back(EmptySID, "E", InfoType::IT_record);
   Method.Namespace.emplace_back(EmptySID, "GlobalNamespace",
                                 InfoType::IT_namespace);
   Method.Access = AccessSpecifier::AS_protected;
   Method.IsMethod = true;
   ExpectedRecordWithMethod.ChildFunctions.emplace_back(std::move(Method));
   CheckRecordInfo(&ExpectedRecordWithMethod, RecordWithMethod);
 
   RecordInfo *F = InfoAsRecord(Infos[4].get());
   RecordInfo ExpectedF(EmptySID, /*Name=*/"F", /*Path=*/"GlobalNamespace");
   ExpectedF.Namespace.emplace_back(EmptySID, "GlobalNamespace",
                                    InfoType::IT_namespace);
   ExpectedF.TagType = TagTypeKind::TTK_Struct;
   ExpectedF.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});

clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

TEST(YAMLGeneratorTest, emitRecordYAML) {
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   I.Members.emplace_back("int", "path/to/int", "X",
                          AccessSpecifier::AS_private);
+
+  // Member documentation.
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique<CommentInfo>());
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+  Brief->Children.emplace_back(std::make_unique<CommentInfo>());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+  I.Members.back().Description.push_back(std::move(TopComment));
+
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
                        AccessSpecifier::AS_public, true);
   I.Bases.back().ChildFunctions.emplace_back();
   I.Bases.back().ChildFunctions.back().Name = "InheritedFunctionOne";
   I.Bases.back().Members.emplace_back("int", "path/to/int", "N",
                                       AccessSpecifier::AS_private);
   // F is in the global namespace
- LineNumber:      12
     Filename:        'test.cpp'
 TagType:         Class
 Members:
   - Type:
       Name:            'int'
       Path:            'path/to/int'
     Name:            'X'
     Access:          Private
+    Description:
+      - Kind:            'FullComment'
+        Children:
+          - Kind:            'ParagraphComment'
+            Children:
+              - Kind:            'TextComment'
+                Text:            'Value of the thing.'
+                Name:            'ParagraphComment'
 Bases:
   - USR:             '0000000000000000000000000000000000000000'
     Name:            'F'
     Path:            'path/to/F'
     Members:
       - Type:
           Name:            'int'
           Path:            'path/to/int'