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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to