[Clang] Fix crash in CIndex, when visiting a static_assert without message

[kiloalphaindia]

Summary

After implementation of "[Clang] Implement P2741R3 - user-generated static_assert messages" (47ccfd7a89e2a9a747a7114db18db1376324799c) the c indexer crashes when handling a static_assert w/o any message.
This is caused by using dyn_cast to get the literal string, which isn't working on nullptr.

[cor3ntin]

clang/tools/libclang/CIndex.cpp

1297

It's probably better to cast to Expr and visit it unconditionally. But it could always be null, so dyn_cast_if_present is probably correct.
dyn_cast_if_present<Expr> should be what we want then

[kiloalphaindia]

Yes, that's a good idea. Now kdevelop also highlights the message-expression properly.

[kiloalphaindia]

Sorry... forgot formatting... removed commented code... changed "Hallo Welt!" to english

[cor3ntin]

There is a failed unit test, can you look into it?

[aaron.ballman]

clang/unittests/libclang/LibclangTest.cpp

1201

This is going to be testing whatever STL headers are installed on the developer's machine; instead of including the STL header, you should mock up the std::string_view class so we're always testing something consistently.

[kiloalphaindia]

Yes, I think so, too. Will have a look this evening.

[kiloalphaindia]

The test is no more using any includes now. Hopefully this fixes it, otherwise I will have to add something that prints the diagnostic.

I don't have write permissions on the repository. Please commit it when OK.

[cor3ntin]

In D156053#4531015, @kiloalphaindia wrote:

The test is no more using any includes now. Hopefully this fixes it, otherwise I will have to add something that prints the diagnostic.

I don't have write permissions on the repository. Please commit it when OK.

Will do! (once the test pass) What name / email address should we use for commiting?

[kiloalphaindia]

Please use Kai Stierand <[e-mail]> as in 5d859a1cdee3c15dce692767ee3e9ad03a8c4c1b

clang/tools/libclang/CIndex.cpp

if (VisitNestedNameSpecifierLoc(QualifierLoc))
       return true;
 
   return false;
 }
 
 bool CursorVisitor::VisitStaticAssertDecl(StaticAssertDecl *D) {
   if (Visit(MakeCXCursor(D->getAssertExpr(), StmtParent, TU, RegionOfInterest)))
     return true;
-  if (auto *Message = dyn_cast<StringLiteral>(D->getMessage()))
+  if (auto *Message = D->getMessage())
     if (Visit(MakeCXCursor(Message, StmtParent, TU, RegionOfInterest)))
       return true;
   return false;
 }
 
 bool CursorVisitor::VisitFriendDecl(FriendDecl *D) {
   if (NamedDecl *FriendD = D->getFriendDecl()) {
     if (Visit(MakeCXCursor(FriendD, TU, RegionOfInterest)))

clang/unittests/libclang/LibclangTest.cpp

if (cursor.kind == CXCursor_UnaryOperator) {
                 CXUnaryOperator_PostInc);
       return CXChildVisit_Break;
     }
 
     return CXChildVisit_Recurse;
   });
 }
 
+TEST_F(LibclangParseTest, VisitStaticAssertDecl_noMessage) {
+  const char testSource[] = R"cpp(static_assert(true))cpp";
+  std::string fileName = "main.cpp";
+  WriteFile(fileName, testSource);
+  const char *Args[] = {"-xc++"};
+  ClangTU = clang_parseTranslationUnit(Index, fileName.c_str(), Args, 1,
+                                       nullptr, 0, TUFlags);
+
+  std::optional<CXCursor> staticAssertCsr;
+  Traverse([&](CXCursor cursor, CXCursor parent) -> CXChildVisitResult {
+    if (cursor.kind == CXCursor_StaticAssert) {
+      staticAssertCsr.emplace(cursor);
+      return CXChildVisit_Break;
+    }
+    return CXChildVisit_Recurse;
+  });
+  ASSERT_TRUE(staticAssertCsr.has_value());
+  Traverse(*staticAssertCsr, [](CXCursor cursor, CXCursor parent) {
+    EXPECT_EQ(cursor.kind, CXCursor_CXXBoolLiteralExpr);
+    return CXChildVisit_Break;
+  });
+  EXPECT_EQ(fromCXString(clang_getCursorSpelling(*staticAssertCsr)), "");
+}
+
+TEST_F(LibclangParseTest, VisitStaticAssertDecl_exprMessage) {
+  const char testSource[] = R"cpp(
+#include <string_view>
+static constexpr std::string_view message{"Hello World!"};
+static_assert(true, message);
+)cpp";
+  std::string fileName = "main.cpp";
+  WriteFile(fileName, testSource);
+  const char *Args[] = {"-xc++", "-std=c++26"};
+  ClangTU = clang_parseTranslationUnit(Index, fileName.c_str(), Args,
+                                       std::size(Args), nullptr, 0, TUFlags);
+  ASSERT_EQ(clang_getNumDiagnostics(ClangTU), 0);
+  std::optional<CXCursor> staticAssertCsr;
+  Traverse([&](CXCursor cursor, CXCursor parent) -> CXChildVisitResult {
+    if (cursor.kind == CXCursor_StaticAssert) {
+      staticAssertCsr.emplace(cursor);
+    }
+    return CXChildVisit_Continue;
+  });
+  ASSERT_TRUE(staticAssertCsr.has_value());
+  size_t argCnt = 0;
+  Traverse(*staticAssertCsr, [&argCnt](CXCursor cursor, CXCursor parent) {
+    switch (argCnt) {
+    case 0:
+      EXPECT_EQ(cursor.kind, CXCursor_CXXBoolLiteralExpr);
+      break;
+    case 1:
+      EXPECT_EQ(cursor.kind, CXCursor_DeclRefExpr);
+      break;
+    }
+    ++argCnt;
+    return CXChildVisit_Continue;
+  });
+  ASSERT_EQ(argCnt, 2);
+  EXPECT_EQ(fromCXString(clang_getCursorSpelling(*staticAssertCsr)), "");
+}
+
 class LibclangRewriteTest : public LibclangParseTest {
 public:
   CXRewriter Rew = nullptr;
   std::string Filename;
   CXFile File = nullptr;
 
   void SetUp() override {
     LibclangParseTest::SetUp();

clang/unittests/libclang/TestUtils.h

auto it = UnsavedFileContents.insert(std::make_pair(
         fixed_addr_string(new std::string(Filename)),
         fixed_addr_string(new std::string(Contents))));
     UnsavedFiles.push_back({
         it.first->first->c_str(),   // filename
         it.first->second->c_str(),  // contents
         it.first->second->size()    // length
     });
   }
-  template<typename F>
-  void Traverse(const F &TraversalFunctor) {
-    CXCursor TuCursor = clang_getTranslationUnitCursor(ClangTU);
+  template <typename F>
+  void Traverse(const CXCursor &cursor, const F &TraversalFunctor) {
     std::reference_wrapper<const F> FunctorRef = std::cref(TraversalFunctor);
-    clang_visitChildren(TuCursor,
+    clang_visitChildren(cursor,
                         &TraverseStateless<std::reference_wrapper<const F>>,
                         &FunctorRef);
   }
+
+  template <typename F> void Traverse(const F &TraversalFunctor) {
+    Traverse(clang_getTranslationUnitCursor(ClangTU), TraversalFunctor);
+  }
+
   static std::string fromCXString(CXString cx_string) {
     std::string string{clang_getCString(cx_string)};
     clang_disposeString(cx_string);
     return string;
   };
 
 protected:
   virtual void CreateIndex() { Index = clang_createIndex(0, 0); }