[clang][ItaniumMangle] Check SizeExpr for DependentSizedArrayType (PR49478)

[oToToT]

Summary

As ArrayType::ArrayType mentioned in clang/lib/AST/Type.cpp, a DependentSizedArrayType might not have size expression because it it used as the type of a dependent array of unknown bound with a dependent braced initializer.

Thus, I add a check when mangling array of that type.

This should fix https://bugs.llvm.org/show_bug.cgi?id=49478

[oToToT]

Remove redundant whitespace in comment.

[yaxunl]

clang/test/AST/ast-dump-array-json.cpp

6 ↗

(On Diff #333526)

I am not sure if this test tests the code you change since the mangled variable name does not encode type.

[oToToT]

clang/test/AST/ast-dump-array-json.cpp

6 ↗

(On Diff #333526)

I think this test my code. Or, at least, this test will trigger a runtime error in the latest clang without this patch when mangling A::Test.

Also, maybe I could try construct a test to mangle variable with element type if needed.

WDYT?

[rsmith]

clang/test/AST/ast-dump-array-json.cpp

6 ↗

(On Diff #333526)

I would like to see a test that verifies the A<type>_ mangling is produced.

[oToToT]

clang/test/AST/ast-dump-array-json.cpp

6 ↗

(On Diff #333526)

Ah, you're right! After some investigation, I found that this CHECK in the test doesn't really check my patch.
This test only ensure that clang won't get runtime error when handling this file.

However, I can't find any clever way to construct a test like this to ensure the output of CXXNameMangler::mangleType(const DependentSizedArrayType *). (It is called here only for checking ABI tags, so I guess no output will be displayed).

I will try replace this test with a unittest to check the output.

[oToToT]

Updated tests to check mangled type name.

However, I not sure whether it is proper to put the tests here.

[oToToT]

Remove redundant trailing spaces.

Sorry for not checking this at the first time I submit.

clang/lib/AST/ItaniumMangle.cpp

void CXXNameMangler::mangleType(const VariableArrayType *T) {
   // decayed vla types (size 0) will just be skipped.
   if (T->getSizeExpr())
     mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }
 void CXXNameMangler::mangleType(const DependentSizedArrayType *T) {
   Out << 'A';
+  // A DependentSizedArrayType might not have size expression as below
+  //
+  // template<int ...N> int arr[] = {N...};
+  if (T->getSizeExpr())
     mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }
 void CXXNameMangler::mangleType(const IncompleteArrayType *T) {
   Out << "A_";
   mangleType(T->getElementType());
 }
 

clang/unittests/AST/DeclTest.cpp

std::unique_ptr<ItaniumMangleContext> MC(
     llvm::raw_string_ostream OS_G(MangleG);
     MC->mangleName(DeclF, OS_F);
     MC->mangleName(DeclG, OS_G);
   }
 
   ASSERT_TRUE(0 == MangleF.compare("\x01" "foo"));
   ASSERT_TRUE(0 == MangleG.compare("goo"));
 }
+
+TEST(Decl, MangleDependentSizedArray) {
+  StringRef Code = R"(
+    template <int ...N>
+    int A[] = {N...};
+
+    template <typename T, int N>
+    struct S {
+      T B[N];
+    };
+  )";
+  auto AST =
+      tooling::buildASTFromCodeWithArgs(Code, {"-target", "i386-apple-darwin"});
+  ASTContext &Ctx = AST->getASTContext();
+  assert(Ctx.getTargetInfo().getDataLayout().getGlobalPrefix() &&
+         "Expected target to have a global prefix");
+  DiagnosticsEngine &Diags = AST->getDiagnostics();
+
+  const auto *DeclA =
+      selectFirst<VarDecl>("A", match(varDecl().bind("A"), Ctx));
+  const auto *DeclB =
+      selectFirst<FieldDecl>("B", match(fieldDecl().bind("B"), Ctx));
+
+  std::string MangleA, MangleB;
+  llvm::raw_string_ostream OS_A(MangleA), OS_B(MangleB);
+  std::unique_ptr<ItaniumMangleContext> MC(
+      ItaniumMangleContext::create(Ctx, Diags));
+
+  MC->mangleTypeName(DeclA->getType(), OS_A);
+  MC->mangleTypeName(DeclB->getType(), OS_B);
+
+  ASSERT_TRUE(0 == MangleA.compare("_ZTSA_i"));
+  ASSERT_TRUE(0 == MangleB.compare("_ZTSAT0__T_"));
+}