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

Julie Hockett via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 28 13:05:52 PDT 2019


juliehockett added inline comments.


================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:366
+
+  if (I->Namespace[0].RefType == InfoType::IT_namespace) {
+    auto Parent = llvm::make_unique<NamespaceInfo>();
----------------
Can you make this a switch statement, doing the appropriate things for `IT_record` and `IT_namespace` and using `llvm_unreachable("Invalid reference type")` for all other possible values? This will help prevent silent regressions down the line.


================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:374
+
+  // At this point any parent will be a RecordInfo
+  auto Parent = llvm::make_unique<RecordInfo>();
----------------
nit: generally try to make comments full sentences


================
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)};
+  // Info es wrapped in its parent scope so it's returned in the second position
+  return {nullptr, std::unique_ptr<Info>{std::move(I)}};
----------------
nit: spelling, here and below


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

https://reviews.llvm.org/D63911





More information about the cfe-commits mailing list