[clang][Interp] Compile field+base destruction into class dtor func

[tbaederr]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

---

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

5 Files Affected:

• (modified) clang/lib/AST/Interp/Compiler.cpp (+58-39)
• (modified) clang/lib/AST/Interp/Compiler.h (+1)
• (modified) clang/lib/AST/Interp/Interp.cpp (-24)
• (modified) clang/test/AST/Interp/cxx20.cpp (+1-2)
• (modified) clang/test/AST/Interp/new-delete.cpp (+2-3)

diff --git a/clang/lib/AST/Interp/Compiler.cpp b/clang/lib/AST/Interp/Compiler.cpp
index dd24cff1bab46e..7be15921384f1f 100644
--- a/clang/lib/AST/Interp/Compiler.cpp
+++ b/clang/lib/AST/Interp/Compiler.cpp
@@ -4856,6 +4856,50 @@ bool Compiler<Emitter>::compileConstructor(const CXXConstructorDecl *Ctor) {
   return this->emitRetVoid(SourceInfo{});
 }
 
+template <class Emitter>
+bool Compiler<Emitter>::compileDestructor(const CXXDestructorDecl *Dtor) {
+  const RecordDecl *RD = Dtor->getParent();
+  const Record *R = this->getRecord(RD);
+  if (!R)
+    return false;
+
+  if (!this->emitThis(Dtor))
+    return false;
+
+  assert(R);
+  if (!R->isUnion()) {
+    // First, destroy all fields.
+    for (const Record::Field &Field : llvm::reverse(R->fields())) {
+      const Descriptor *D = Field.Desc;
+      if (!D->isPrimitive() && !D->isPrimitiveArray()) {
+        if (!this->emitGetPtrField(Field.Offset, SourceInfo{}))
+          return false;
+        if (!this->emitDestruction(D))
+          return false;
+        if (!this->emitPopPtr(SourceInfo{}))
+          return false;
+      }
+    }
+  }
+
+  if (!Dtor->isTrivial()) {
+    if (!this->visitStmt(Dtor->getBody()))
+      return false;
+  }
+
+  for (const Record::Base &Base : llvm::reverse(R->bases())) {
+    if (!this->emitGetPtrBase(Base.Offset, SourceInfo{}))
+      return false;
+    if (!this->emitRecordDestruction(Base.R))
+      return false;
+    if (!this->emitPopPtr(SourceInfo{}))
+      return false;
+  }
+
+  // FIXME: Virtual bases.
+  return this->emitPopPtr(Dtor) && this->emitRetVoid(Dtor);
+}
+
 template <class Emitter>
 bool Compiler<Emitter>::visitFunc(const FunctionDecl *F) {
   // Classify the return type.
@@ -4863,6 +4907,8 @@ bool Compiler<Emitter>::visitFunc(const FunctionDecl *F) {
 
   if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(F))
     return this->compileConstructor(Ctor);
+  if (const auto *Dtor = dyn_cast<CXXDestructorDecl>(F))
+    return this->compileDestructor(Dtor);
 
   // Emit custom code if this is a lambda static invoker.
   if (const auto *MD = dyn_cast<CXXMethodDecl>(F);
@@ -5573,46 +5619,19 @@ bool Compiler<Emitter>::emitComplexComparison(const Expr *LHS, const Expr *RHS,
 template <class Emitter>
 bool Compiler<Emitter>::emitRecordDestruction(const Record *R) {
   assert(R);
-  if (!R->isUnion()) {
-    // First, destroy all fields.
-    for (const Record::Field &Field : llvm::reverse(R->fields())) {
-      const Descriptor *D = Field.Desc;
-      if (!D->isPrimitive() && !D->isPrimitiveArray()) {
-        if (!this->emitGetPtrField(Field.Offset, SourceInfo{}))
-          return false;
-        if (!this->emitDestruction(D))
-          return false;
-        if (!this->emitPopPtr(SourceInfo{}))
-          return false;
-      }
-    }
-  }
-
-  // Now emit the destructor and recurse into base classes.
-  if (const CXXDestructorDecl *Dtor = R->getDestructor();
-      Dtor && !Dtor->isTrivial()) {
-    const Function *DtorFunc = getFunction(Dtor);
-    if (!DtorFunc)
-      return false;
-    assert(DtorFunc->hasThisPointer());
-    assert(DtorFunc->getNumParams() == 1);
-    if (!this->emitDupPtr(SourceInfo{}))
-      return false;
-    if (!this->emitCall(DtorFunc, 0, SourceInfo{}))
-      return false;
-  }
-
-  for (const Record::Base &Base : llvm::reverse(R->bases())) {
-    if (!this->emitGetPtrBase(Base.Offset, SourceInfo{}))
-      return false;
-    if (!this->emitRecordDestruction(Base.R))
-      return false;
-    if (!this->emitPopPtr(SourceInfo{}))
-      return false;
-  }
+  const CXXDestructorDecl *Dtor = R->getDestructor();
+  if (!Dtor || Dtor->isTrivial())
+    return true;
 
-  // FIXME: Virtual bases.
-  return true;
+  assert(Dtor);
+  const Function *DtorFunc = getFunction(Dtor);
+  if (!DtorFunc)
+    return false;
+  assert(DtorFunc->hasThisPointer());
+  assert(DtorFunc->getNumParams() == 1);
+  if (!this->emitDupPtr(SourceInfo{}))
+    return false;
+  return this->emitCall(DtorFunc, 0, SourceInfo{});
 }
 /// When calling this, we have a pointer of the local-to-destroy
 /// on the stack.
diff --git a/clang/lib/AST/Interp/Compiler.h b/clang/lib/AST/Interp/Compiler.h
index d94d3613775a19..112219c49e8bdd 100644
--- a/clang/lib/AST/Interp/Compiler.h
+++ b/clang/lib/AST/Interp/Compiler.h
@@ -358,6 +358,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
                              const QualType DerivedType);
   bool emitLambdaStaticInvokerBody(const CXXMethodDecl *MD);
   bool compileConstructor(const CXXConstructorDecl *Ctor);
+  bool compileDestructor(const CXXDestructorDecl *Dtor);
 
   bool checkLiteralType(const Expr *E);
 
diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 13390007fde33c..49a90edf8917c7 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -867,23 +867,6 @@ static bool runRecordDestructor(InterpState &S, CodePtr OpPC,
     return false;
   }
 
-  // Fields.
-  for (const Record::Field &Field : llvm::reverse(R->fields())) {
-    const Descriptor *D = Field.Desc;
-    if (D->isRecord()) {
-      if (!runRecordDestructor(S, OpPC, BasePtr.atField(Field.Offset), D))
-        return false;
-    } else if (D->isCompositeArray()) {
-      const Descriptor *ElemDesc = Desc->ElemDesc;
-      assert(ElemDesc->isRecord());
-      for (unsigned I = 0; I != Desc->getNumElems(); ++I) {
-        if (!runRecordDestructor(S, OpPC, BasePtr.atIndex(I).narrow(),
-                                 ElemDesc))
-          return false;
-      }
-    }
-  }
-
   // Destructor of this record.
   if (const CXXDestructorDecl *Dtor = R->getDestructor();
       Dtor && !Dtor->isTrivial()) {
@@ -895,13 +878,6 @@ static bool runRecordDestructor(InterpState &S, CodePtr OpPC,
     if (!Call(S, OpPC, DtorFunc, 0))
       return false;
   }
-
-  // Bases.
-  for (const Record::Base &Base : llvm::reverse(R->bases())) {
-    if (!runRecordDestructor(S, OpPC, BasePtr.atField(Base.Offset), Base.Desc))
-      return false;
-  }
-
   return true;
 }
 
diff --git a/clang/test/AST/Interp/cxx20.cpp b/clang/test/AST/Interp/cxx20.cpp
index 389d9d883725f4..77a967d42c4efe 100644
--- a/clang/test/AST/Interp/cxx20.cpp
+++ b/clang/test/AST/Interp/cxx20.cpp
@@ -859,7 +859,6 @@ namespace DefinitionLoc {
                                           // both-note {{non-constexpr constructor}}
 }
 
-/// FIXME: Call base dtors when explicitly calling dtor.
 namespace VirtDtor {
   class B {
   public:
@@ -900,5 +899,5 @@ namespace VirtDtor {
     return buff[0] == a && buff[1] == b;
   }
 
-  static_assert(test('C', 'B')); // expected-error {{failed}}
+  static_assert(test('C', 'B'));
 }
diff --git a/clang/test/AST/Interp/new-delete.cpp b/clang/test/AST/Interp/new-delete.cpp
index 325ce27c6d51da..6bb30bc19f110c 100644
--- a/clang/test/AST/Interp/new-delete.cpp
+++ b/clang/test/AST/Interp/new-delete.cpp
@@ -514,8 +514,7 @@ namespace DeleteRunsDtors {
   static_assert(abc2() == 1);
 }
 
-/// FIXME: There is a slight difference in diagnostics here, because we don't
-/// create a new frame when we delete record fields or bases at all.
+/// FIXME: There is a slight difference in diagnostics here.
 namespace FaultyDtorCalledByDelete {
   struct InnerFoo {
     int *mem;
@@ -536,7 +535,7 @@ namespace FaultyDtorCalledByDelete {
       a = new int(13);
       IF.mem = new int(100);
     }
-    constexpr ~Foo() { delete a; }
+    constexpr ~Foo() { delete a; } // expected-note {{in call to}}
   };
 
   constexpr int abc() {