gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang/tools/libclang/CXString.cpp:85 + if (String.empty()) + return createEmpty(); + ---------------- egorzhdan wrote: > gribozavr2 wrote: > > Please split this change into a separate patch and add a unit test. > > > Do you know a good way to add a unit-test for this? This function only gets > called from within libclang, it isn't exposed to clients of libclang, > including libclangTest. So I think a unit test would probably need to invoke > the same CXComment API that `c-index-test` invokes while running > `comment-to-html-xml-conversion.cpp`. Would that be worth it? It is unfortunate that the headers for CXString are not visible to llvm-project/clang/unittests/libclang/LibclangTest.cpp. It wouldn't make sense to use the CXComment API because that's exactly what comment-to-html-xml-conversion.cpp does. A benefit of the unit test would be clear, direct testing of CXString. Anyhow, please split the CXString change to a separate patch (without tests, unfortunately). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132932/new/ https://reviews.llvm.org/D132932 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
