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

[hokein]

clang-tools-extra/include-cleaner/test/nocrash.cpp

2

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.

[kadircet]

thanks, LGTM!

clang-tools-extra/include-cleaner/test/nocrash.cpp

2

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.

clang-tools-extra/include-cleaner/lib/FindHeaders.cpp

headersForSpecialSymbol(const Symbol &S, const SourceManager &SM,
                         const PragmaIncludes *PI) {
   if (S.kind() != Symbol::Declaration || !S.declaration().isInStdNamespace())
     return {};
 
   const auto *FD = S.declaration().getAsFunction();
   if (!FD)
     return {};
 
-  llvm::StringRef FName = FD->getName();
+  llvm::StringRef FName = symbolName(S);
   llvm::SmallVector<tooling::stdlib::Header> Headers;
   if (FName == "move") {
     if (FD->getNumParams() == 1)
       // move(T&& t)
       Headers.push_back(*tooling::stdlib::Header::named("<utility>"));
     if (FD->getNumParams() == 3)
       // move(InputIt first, InputIt last, OutputIt dest);
       Headers.push_back(*tooling::stdlib::Header::named("<algorithm>"));

clang-tools-extra/include-cleaner/test/nocrash.cpp

+// RUN: clang-include-cleaner %s --
+
+namespace std {
+class Foo {};
+bool operator==(Foo, int) { return false; }
+}
+// no crash on a reference to a non-identifier symbol (operator ==).
+bool s = std::operator==(std::Foo(), 1);