[clang][Interp] Correctly emit destructors for multi-dimensional arrays

[tbaederr]

We were not taking those into account correctly when emitting destructors. Fix that and add tests for it.

Fixes #69115

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

We were not taking those into account correctly when emitting destructors. Fix that and add tests for it.

Fixes #69115

---

Full diff: https://github.com/llvm/llvm-project/pull/69140.diff

2 Files Affected:

• (modified) clang/lib/AST/Interp/ByteCodeExprGen.cpp (+21-12)
• (modified) clang/test/AST/Interp/arrays.cpp (+57)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index e9e20b222d5d34f..122c505078c77ef 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -2661,19 +2661,28 @@ bool ByteCodeExprGen<Emitter>::emitRecordDestruction(const Descriptor *Desc) {
   // Arrays.
   if (Desc->isArray()) {
     const Descriptor *ElemDesc = Desc->ElemDesc;
-    const Record *ElemRecord = ElemDesc->ElemRecord;
-    assert(ElemRecord); // This is not a primitive array.
+    assert(ElemDesc);
+
+    // Don't need to do anything for these.
+    if (ElemDesc->isPrimitiveArray())
+      return this->emitPopPtr(SourceInfo{});
+
+    // If this is an array of record types, check if we need
+    // to call the element destructors at all. If not, try
+    // to save the work.
+    if (const Record *ElemRecord = ElemDesc->ElemRecord) {
+      if (const CXXDestructorDecl *Dtor = ElemRecord->getDestructor();
+          !Dtor || Dtor->isTrivial())
+        return this->emitPopPtr(SourceInfo{});
+    }
 
-    if (const CXXDestructorDecl *Dtor = ElemRecord->getDestructor();
-        Dtor && !Dtor->isTrivial()) {
-      for (ssize_t I = Desc->getNumElems() - 1; I >= 0; --I) {
-        if (!this->emitConstUint64(I, SourceInfo{}))
-          return false;
-        if (!this->emitArrayElemPtrUint64(SourceInfo{}))
-          return false;
-        if (!this->emitRecordDestruction(Desc->ElemDesc))
-          return false;
-      }
+    for (ssize_t I = Desc->getNumElems() - 1; I >= 0; --I) {
+      if (!this->emitConstUint64(I, SourceInfo{}))
+        return false;
+      if (!this->emitArrayElemPtrUint64(SourceInfo{}))
+        return false;
+      if (!this->emitRecordDestruction(ElemDesc))
+        return false;
     }
     return this->emitPopPtr(SourceInfo{});
   }
diff --git a/clang/test/AST/Interp/arrays.cpp b/clang/test/AST/Interp/arrays.cpp
index 281835f828bbd7c..3aa58443089d499 100644
--- a/clang/test/AST/Interp/arrays.cpp
+++ b/clang/test/AST/Interp/arrays.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
 // RUN: %clang_cc1 -verify=ref %s
+// RUN: %clang_cc1 -verify=ref -std=c++20 %s
 
 constexpr int m = 3;
 constexpr const int *foo[][5] = {
@@ -390,3 +392,58 @@ namespace StringZeroFill {
   static_assert(b[4] == '\0', "");
   static_assert(b[5] == '\0', "");
 }
+
+namespace GH69115 {
+  /// This used to crash because we were trying to emit destructors for the
+  /// array.
+  constexpr int foo() {
+    int arr[2][2] = {1, 2, 3, 4};
+    return 0;
+  }
+  static_assert(foo() == 0, "");
+
+  /// Test that we still emit the destructors for multi-dimensional
+  /// composite arrays.
+#if __cplusplus >= 202002L
+  constexpr void assert(bool C) {
+    if (C)
+      return;
+    // Invalid in constexpr.
+    (void)(1 / 0); // expected-warning {{undefined}} \
+                   // ref-warning {{undefined}}
+  }
+
+  class F {
+  public:
+    int a;
+    int *dtor;
+    int &idx;
+    constexpr F(int a, int *dtor, int &idx) : a(a), dtor(dtor), idx(idx) {}
+    constexpr ~F() noexcept(false){
+      dtor[idx] = a;
+      ++idx;
+    }
+  };
+  constexpr int foo2() {
+    int dtorIndices[] = {0, 0, 0, 0};
+    int idx = 0;
+
+    {
+      F arr[2][2] = {F(1, dtorIndices, idx),
+                     F(2, dtorIndices, idx),
+                     F(3, dtorIndices, idx),
+                     F(4, dtorIndices, idx)};
+    }
+
+    /// Reverse-reverse order.
+    assert(idx == 4);
+    assert(dtorIndices[0] == 4);
+    assert(dtorIndices[1] == 3);
+    assert(dtorIndices[2] == 2);
+    assert(dtorIndices[3] == 1);
+
+    return 0;
+  }
+  static_assert(foo2() == 0, "");
+#endif
+}

[tbaederr]

Ping

[AaronBallman]

LGTM