[PATCH] D41102: Setup clang-doc frontend framework

[Athos via Phabricator via cfe-commits]

Athosvk added a comment.

The changes seem good (both mapper and additional changes)! I've added some comments, but those are primarily details.

What I'm however primarily interested in is the following:

1. You seemingly output only little information for declarations that are not definition. Is that temporary? Will you //need// to have a definition in order for a function be documented?
2. I've mentioned it before as a comment, but to what extent will you be parsing information in this frontend? Currently the links between types are primarily stored as strings. Are you planning to have the backend that generates the MarkDown parse those strings and link them to types? E.g. the parenttype is a std::vector<std::string> and assuming you want the markdown to have a link to this parent type, will the MarkDown have to parse this type and lookup the link to the original type? Or will you embed references to other types within the intermediate format?

I'm curious to hear your thoughts on this!



================
Comment at: clang-doc/ClangDocRepresentation.h:39
+  llvm::SmallVector<std::string, 4> Position;
+  std::vector<std::shared_ptr<CommentInfo>> Children;
+};
----------------
I might be missing something, but can't this be a unique ptr? Shouldn't children of comments only have one parent?


================
Comment at: clang-doc/ClangDocRepresentation.h:46
+struct NamedType {
+  enum FieldName { PARAM = 1, MEMBER, RETTYPE };
+  FieldName Field;
----------------
Perhaps use an enum class instead? Same goes for the other enums


================
Comment at: clang-doc/ClangDocRepresentation.h:63
+  std::string SimpleName;
+  std::string Namespace;
+  llvm::SmallVector<CommentInfo, 2> Description;
----------------
It's not too important for now , but you probably want to at least store the namespace identifier for each nested namespace at some point. So instead you store a vector of namespaces, which in the final markdown generation stage allows you to link to each namespace individually (assuming you'll have some kind of namespace overview pages)


================
Comment at: tools/clang-doc/ClangDocReporter.h:42
   std::string Name;
   AccessSpecifier Access;
 };
----------------
You might want to separate this out to a FieldType/MemberType or something alike, as only class members will have this set, while you also use this for parameters/return types etc. I know there's AS_NONE but it seems a little wasteful considering the amount of instances that will not have this set


https://reviews.llvm.org/D41102