[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.


================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:21
 
 namespace {
 
----------------
Eugene.Zelenko wrote:
> Anonymous namespace is used for functions when LLVM Coding Guidelines tells using static for same purpose.
Actually, since these aren't used outside of this file, please use the `static` keyword instead of wrapping them in a namespace.


================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:58
+    if (!R.Path.empty()) {
+      Stream << genLink(R.Name, R.Path + "/" + R.Name + ".html");
+    } else {
----------------
Use llvm::sys::path to allow for other platforms that use a different separator.


================
Comment at: clang-tools-extra/clang-doc/MDGenerator.cpp:21
 
-namespace {
+namespace md_generator {
 
----------------
Same here, just use static.


================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:31
+// Example: Given the below, the <ext> path for class C will be <
+// root>/A/B/C
+//
----------------
<root>/A/B/C.<ext>


================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:230
       if (const auto *N = dyn_cast<EnumDecl>(T)) {
         I.Members.emplace_back(getUSRForDecl(T), N->getNameAsString(),
                                InfoType::IT_enum, F->getNameAsString(),
----------------
Also for members


================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:255
       if (const auto *N = dyn_cast<EnumDecl>(T)) {
         I.Params.emplace_back(getUSRForDecl(N), N->getNameAsString(),
                               InfoType::IT_enum, P->getNameAsString());
----------------
Also for param references, if applicable


================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:308-310
+    if (const auto *P = getDeclForType(B.getType()))
+      I.VirtualParents.emplace_back(getUSRForDecl(P), P->getNameAsString(),
+                                    InfoType::IT_record);
----------------
Can we do the path construction for virtual bases as well, so they can be linked too?


================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:341-350
   if (const auto *T = getDeclForType(D->getReturnType())) {
     if (dyn_cast<EnumDecl>(T))
       I.ReturnType =
           TypeInfo(getUSRForDecl(T), T->getNameAsString(), InfoType::IT_enum);
     else if (dyn_cast<RecordDecl>(T))
       I.ReturnType =
           TypeInfo(getUSRForDecl(T), T->getNameAsString(), InfoType::IT_record);
----------------
Add path information to these references as well


================
Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:145-146
   void ProtectedMethod();
-};)raw", 3, /*Public=*/false, Infos);
+};)raw",
+                       3, /*Public=*/false, Infos);
 
----------------
formatting change?


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

https://reviews.llvm.org/D63663