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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
