kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land.
thanks, LGTM! ================ Comment at: clang-tools-extra/include-cleaner/test/nocrash.cpp:1 +// RUN: clang-include-cleaner %s -- + ---------------- hokein wrote: > I have considered to add a unittest in `FindHeadersTest`, but that is not > trivial to do, the unittest is written based on the assumption that the > captured symbol name is a simple identifier (we use the it to find the symbol > based on a given text name). > > Other ideas are welcome. > i think this LG, any place that needs to test these APIs requires handles to the decls. So we either need to have a test that does complicated AST traversals to reach a particular node or test things through an integration test, something like this or a unittest on top of walkUsed/analyze. I don't think former is worth for a crash test and having a unittest based on walkUsed/analyze for checking a crash inside headersForSymbol is more confusing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145364/new/ https://reviews.llvm.org/D145364 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits