[clang] [Sema] Instantiate destructors for initialized anonymous union fields (PR #128866)

Maurice Heumann via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 4 23:23:43 PST 2025


https://github.com/momo5502 updated https://github.com/llvm/llvm-project/pull/128866

>From bb4091d2f9b7062aa83e5bee2ba525478a7dbd0a Mon Sep 17 00:00:00 2001
From: Maurice Heumann <maurice.heumann at wibu.com>
Date: Wed, 26 Feb 2025 14:31:47 +0100
Subject: [PATCH 1/3] Instantiate destructors from initialized anonymous union
 fields

---
 clang/include/clang/Sema/Sema.h               |  2 +
 clang/lib/Sema/SemaDeclCXX.cpp                | 95 ++++++++++++-------
 clang/test/SemaCXX/cxx0x-nontrivial-union.cpp |  6 +-
 .../test/SemaCXX/union-member-destructor.cpp  | 48 ++++++++++
 4 files changed, 113 insertions(+), 38 deletions(-)
 create mode 100644 clang/test/SemaCXX/union-member-destructor.cpp

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5b5cee5810488..c5b2d58a78fd2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5432,6 +5432,8 @@ class Sema final : public SemaBase {
   void MarkBaseAndMemberDestructorsReferenced(SourceLocation Loc,
                                               CXXRecordDecl *Record);
 
+  void MarkFieldDestructorReferenced(SourceLocation Loc, FieldDecl *Field);
+
   /// Mark destructors of virtual bases of this class referenced. In the Itanium
   /// C++ ABI, this is done when emitting a destructor for any non-abstract
   /// class. In the Microsoft C++ ABI, this is done any time a class's
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index a3a028b9485d6..761f6a09037a7 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -5450,10 +5450,31 @@ bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors,
            NumInitializers * sizeof(CXXCtorInitializer*));
     Constructor->setCtorInitializers(baseOrMemberInitializers);
 
+    SourceLocation Location = Constructor->getLocation();
+
+    for (CXXCtorInitializer *Initializer : Info.AllToInit) {
+      FieldDecl *Field = Initializer->getAnyMember();
+      if (!Field)
+        continue;
+
+      RecordDecl *Parent = Field->getParent();
+
+      while (Parent) {
+        if (Parent->isUnion()) {
+          MarkFieldDestructorReferenced(Location, Field);
+          break;
+        }
+
+        if (!Parent->isAnonymousStructOrUnion() || Parent == ClassDecl) {
+          break;
+        }
+
+        Parent = cast<RecordDecl>(Parent->getDeclContext());
+      }
+    }
     // Constructors implicitly reference the base and member
     // destructors.
-    MarkBaseAndMemberDestructorsReferenced(Constructor->getLocation(),
-                                           Constructor->getParent());
+    MarkBaseAndMemberDestructorsReferenced(Location, Constructor->getParent());
   }
 
   return HadError;
@@ -5758,6 +5779,42 @@ void Sema::ActOnMemInitializers(Decl *ConstructorDecl,
   DiagnoseUninitializedFields(*this, Constructor);
 }
 
