[PATCH] D41102: Setup clang-doc frontend framework

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added inline comments.


================
Comment at: tools/clang-doc/ClangDocReporter.cpp:55
+    Docs.Namespaces[Name] = std::move(I);
+    populateBasicInfo(*Docs.Namespaces[Name], Name, D->getNameAsString(),
+                      getParentNamespace(D));
----------------
If you make a "populateNamespaceInfo" method that just calls populateBasicInfo but checks to see that the namespace hasn't already been added you can move this outside of this if statement which will make it more uniform with the other invocations. Also if you then specialize a general form of "populateInfo",  include a specialization for NamespaceInfo, and do a few more things in other methods I think most of these methods become identical (the Function stuff is still different).


================
Comment at: tools/clang-doc/ClangDocReporter.cpp:70
+  if (Pair == Docs.Types.end()) {
+    std::unique_ptr<TypeInfo> I = make_unique<TypeInfo>();
+    Docs.Types[Name] = std::move(I);
----------------
ditto


================
Comment at: tools/clang-doc/ClangDocReporter.cpp:74
+
+  if (D->isThisDeclarationADefinition())
+    populateTypeInfo(*Docs.Types[Name], D, Name, File);
----------------
Is this a check that populateTypeInfo could do instead? Or do we sometimes want to call populateTypeInfo on non-definitions?


================
Comment at: tools/clang-doc/ClangDocReporter.cpp:87
+  if (Pair == Docs.Enums.end()) {
+    std::unique_ptr<EnumInfo> I = make_unique<EnumInfo>();
+    Docs.Enums[Name] = std::move(I);
----------------
ditto


================
Comment at: tools/clang-doc/ClangDocReporter.cpp:91
+
+  if (D->isThisDeclarationADefinition())
+    populateEnumInfo(*Docs.Enums[Name], D, Name, File);
----------------
Same comment here as I had on createTypeInfo/populateTypeInfo


================
Comment at: tools/clang-doc/ClangDocReporter.cpp:118
+
+  if (D->isThisDeclarationADefinition())
+    populateFunctionInfo(*Docs.Namespaces[Namespace]->Functions[MangledName], D,
----------------
Is this something that can go inside populateFunctionInfo?


================
Comment at: tools/clang-doc/ClangDocReporter.h:166
+                         StringRef Namespace);
+  void populateTypeInfo(TypeInfo &I, const RecordDecl *D, StringRef Name,
+                        StringRef File);
----------------
sans the BasicInfo one I think you should use the same specialization trick here. After you do that the main difference between createInfo methods will be what collection they add too. That suggests to me that the collection the info is added to should be made a parameter to a method that does all the actual work.


https://reviews.llvm.org/D41102