[PATCH] D139154: [clang-doc] Add template support

[Paul Kirth via Phabricator via cfe-commits]

paulkirth accepted this revision.
paulkirth added a comment.
This revision is now accepted and ready to land.

This is basically LGTM. There are a few comments I think could be phrased a bit more clearly, but otherwise are more or less fine.



================
Comment at: clang-tools-extra/clang-doc/Representation.h:119-123
+  // This variant (that takes no qualified name parameter) uses the Name as the
+  // QualName (very useful in unit tests to reduce verbosity). This can't use
+  // the empty string as a default because the global namespace uses
+  // "GlobalNamespace" as the name, but should have an empty QualName
+  // (corresponding to the name in code).
----------------
Does this imply that its illegal to use `""` as the name with this constructor? if so, it probably requires at least an assert.


================
Comment at: clang-tools-extra/clang-doc/Representation.h:200-202
+// is quite involved. Currently the documentation uses of this data just involve
+// echoing it back to reconstruct the declaration of the class or function, so
+// the full contents is sufficient.
----------------
I found this comment a bit hard to parse. Do you think this is a good way to rephrase?


================
Comment at: clang-tools-extra/clang-doc/Representation.h:210-212
+  // The literal contents of the code for everything that specifies this
+  // template parameter. This will include all components, so for a template
+  // declaration it could be "typename T = int".
----------------
I think this is equivalent. Do you mean this contains //all// instantiations of the parameter across the codebase? or for a single declaration?


================
Comment at: clang-tools-extra/clang-doc/Representation.h:224-225
+
+// This struct will be a member of the record/function that is the template or
+// specialization.
+struct TemplateInfo {
----------------
Can you rephrase this? I think you're saying this type contains template information for a function or type that contains a template or template specialization. Is that correct?


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

https://reviews.llvm.org/D139154