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

Paul Kirth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 5 19:08:32 PST 2022


paulkirth added a comment.

Can we also update the end-to-end tests with some relevant test cases? I think that's especially important, given that I'm not sure the unittests are running as part of the normal test suite.

Overall this is mostly LGTM with some minor comments/questions.



================
Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:376
+                        TemplateSpecializationInfo *I) {
+  switch (ID) {
+  case TEMPLATE_SPECIALIZATION_OF:
----------------
Do you expect to add handling for more cases here? If so, a comment with FIXME or TODO is appropriate. If not, then I'm not sure a `switch` is a better choice than a branch ...


================
Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:631
 
+TEST(SerializeTests, emitFunctionTemplate) {
+  EmittedInfoList Infos;
----------------
I may have missed these cases in the test file, so if they exist just ignore me.

-  a template w/ no concrete types e.g., `template<typename T> void GetFoo(T);` This seems like a very common case, so it would be good to have a test explicitly for it. 
- a nested template, both where the inner template type is known, and where it is not.



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

https://reviews.llvm.org/D139154



More information about the cfe-commits mailing list