[PATCH] D131298: [clang-doc] Read docstrings for record members

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

LGTM, modulo the small fixes I've left inline



================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:439
+static void populateMemberTypeInfo(MemberTypeInfo &I, const FieldDecl *D) {
+  assert(D);
+  ASTContext& Context = D->getASTContext();
----------------
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




================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:440
+  ASTContext& Context = D->getASTContext();
+  RawComment *Comment = Context.getRawCommentForDeclNoCache(D);
+  if (!Comment)
----------------
brettw wrote:
> paulkirth wrote:
> > 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.
> I have no idea how this works, I copied this snipped from MapASTVisitor::getComment in Mapper.cpp
That's fine, but lets add a TODO here to consider the other API, so we track this.


================
Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:166
+  ExpectedE.Members.emplace_back("int", "value", AccessSpecifier::AS_public);
+  //ExpectedE.Members.back().Description.push_back(MakeOneLineCommentInfo(" Some docs"));
   CheckRecordInfo(&ExpectedE, E);
----------------
brettw wrote:
> 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.
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131298/new/

https://reviews.llvm.org/D131298