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