paulkirth added a comment.
Thanks for the patch. This is mostly LGTM modulo a few nits.
My one question is regarding documentation. Do you think this needs to be
described in the clang-tools-extra/doc/clang-doc.rst? And are there any changes
in workflow or tool usage that we should document? I assume not, since this
just changes the YAML format. Is that correct?
================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:300
- auto& member = I.Members.emplace_back(
+ auto &member = I.Members.emplace_back(
F->getTypeSourceInfo()->getType().getAsString(), F->getNameAsString(),
----------------
nit: can we avoid introducing unrelated formatting changes, here and elsewhere
in the patch?
================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:324-329
+ } else {
+ FieldInfo = &I.Params.emplace_back(P->getOriginalType().getAsString(),
+ P->getNameAsString());
}
+ } else {
+ FieldInfo = &I.Params.emplace_back(P->getOriginalType().getAsString(),
----------------
can we combine these conditions somehow, given that they do the same thing?
Maybe this? WDYT?
```
if(!FieldInfo)
FieldInfo = &I.Params.emplace_back(P->getOriginalType().getAsString(), ...
```
================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:452
- ASTContext& Context = D->getASTContext();
+ ASTContext &Context = D->getASTContext();
// TODO investigate whether we can use ASTContext::getCommentForDecl instead
----------------
nit: same here
================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:460
Comment->setAttached();
- if (comments::FullComment* fc = Comment->parse(Context, nullptr, D)) {
+ if (comments::FullComment *fc = Comment->parse(Context, nullptr, D)) {
I.Description.emplace_back();
----------------
nit: ditto
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133732/new/
https://reviews.llvm.org/D133732
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