+void Sema::MarkFieldDestructorReferenced(SourceLocation Location,
+                                         FieldDecl *Field) {
+  if (Field->isInvalidDecl())
+    return;
+
+  // Don't destroy incomplete or zero-length arrays.
+  if (isIncompleteOrZeroLengthArrayType(Context, Field->getType()))
+    return;
+
+  QualType FieldType = Context.getBaseElementType(Field->getType());
+
+  const RecordType *RT = FieldType->getAs<RecordType>();
+  if (!RT)
+    return;
+
+  CXXRecordDecl *FieldClassDecl = cast<CXXRecordDecl>(RT->getDecl());
+  if (FieldClassDecl->isInvalidDecl())
+    return;
+  if (FieldClassDecl->hasIrrelevantDestructor())
+    return;
+  // The destructor for an implicit anonymous union member is never invoked.
+  if (FieldClassDecl->isUnion() && FieldClassDecl->isAnonymousStructOrUnion())
+    return;
+
+  CXXDestructorDecl *Dtor = LookupDestructor(FieldClassDecl);
+  // Dtor might still be missing, e.g because it's invalid.
+  if (!Dtor)
+    return;
+  CheckDestructorAccess(Field->getLocation(), Dtor,
+                        PDiag(diag::err_access_dtor_field)
+                            << Field->getDeclName() << FieldType);
+
+  MarkFunctionReferenced(Location, Dtor);
+  DiagnoseUseOfDecl(Dtor, Location);
+}
+
 void
 Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
                                              CXXRecordDecl *ClassDecl) {
@@ -5773,39 +5830,7 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
 
   // Non-static data members.
   for (auto *Field : ClassDecl->fields()) {
-    if (Field->isInvalidDecl())
-      continue;
-
-    // Don't destroy incomplete or zero-length arrays.
-    if (isIncompleteOrZeroLengthArrayType(Context, Field->getType()))
-      continue;
-
-    QualType FieldType = Context.getBaseElementType(Field->getType());
-
-    const RecordType* RT = FieldType->getAs<RecordType>();
-    if (!RT)
-      continue;
-
-    CXXRecordDecl *FieldClassDecl = cast<CXXRecordDecl>(RT->getDecl());
-    if (FieldClassDecl->isInvalidDecl())
-      continue;
-    if (FieldClassDecl->hasIrrelevantDestructor())
-      continue;
-    // The destructor for an implicit anonymous union member is never invoked.
-    if (FieldClassDecl->isUnion() && FieldClassDecl->isAnonymousStructOrUnion())
-      continue;
-
-    CXXDestructorDecl *Dtor = LookupDestructor(FieldClassDecl);
-    // Dtor might still be missing, e.g because it's invalid.
-    if (!Dtor)
-      continue;
-    CheckDestructorAccess(Field->getLocation(), Dtor,
-                          PDiag(diag::err_access_dtor_field)
-                            << Field->getDeclName()
-                            << FieldType);
-
-    MarkFunctionReferenced(Location, Dtor);
-    DiagnoseUseOfDecl(Dtor, Location);
+    MarkFieldDestructorReferenced(Location, Field);
   }
 
   // We only potentially invoke the destructors of potentially constructed
diff --git a/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp b/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp
index c7cdf76d850db..4bb012f6e4247 100644
--- a/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp
+++ b/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp
@@ -51,7 +51,7 @@ namespace disabled_dtor {
   union disable_dtor {
     T val;
     template<typename...U>
-    disable_dtor(U &&...u) : val(forward<U>(u)...) {}
+    disable_dtor(U &&...u) : val(forward<U>(u)...) {} // expected-error {{attempt to use a deleted function}}
     ~disable_dtor() {}
   };
 
@@ -59,10 +59,10 @@ namespace disabled_dtor {
     deleted_dtor(int n, char c) : n(n), c(c) {}
     int n;
     char c;
-    ~deleted_dtor() = delete;
+    ~deleted_dtor() = delete; // expected-note {{'~deleted_dtor' has been explicitly marked deleted here}}
   };
 
-  disable_dtor<deleted_dtor> dd(4, 'x');
+  disable_dtor<deleted_dtor> dd(4, 'x');  // expected-note {{in instantiation of function template specialization 'disabled_dtor::disable_dtor<disabled_dtor::deleted_dtor>::disable_dtor<int, char>' requested here}}
 }
 
 namespace optional {
diff --git a/clang/test/SemaCXX/union-member-destructor.cpp b/clang/test/SemaCXX/union-member-destructor.cpp
new file mode 100644
index 0000000000000..38fd505005940
--- /dev/null
+++ b/clang/test/SemaCXX/union-member-destructor.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+namespace t1{
+template <class T> struct VSX {
+  ~VSX() { static_assert(sizeof(T) != 4, ""); } // expected-error {{static assertion failed due to requirement 'sizeof(int) != 4':}} \
+                                                // expected-note {{expression evaluates to '4 != 4'}}
+};
+struct VS {
+  union {
+    VSX<int> _Tail;
+  };
+  ~VS() { }
+  VS(short);
+};
+VS::VS(short) : _Tail() { } // expected-note {{in instantiation of member function 't1::VSX<int>::~VSX' requested here}}
+}
+
+
+namespace t2 {
+template <class T> struct VSX {
+  ~VSX() { static_assert(sizeof(T) != 4, ""); } // expected-error {{static assertion failed due to requirement 'sizeof(int) != 4':}} \
+                                                // expected-note {{expression evaluates to '4 != 4'}}
+};
+struct VS {
+  union {
+    struct {
+      VSX<int> _Tail;
+    };
+  };
+  ~VS() { }
+  VS(short);
+};
+VS::VS(short) : _Tail() { } // expected-note {{in instantiation of member function 't2::VSX<int>::~VSX' requested here}}
+}
+
+
+namespace t3 {
+template <class T> struct VSX {
+  ~VSX() { static_assert(sizeof(T) != 4, ""); } // expected-error {{static assertion failed due to requirement 'sizeof(int) != 4':}} \
+                                                // expected-note {{expression evaluates to '4 != 4'}}
+};
+union VS {
+  VSX<int> _Tail;
+  ~VS() { }
+  VS(short);
+};
+VS::VS(short) : _Tail() { } // expected-note {{in instantiation of member function 't3::VSX<int>::~VSX' requested here}}
+}

>From d95c274d7acd4fb7dbf645f5a58e238785539678 Mon Sep 17 00:00:00 2001
From: Maurice Heumann <maurice.heumann at wibu.com>
Date: Mon, 3 Mar 2025 08:52:55 +0100
Subject: [PATCH 2/3] Split up base and member destructor referencing

---
 clang/include/clang/Sema/Sema.h |  1 +
 clang/lib/Sema/SemaDeclCXX.cpp  | 62 +++++++++++++++------------------
 2 files changed, 30 insertions(+), 33 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c5b2d58a78fd2..845a3becf76cf 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5432,6 +5432,7 @@ class Sema final : public SemaBase {
   void MarkBaseAndMemberDestructorsReferenced(SourceLocation Loc,
                                               CXXRecordDecl *Record);
 
+  void MarkBaseDestructorsReferenced(SourceLocation Loc, CXXRecordDecl *Record);
   void MarkFieldDestructorReferenced(SourceLocation Loc, FieldDecl *Field);
 
   /// Mark destructors of virtual bases of this class referenced. In the Itanium
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 761f6a09037a7..7f95346ca5de2 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -5452,29 +5452,18 @@ bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors,
 
     SourceLocation Location = Constructor->getLocation();
 
+    // Constructors implicitly reference the base and member
+    // destructors.
+
     for (CXXCtorInitializer *Initializer : Info.AllToInit) {
       FieldDecl *Field = Initializer->getAnyMember();
       if (!Field)
         continue;
 
-      RecordDecl *Parent = Field->getParent();
-
-      while (Parent) {
-        if (Parent->isUnion()) {
-          MarkFieldDestructorReferenced(Location, Field);
-          break;
-        }
-
-        if (!Parent->isAnonymousStructOrUnion() || Parent == ClassDecl) {
-          break;
-        }
-
-        Parent = cast<RecordDecl>(Parent->getDeclContext());
-      }
+      MarkFieldDestructorReferenced(Location, Field);
     }
-    // Constructors implicitly reference the base and member
-    // destructors.
-    MarkBaseAndMemberDestructorsReferenced(Location, Constructor->getParent());
+
+    MarkBaseDestructorsReferenced(Location, Constructor->getParent());
   }
 
   return HadError;
@@ -5815,24 +5804,11 @@ void Sema::MarkFieldDestructorReferenced(SourceLocation Location,
   DiagnoseUseOfDecl(Dtor, Location);
 }
 
-void
-Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
-                                             CXXRecordDecl *ClassDecl) {
-  // Ignore dependent contexts. Also ignore unions, since their members never
-  // have destructors implicitly called.
-  if (ClassDecl->isDependentContext() || ClassDecl->isUnion())
+void Sema::MarkBaseDestructorsReferenced(SourceLocation Location,
+                                         CXXRecordDecl *ClassDecl) {
+  if (ClassDecl->isDependentContext())
     return;
 
-  // FIXME: all the access-control diagnostics are positioned on the
-  // field/base declaration.  That's probably good; that said, the
-  // user might reasonably want to know why the destructor is being
-  // emitted, and we currently don't say.
-
-  // Non-static data members.
-  for (auto *Field : ClassDecl->fields()) {
-    MarkFieldDestructorReferenced(Location, Field);
-  }
-
   // We only potentially invoke the destructors of potentially constructed
   // subobjects.
   bool VisitVirtualBases = !ClassDecl->isAbstract();
@@ -5888,6 +5864,26 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
                                          &DirectVirtualBases);
 }
 
+void Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
+                                                  CXXRecordDecl *ClassDecl) {
+  // Ignore dependent contexts. Also ignore unions, since their members never
+  // have destructors implicitly called.
+  if (ClassDecl->isDependentContext() || ClassDecl->isUnion())
+    return;
+
+  // FIXME: all the access-control diagnostics are positioned on the
+  // field/base declaration.  That's probably good; that said, the
+  // user might reasonably want to know why the destructor is being
+  // emitted, and we currently don't say.
+
+  // Non-static data members.
+  for (auto *Field : ClassDecl->fields()) {
+    MarkFieldDestructorReferenced(Location, Field);
+  }
+
+  MarkBaseDestructorsReferenced(Location, ClassDecl);
+}
+
 void Sema::MarkVirtualBaseDestructorsReferenced(
     SourceLocation Location, CXXRecordDecl *ClassDecl,
     llvm::SmallPtrSetImpl<const RecordType *> *DirectVirtualBases) {

>From 3bb966872a0b577013c0042f658e84d7c16f7876 Mon Sep 17 00:00:00 2001
From: Maurice Heumann <maurice.heumann at wibu.com>
Date: Wed, 5 Mar 2025 08:23:25 +0100
Subject: [PATCH 3/3] Include std reference and add delegating constructor test

---
 clang/lib/Sema/SemaDeclCXX.cpp                 | 5 +++++
 clang/test/SemaCXX/union-member-destructor.cpp | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 7f95346ca5de2..62ef4adb28cc9 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -5460,6 +5460,11 @@ bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors,
       if (!Field)
         continue;
 
+      // C++ [class.base.init]p12:
+      //   In a non-delegating constructor, the destructor for each
+      //   potentially constructed subobject of class type is potentially
+      //   invoked
+      //   ([class.dtor]).
       MarkFieldDestructorReferenced(Location, Field);
     }
 
diff --git a/clang/test/SemaCXX/union-member-destructor.cpp b/clang/test/SemaCXX/union-member-destructor.cpp
index 38fd505005940..de809079693a7 100644
--- a/clang/test/SemaCXX/union-member-destructor.cpp
+++ b/clang/test/SemaCXX/union-member-destructor.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
 
-namespace t1{
+namespace t1 {
 template <class T> struct VSX {
   ~VSX() { static_assert(sizeof(T) != 4, ""); } // expected-error {{static assertion failed due to requirement 'sizeof(int) != 4':}} \
                                                 // expected-note {{expression evaluates to '4 != 4'}}
@@ -11,7 +11,9 @@ struct VS {
   };
   ~VS() { }
   VS(short);
+  VS();
 };
+VS::VS() : VS(0) { }
 VS::VS(short) : _Tail() { } // expected-note {{in instantiation of member function 't1::VSX<int>::~VSX' requested here}}
 }
 



More information about the cfe-commits mailing list