[PATCH] D63911: [clang-doc] Serialize child namespaces and records

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.


================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:341
+  Parent->USR = ParentUSR;
+  Parent->ChildNamespaces.emplace_back(I->USR, I->Name, InfoType::IT_namespace);
+  return {std::unique_ptr<Info>{std::move(I)},
----------------
You're probably going to want the path in this too, here and below


================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:398
   I->ChildFunctions.emplace_back(std::move(Func));
-  return std::unique_ptr<Info>{std::move(I)};
+  return {std::unique_ptr<Info>{std::move(I)}, nullptr};
 }
----------------
Maybe return the parent in the second position, and then comment about how the info itself is wrapped in its parent scope? That would make more sense logically, and they all end up in the same place. Same for methods/enums.


================
Comment at: clang-tools-extra/clang-doc/Serialize.h:30
 
-std::unique_ptr<Info> emitInfo(const NamespaceDecl *D, const FullComment *FC,
-                               int LineNumber, StringRef File, bool PublicOnly);
-std::unique_ptr<Info> emitInfo(const RecordDecl *D, const FullComment *FC,
-                               int LineNumber, StringRef File, bool PublicOnly);
-std::unique_ptr<Info> emitInfo(const EnumDecl *D, const FullComment *FC,
-                               int LineNumber, StringRef File, bool PublicOnly);
-std::unique_ptr<Info> emitInfo(const FunctionDecl *D, const FullComment *FC,
-                               int LineNumber, StringRef File, bool PublicOnly);
-std::unique_ptr<Info> emitInfo(const CXXMethodDecl *D, const FullComment *FC,
-                               int LineNumber, StringRef File, bool PublicOnly);
+std::pair<std::unique_ptr<Info>, std::unique_ptr<Info>>
+emitInfo(const NamespaceDecl *D, const FullComment *FC, int LineNumber,
----------------
Comment here what each of these represents, e.g. the first info contains the relevant information about the declaration, the second records the parent.


================
Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:41-44
+    if (I.first)
+      EmittedInfos.emplace_back(std::move(I.first));
+    if (I.second)
+      EmittedInfos.emplace_back(std::move(I.second));
----------------
Extract this logic into a mapDecl function, since it's repeated so much


================
Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:362
+  NamespaceInfo *ParentA = InfoAsNamespace(Infos[1].get());
+  NamespaceInfo ExpectedParentA(EmptySID);
+  ExpectedParentA.ChildRecords.emplace_back(Infos[0]->USR, "A",
----------------
Just use the EmptySID -- the checking function ignores the USRs since they can be system-dependent. Here and below.


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

https://reviews.llvm.org/D63911