[PATCH] D63663: [clang-doc] Add html links to references

Julie Hockett via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 9 09:26:08 PDT 2019


juliehockett added inline comments.


================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:206
+  llvm::SmallString<128> Path = Type.Path;
+  ::llvm::sys::path::append(Path, Type.Name + ".html");
+  return genLink(Type.Name, Path);
----------------
You shouldn't need the prefixed `::`


================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:466
   default:
     llvm_unreachable("Invalid reference type");
   }
----------------
While you're here, can you update this error to something like "Invalid reference type for parent namespace"?


================
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:567
+      ParentI->Namespace = llvm::SmallVector<Reference, 4>(
+          Enum.Namespace.begin() + 1, Enum.Namespace.end());
+      ParentI->Path = getInfoOutputFile(OutDirectory, ParentI->Namespace);
----------------
nit: use `++Enum.Namespace.begin()`


================
Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:219
     doc::Info *I = Reduced.get().get();
-
-    auto InfoPath = getInfoOutputFile(OutDirectory, I->Namespace,
-                                      I->extractName(), "." + Format);
+    auto InfoPath = getInfoOutputFile(I->Path, I->extractName(), "." + Format);
     if (!InfoPath) {
----------------
Could it add the OutDirectory here, and just store the relative path in `I->Path`? Since `OutDirectory` is constant throughout. You also wouldn't have to plumb it through all the serialize functions.


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

https://reviews.llvm.org/D63663





More information about the cfe-commits mailing list