[PATCH] D145364: [include-cleaner] Fix a crash on non-identifier-name symbols.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 6 04:08:21 PST 2023


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



More information about the cfe-commits mailing list