Author: Maurice Heumann Date: 2025-03-13T09:34:02+01:00 New Revision: cb28ec6cccf9e27eb74f8dc2e7b69c006c1e3544
URL: https://github.com/llvm/llvm-project/commit/cb28ec6cccf9e27eb74f8dc2e7b69c006c1e3544 DIFF: https://github.com/llvm/llvm-project/commit/cb28ec6cccf9e27eb74f8dc2e7b69c006c1e3544.diff LOG: [Sema] Instantiate destructors for initialized members (#128866) Initializing fields, that are part of an anonymous union, in a constructor, requires their destructors to be instantiated. In general, initialized members within non-delegating constructors, need their destructor instantiated. This fixes #93251 Added: clang/test/SemaCXX/union-member-destructor.cpp Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDeclCXX.cpp clang/test/SemaCXX/cxx0x-nontrivial-union.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index e587c6c7b74f2..8989124611e66 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -309,6 +309,7 @@ Bug Fixes to C++ Support not in the last position. - Clang now correctly parses ``if constexpr`` expressions in immediate function context. (#GH123524) - Fixed an assertion failure affecting code that uses C++23 "deducing this". (#GH130272) +- Clang now properly instantiates destructors for initialized members within non-delegating constructors. (#GH93251) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 9ac26d8728446..b6bf1e468ef2c 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5451,7 +5451,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 1c62a551ee732..dd779ee377309 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -5289,6 +5289,100 @@ 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()) + return; + + // Don't destroy incomplete or zero-length arrays. + if (isIncompleteOrZeroLengthArrayType(S.Context, Field->getType())) + return; + + QualType FieldType = S.Context.getBaseElementType(Field->getType()); + + auto *FieldClassDecl = FieldType->getAsCXXRecordDecl(); + if (!FieldClassDecl) + return; + + // The destructor for an implicit anonymous union member is never invoked. + if (FieldClassDecl->isUnion() && FieldClassDecl->isAnonymousStructOrUnion()) + return; + + auto *Dtor = LookupDestructorIfRelevant(S, FieldClassDecl); + 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 CXXRecordDecl *, 8> DirectVirtualBases; + + // Bases. + for (const auto &Base : ClassDecl->bases()) { + auto *BaseClassDecl = Base.getType()->getAsCXXRecordDecl(); + if (!BaseClassDecl) + continue; + + // Remember direct virtual bases. + if (Base.isVirtual()) { + if (!VisitVirtualBases) + continue; + DirectVirtualBases.insert(BaseClassDecl); + } + + auto *Dtor = LookupDestructorIfRelevant(S, BaseClassDecl); + 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()) { @@ -5456,10 +5550,24 @@ bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors, NumInitializers * sizeof(CXXCtorInitializer*)); Constructor->setCtorInitializers(baseOrMemberInitializers); + SourceLocation Location = Constructor->getLocation(); + // Constructors implicitly reference the base and member // destructors. - MarkBaseAndMemberDestructorsReferenced(Constructor->getLocation(), - Constructor->getParent()); + + for (CXXCtorInitializer *Initializer : Info.AllToInit) { + FieldDecl *Field = Initializer->getAnyMember(); + 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. + MarkFieldDestructorReferenced(*this, Location, Field); + } + + MarkBaseDestructorsReferenced(*this, Location, Constructor->getParent()); } return HadError; @@ -5764,9 +5872,8 @@ void Sema::ActOnMemInitializers(Decl *ConstructorDecl, DiagnoseUninitializedFields(*this, Constructor); } -void -Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location, - CXXRecordDecl *ClassDecl) { +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()) @@ -5779,119 +5886,29 @@ 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); - } - - // 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; + MarkFieldDestructorReferenced(*this, Location, Field); } - 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); + MarkBaseDestructorsReferenced(*this, Location, ClassDecl); } 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) 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..7305290193d8c --- /dev/null +++ b/clang/test/SemaCXX/union-member-destructor.cpp @@ -0,0 +1,50 @@ +// 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::VS() : VS(0) { } // delegating constructors should not produce errors +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}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
