[clang] [clang][Interp] Compile field+base destruction into class dtor func (PR #102871)

Timm Baeder via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 12 05:51:56 PDT 2024


https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/102871

>From 3c780a997df63f6c60d25d34067675af30c9ca02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Mon, 12 Aug 2024 11:45:27 +0200
Subject: [PATCH] [clang][Interp] Compile field+base destruction into class
 dtor func

---
 clang/lib/AST/Interp/Compiler.cpp    | 97 +++++++++++++++++-----------
 clang/lib/AST/Interp/Compiler.h      |  1 +
 clang/lib/AST/Interp/Interp.cpp      | 24 -------
 clang/test/AST/Interp/cxx20.cpp      |  3 +-
 clang/test/AST/Interp/new-delete.cpp |  5 +-
 clang/test/AST/Interp/records.cpp    | 31 +++++++++
 6 files changed, 93 insertions(+), 68 deletions(-)

diff --git a/clang/lib/AST/Interp/Compiler.cpp b/clang/lib/AST/Interp/Compiler.cpp
index dd24cff1bab46e..d32b595f9f0724 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 (!Dtor->isTrivial() && Dtor->getBody()) {
+    if (!this->visitStmt(Dtor->getBody()))
+      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;
+      }
+    }
+  }
+
+  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() {
diff --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
index e620bf9e0e041e..3ab05ea85040c2 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -1596,3 +1596,34 @@ namespace VirtDtor {
   static_assert(virt_dtor(0, "ZYX"));
 }
 #endif
+
+namespace DtorDestroysFieldsAfterSelf {
+    struct  S {
+      int a = 10;
+      constexpr ~S() {
+        a = 0;
+      }
+
+    };
+    struct F {
+      S s;
+      int a;
+      int &b;
+      constexpr F(int a, int &b) : a(a), b(b) {}
+      constexpr ~F() {
+        b += s.a;
+      }
+    };
+
+  constexpr int foo() {
+    int a = 10;
+    int b = 5;
+    {
+      F f(a, b);
+    }
+
+    return b;
+  }
+
+  static_assert(foo() == 15);
+}



More information about the cfe-commits mailing list