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

Reply via email to