[clang] [Sema] Instantiate destructors for initialized members (PR #128866)
Maurice Heumann via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 6 00:57:08 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/7] 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/7] 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/7] 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}}
}
>From dd9dd124a47e91c962ba0b1ef18becb3a8f1bdfa Mon Sep 17 00:00:00 2001
From: Maurice Heumann <maurice.heumann at wibu.com>
Date: Wed, 5 Mar 2025 08:49:20 +0100
Subject: [PATCH 4/7] Add release note
---
clang/docs/ReleaseNotes.rst | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 688d50a394c62..162f100acd50a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -261,6 +261,7 @@ Bug Fixes to C++ Support
- The initialization kind of elements of structured bindings
direct-list-initialized from an array is corrected to direct-initialization.
- Clang no longer crashes when a coroutine is declared ``[[noreturn]]``. (#GH127327)
+- Clang now properly instantiates destructors for initialized members within non-delegating constructors. (#GH93251)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
>From 1752fd06db2a0138f0f89ee4fec6b013049848ea Mon Sep 17 00:00:00 2001
From: Maurice Heumann <maurice.heumann at wibu.com>
Date: Wed, 5 Mar 2025 11:51:25 +0100
Subject: [PATCH 5/7] Make destructor referencing methods static
---
clang/include/clang/Sema/Sema.h | 3 -
clang/lib/Sema/SemaDeclCXX.cpp | 200 ++++++++++++++++----------------
2 files changed, 100 insertions(+), 103 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 845a3becf76cf..5b5cee5810488 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5432,9 +5432,6 @@ 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
/// 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 62ef4adb28cc9..a12535ffc53e7 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -5283,6 +5283,102 @@ Sema::SetDelegatingInitializer(CXXConstructorDecl *Constructor,
return false;
}
+static void MarkFieldDestructorReferenced(Sema &S, SourceLocation Location,
+ FieldDecl *Field) {
+ if (Field->isInvalidDecl())
+ return;
+
+ // Don't destroy incomplete or zero-length arrays.
+ if (isIncompleteOrZeroLengthArrayType(S.Context, Field->getType()))
+ return;
+
+ QualType FieldType = S.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 = S.LookupDestructor(FieldClassDecl);
+ // Dtor might still be missing, e.g because it's invalid.
+ if (!Dtor)
+ return;
+ S.CheckDestructorAccess(Field->getLocation(), Dtor,
+ S.PDiag(diag::err_access_dtor_field)
+ << Field->getDeclName() << FieldType);
+
+ S.MarkFunctionReferenced(Location, Dtor);
+ S.DiagnoseUseOfDecl(Dtor, Location);
+}
+
+static void MarkBaseDestructorsReferenced(Sema &S, SourceLocation Location,
+ CXXRecordDecl *ClassDecl) {
+ if (ClassDecl->isDependentContext())
+ return;
+
+ // We only potentially invoke the destructors of potentially constructed
+ // subobjects.
+ bool VisitVirtualBases = !ClassDecl->isAbstract();
+
+ // If the destructor exists and has already been marked used in the MS ABI,
+ // then virtual base destructors have already been checked and marked used.
+ // Skip checking them again to avoid duplicate diagnostics.
+ if (S.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+ CXXDestructorDecl *Dtor = ClassDecl->getDestructor();
+ if (Dtor && Dtor->isUsed())
+ VisitVirtualBases = false;
+ }
+
+ llvm::SmallPtrSet<const RecordType *, 8> DirectVirtualBases;
+
+ // Bases.
+ for (const auto &Base : ClassDecl->bases()) {
+ const RecordType *RT = Base.getType()->getAs<RecordType>();
+ if (!RT)
+ continue;
+
+ // Remember direct virtual bases.
+ if (Base.isVirtual()) {
+ if (!VisitVirtualBases)
+ continue;
+ DirectVirtualBases.insert(RT);
+ }
+
+ CXXRecordDecl *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
+ // If our base class is invalid, we probably can't get its dtor anyway.
+ if (BaseClassDecl->isInvalidDecl())
+ continue;
+ if (BaseClassDecl->hasIrrelevantDestructor())
+ continue;
+
+ CXXDestructorDecl *Dtor = S.LookupDestructor(BaseClassDecl);
+ // Dtor might still be missing, e.g because it's invalid.
+ if (!Dtor)
+ continue;
+
+ // FIXME: caret should be on the start of the class name
+ S.CheckDestructorAccess(Base.getBeginLoc(), Dtor,
+ S.PDiag(diag::err_access_dtor_base)
+ << Base.getType() << Base.getSourceRange(),
+ S.Context.getTypeDeclType(ClassDecl));
+
+ S.MarkFunctionReferenced(Location, Dtor);
+ S.DiagnoseUseOfDecl(Dtor, Location);
+ }
+
+ if (VisitVirtualBases)
+ S.MarkVirtualBaseDestructorsReferenced(Location, ClassDecl,
+ &DirectVirtualBases);
+}
+
bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors,
ArrayRef<CXXCtorInitializer *> Initializers) {
if (Constructor->isDependentContext()) {
@@ -5465,10 +5561,10 @@ bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors,
// potentially constructed subobject of class type is potentially
// invoked
// ([class.dtor]).
- MarkFieldDestructorReferenced(Location, Field);
+ MarkFieldDestructorReferenced(*this, Location, Field);
}
- MarkBaseDestructorsReferenced(Location, Constructor->getParent());
+ MarkBaseDestructorsReferenced(*this, Location, Constructor->getParent());
}
return HadError;
@@ -5773,102 +5869,6 @@ 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::MarkBaseDestructorsReferenced(SourceLocation Location,
- CXXRecordDecl *ClassDecl) {
- if (ClassDecl->isDependentContext())
- return;
-
- // We only potentially invoke the destructors of potentially constructed
- // subobjects.
- bool VisitVirtualBases = !ClassDecl->isAbstract();
-
- // If the destructor exists and has already been marked used in the MS ABI,
- // then virtual base destructors have already been checked and marked used.
- // Skip checking them again to avoid duplicate diagnostics.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
- CXXDestructorDecl *Dtor = ClassDecl->getDestructor();
- if (Dtor && Dtor->isUsed())
- VisitVirtualBases = false;
- }
-
- llvm::SmallPtrSet<const RecordType *, 8> DirectVirtualBases;
-
- // Bases.
- for (const auto &Base : ClassDecl->bases()) {
- const RecordType *RT = Base.getType()->getAs<RecordType>();
- if (!RT)
- continue;
-
- // Remember direct virtual bases.
- if (Base.isVirtual()) {
- if (!VisitVirtualBases)
- continue;
- DirectVirtualBases.insert(RT);
- }
-
- CXXRecordDecl *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
- // If our base class is invalid, we probably can't get its dtor anyway.
- if (BaseClassDecl->isInvalidDecl())
- continue;
- if (BaseClassDecl->hasIrrelevantDestructor())
- continue;
-
- CXXDestructorDecl *Dtor = LookupDestructor(BaseClassDecl);
- // Dtor might still be missing, e.g because it's invalid.
- if (!Dtor)
- continue;
-
- // FIXME: caret should be on the start of the class name
- CheckDestructorAccess(Base.getBeginLoc(), Dtor,
- PDiag(diag::err_access_dtor_base)
- << Base.getType() << Base.getSourceRange(),
- Context.getTypeDeclType(ClassDecl));
-
- MarkFunctionReferenced(Location, Dtor);
- DiagnoseUseOfDecl(Dtor, Location);
- }
-
- if (VisitVirtualBases)
- MarkVirtualBaseDestructorsReferenced(Location, ClassDecl,
- &DirectVirtualBases);
-}
-
void Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
CXXRecordDecl *ClassDecl) {
// Ignore dependent contexts. Also ignore unions, since their members never
@@ -5883,10 +5883,10 @@ void Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
// Non-static data members.
for (auto *Field : ClassDecl->fields()) {
- MarkFieldDestructorReferenced(Location, Field);
+ MarkFieldDestructorReferenced(*this, Location, Field);
}
- MarkBaseDestructorsReferenced(Location, ClassDecl);
+ MarkBaseDestructorsReferenced(*this, Location, ClassDecl);
}
void Sema::MarkVirtualBaseDestructorsReferenced(
>From 5e18c122e0a4fae8e76fca109bde5394ca3523f6 Mon Sep 17 00:00:00 2001
From: Maurice Heumann <MauriceHeumann at gmail.com>
Date: Thu, 6 Mar 2025 09:11:03 +0100
Subject: [PATCH 6/7] Use auto
Co-authored-by: Aaron Ballman <aaron at aaronballman.com>
---
clang/lib/Sema/SemaDeclCXX.cpp | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index a12535ffc53e7..54a12daec472d 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -5294,11 +5294,11 @@ static void MarkFieldDestructorReferenced(Sema &S, SourceLocation Location,
QualType FieldType = S.Context.getBaseElementType(Field->getType());
- const RecordType *RT = FieldType->getAs<RecordType>();
+ const auto *RT = FieldType->getAs<RecordType>();
if (!RT)
return;
- CXXRecordDecl *FieldClassDecl = cast<CXXRecordDecl>(RT->getDecl());
+ auto *FieldClassDecl = cast<CXXRecordDecl>(RT->getDecl());
if (FieldClassDecl->isInvalidDecl())
return;
if (FieldClassDecl->hasIrrelevantDestructor())
@@ -5341,7 +5341,7 @@ static void MarkBaseDestructorsReferenced(Sema &S, SourceLocation Location,
// Bases.
for (const auto &Base : ClassDecl->bases()) {
- const RecordType *RT = Base.getType()->getAs<RecordType>();
+ const auto *RT = Base.getType()->getAs<RecordType>();
if (!RT)
continue;
@@ -5352,7 +5352,7 @@ static void MarkBaseDestructorsReferenced(Sema &S, SourceLocation Location,
DirectVirtualBases.insert(RT);
}
- CXXRecordDecl *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
+ auto *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
// If our base class is invalid, we probably can't get its dtor anyway.
if (BaseClassDecl->isInvalidDecl())
continue;
@@ -5559,8 +5559,7 @@ bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors,
// 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]).
+ // invoked.
MarkFieldDestructorReferenced(*this, Location, Field);
}
>From ffe7d106b022878dc12774679d27eeb8805120ec Mon Sep 17 00:00:00 2001
From: Maurice Heumann <maurice.heumann at wibu.com>
Date: Thu, 6 Mar 2025 09:55:20 +0100
Subject: [PATCH 7/7] Extract duplicated destructor lookups
---
clang/include/clang/Sema/Sema.h | 3 +-
clang/lib/Sema/SemaDeclCXX.cpp | 60 +++++++++++++++------------------
2 files changed, 29 insertions(+), 34 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5b5cee5810488..6f02dd82a241c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5438,7 +5438,8 @@ class Sema final : public SemaBase {
/// destructor is referenced.
void MarkVirtualBaseDestructorsReferenced(
SourceLocation Location, CXXRecordDecl *ClassDecl,
- llvm::SmallPtrSetImpl<const RecordType *> *DirectVirtualBases = nullptr);
+ llvm::SmallPtrSetImpl<const CXXRecordDecl *> *DirectVirtualBases =
+ nullptr);
/// Do semantic checks to allow the complete destructor variant to be emitted
/// when the destructor is defined in another translation unit. In the Itanium
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 54a12daec472d..e8e8e939ee025 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -5283,6 +5283,17 @@ Sema::SetDelegatingInitializer(CXXConstructorDecl *Constructor,
return false;
}
+static CXXDestructorDecl *
+LookupDestructorIfRelevant(Sema &S, CXXRecordDecl *Class) {
+ if (Class->isInvalidDecl())
+ return nullptr;
+ if (Class->hasIrrelevantDestructor())
+ return nullptr;
+
+ // Dtor might still be missing, e.g because it's invalid.
+ return S.LookupDestructor(Class);
+}
+
static void MarkFieldDestructorReferenced(Sema &S, SourceLocation Location,
FieldDecl *Field) {
if (Field->isInvalidDecl())
@@ -5294,23 +5305,18 @@ static void MarkFieldDestructorReferenced(Sema &S, SourceLocation Location,
QualType FieldType = S.Context.getBaseElementType(Field->getType());
- const auto *RT = FieldType->getAs<RecordType>();
- if (!RT)
+ auto *FieldClassDecl = FieldType->getAsCXXRecordDecl();
+ if (!FieldClassDecl)
return;
- auto *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 = S.LookupDestructor(FieldClassDecl);
- // Dtor might still be missing, e.g because it's invalid.
+ auto *Dtor = LookupDestructorIfRelevant(S, FieldClassDecl);
if (!Dtor)
return;
+
S.CheckDestructorAccess(Field->getLocation(), Dtor,
S.PDiag(diag::err_access_dtor_field)
<< Field->getDeclName() << FieldType);
@@ -5337,7 +5343,7 @@ static void MarkBaseDestructorsReferenced(Sema &S, SourceLocation Location,
VisitVirtualBases = false;
}
- llvm::SmallPtrSet<const RecordType *, 8> DirectVirtualBases;
+ llvm::SmallPtrSet<const CXXRecordDecl *, 8> DirectVirtualBases;
// Bases.
for (const auto &Base : ClassDecl->bases()) {
@@ -5345,22 +5351,16 @@ static void MarkBaseDestructorsReferenced(Sema &S, SourceLocation Location,
if (!RT)
continue;
+ auto *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
+
// Remember direct virtual bases.
if (Base.isVirtual()) {
if (!VisitVirtualBases)
continue;
- DirectVirtualBases.insert(RT);
+ DirectVirtualBases.insert(BaseClassDecl);
}
- auto *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
- // If our base class is invalid, we probably can't get its dtor anyway.
- if (BaseClassDecl->isInvalidDecl())
- continue;
- if (BaseClassDecl->hasIrrelevantDestructor())
- continue;
-
- CXXDestructorDecl *Dtor = S.LookupDestructor(BaseClassDecl);
- // Dtor might still be missing, e.g because it's invalid.
+ auto *Dtor = LookupDestructorIfRelevant(S, BaseClassDecl);
if (!Dtor)
continue;
@@ -5890,27 +5890,21 @@ void Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
void Sema::MarkVirtualBaseDestructorsReferenced(
SourceLocation Location, CXXRecordDecl *ClassDecl,
- llvm::SmallPtrSetImpl<const RecordType *> *DirectVirtualBases) {
+ llvm::SmallPtrSetImpl<const CXXRecordDecl *> *DirectVirtualBases) {
// Virtual bases.
for (const auto &VBase : ClassDecl->vbases()) {
- // Bases are always records in a well-formed non-dependent class.
- const RecordType *RT = VBase.getType()->castAs<RecordType>();
-
- // Ignore already visited direct virtual bases.
- if (DirectVirtualBases && DirectVirtualBases->count(RT))
+ auto *BaseClassDecl = VBase.getType()->getAsCXXRecordDecl();
+ if (!BaseClassDecl)
continue;
- CXXRecordDecl *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
- // If our base class is invalid, we probably can't get its dtor anyway.
- if (BaseClassDecl->isInvalidDecl())
- continue;
- if (BaseClassDecl->hasIrrelevantDestructor())
+ // Ignore already visited direct virtual bases.
+ if (DirectVirtualBases && DirectVirtualBases->count(BaseClassDecl))
continue;
- CXXDestructorDecl *Dtor = LookupDestructor(BaseClassDecl);
- // Dtor might still be missing, e.g because it's invalid.
+ auto *Dtor = LookupDestructorIfRelevant(*this, BaseClassDecl);
if (!Dtor)
continue;
+
if (CheckDestructorAccess(
ClassDecl->getLocation(), Dtor,
PDiag(diag::err_access_dtor_vbase)
More information about the cfe-commits
mailing list