https://github.com/momo5502 updated https://github.com/llvm/llvm-project/pull/128866
>From f23cf926c4dbf934971e5f4f8b40105e3b41bb0f Mon Sep 17 00:00:00 2001 From: Maurice Heumann <maurice.heum...@wibu.com> Date: Wed, 26 Feb 2025 14:31:47 +0100 Subject: [PATCH 1/6] 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 80177996b48b0..0afff5dd8d415 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5439,6 +5439,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 27c321055411edcbe101232745ff317c389be0b6 Mon Sep 17 00:00:00 2001 From: Maurice Heumann <maurice.heum...@wibu.com> Date: Mon, 3 Mar 2025 08:52:55 +0100 Subject: [PATCH 2/6] 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 0afff5dd8d415..683a1f19f884e 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5439,6 +5439,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 b2f4595653b6f84b098dcf2df6bf64c12f4761d9 Mon Sep 17 00:00:00 2001 From: Maurice Heumann <maurice.heum...@wibu.com> Date: Wed, 5 Mar 2025 08:23:25 +0100 Subject: [PATCH 3/6] 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 94d46ba84bbe4d040cdce1ae5cff10aa81fb125d Mon Sep 17 00:00:00 2001 From: Maurice Heumann <maurice.heum...@wibu.com> Date: Wed, 5 Mar 2025 08:49:20 +0100 Subject: [PATCH 4/6] 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 86bf836b4a999..a15673c9a488b 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -279,6 +279,7 @@ Bug Fixes to C++ Support direct-list-initialized from an array is corrected to direct-initialization. - Clang no longer crashes when a coroutine is declared ``[[noreturn]]``. (#GH127327) - Clang now uses the parameter location for abbreviated function templates in ``extern "C"``. (#GH46386) +- Clang now properly instantiates destructors for initialized members within non-delegating constructors. (#GH93251) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ >From 3acb02ad7bacfe1eb62ea01de15815b208b64410 Mon Sep 17 00:00:00 2001 From: Maurice Heumann <maurice.heum...@wibu.com> Date: Wed, 5 Mar 2025 11:51:25 +0100 Subject: [PATCH 5/6] 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 683a1f19f884e..80177996b48b0 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5439,9 +5439,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 fd9a337d45563c33016a8fb46277d15c9a455431 Mon Sep 17 00:00:00 2001 From: Maurice Heumann <maurice.heum...@wibu.com> Date: Thu, 6 Mar 2025 10:04:43 +0100 Subject: [PATCH 6/6] Extract duplicated destructor lookups --- clang/include/clang/Sema/Sema.h | 3 +- clang/lib/Sema/SemaDeclCXX.cpp | 65 ++++++++++++++------------------- 2 files changed, 30 insertions(+), 38 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 80177996b48b0..414da47a53a25 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5445,7 +5445,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 a12535ffc53e7..33cab36c0dae9 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 RecordType *RT = FieldType->getAs<RecordType>(); - if (!RT) + auto *FieldClassDecl = FieldType->getAsCXXRecordDecl(); + if (!FieldClassDecl) 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. + auto *Dtor = LookupDestructorIfRelevant(S, FieldClassDecl); if (!Dtor) return; + S.CheckDestructorAccess(Field->getLocation(), Dtor, S.PDiag(diag::err_access_dtor_field) << Field->getDeclName() << FieldType); @@ -5337,30 +5343,22 @@ 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()) { - const RecordType *RT = Base.getType()->getAs<RecordType>(); - if (!RT) + auto *BaseClassDecl = Base.getType()->getAsCXXRecordDecl(); + if (!BaseClassDecl) continue; // Remember direct virtual bases. if (Base.isVirtual()) { if (!VisitVirtualBases) continue; - DirectVirtualBases.insert(RT); + DirectVirtualBases.insert(BaseClassDecl); } - 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. + auto *Dtor = LookupDestructorIfRelevant(S, BaseClassDecl); if (!Dtor) continue; @@ -5559,8 +5557,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); } @@ -5891,27 +5888,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) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits