https://github.com/Fznamznon created https://github.com/llvm/llvm-project/pull/188218
Reverts llvm/llvm-project#185653 This is problematic likely due to missing support for weak aliases in CFG table emission. Revert to unblock people until this is fixed. >From e50b687b1517acbe07cca12e5a330a7e978f4afb Mon Sep 17 00:00:00 2001 From: Mariya Podchishchaeva <[email protected]> Date: Tue, 24 Mar 2026 11:23:27 +0100 Subject: [PATCH] =?UTF-8?q?Revert=20"[clang][win]=20Define=20vector=20dele?= =?UTF-8?q?ting=20dtor=20body=20for=20declared-only=20dtor=20=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 9b5084f894cb941d76b4638c1305bf3f869c78d4. --- clang/include/clang/AST/ASTContext.h | 9 +- clang/lib/AST/ASTContext.cpp | 16 ++- clang/lib/CodeGen/CGExprCXX.cpp | 6 - clang/lib/CodeGen/CodeGenModule.cpp | 52 -------- clang/lib/CodeGen/CodeGenModule.h | 12 -- clang/lib/CodeGen/MicrosoftCXXABI.cpp | 2 +- clang/lib/Sema/SemaDeclCXX.cpp | 10 +- clang/lib/Sema/SemaExprCXX.cpp | 36 ++---- ...rosoft-vector-deleting-dtors-new-array.cpp | 122 ------------------ clang/test/SemaCXX/gh134265.cpp | 31 +---- 10 files changed, 30 insertions(+), 266 deletions(-) delete mode 100644 clang/test/CodeGenCXX/microsoft-vector-deleting-dtors-new-array.cpp diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 40091acd878fd..560248ea2224e 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -389,9 +389,8 @@ class ASTContext : public RefCountedBase<ASTContext> { mutable llvm::DenseMap<const CXXDestructorDecl *, FunctionDecl *> GlobalArrayOperatorDeletesForVirtualDtor; - /// To remember for which types we met new[] call, these potentially require a - /// vector deleting dtor. - llvm::DenseSet<const CXXRecordDecl *> MaybeRequireVectorDeletingDtor; + /// To remember which types did require a vector deleting dtor. + llvm::DenseSet<const CXXRecordDecl *> RequireVectorDeletingDtor; /// The next string literal "version" to allocate during constant evaluation. /// This is used to distinguish between repeated evaluations of the same @@ -3549,8 +3548,8 @@ class ASTContext : public RefCountedBase<ASTContext> { OperatorDeleteKind K) const; bool dtorHasOperatorDelete(const CXXDestructorDecl *Dtor, OperatorDeleteKind K) const; - void setClassMaybeNeedsVectorDeletingDestructor(const CXXRecordDecl *RD); - bool classMaybeNeedsVectorDeletingDestructor(const CXXRecordDecl *RD); + void setClassNeedsVectorDeletingDestructor(const CXXRecordDecl *RD); + bool classNeedsVectorDeletingDestructor(const CXXRecordDecl *RD); /// Retrieve the context for computing mangling numbers in the given /// DeclContext. diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index a8cfdca1cb96d..1e79ee4bc5bcb 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -13626,20 +13626,24 @@ ASTContext::getOperatorDeleteForVDtor(const CXXDestructorDecl *Dtor, return nullptr; } -bool ASTContext::classMaybeNeedsVectorDeletingDestructor( - const CXXRecordDecl *RD) { +bool ASTContext::classNeedsVectorDeletingDestructor(const CXXRecordDecl *RD) { if (!getTargetInfo().emitVectorDeletingDtors(getLangOpts())) return false; + CXXDestructorDecl *Dtor = RD->getDestructor(); + // The compiler can't know if new[]/delete[] will be used outside of the DLL, + // so just force vector deleting destructor emission if dllexport is present. + // This matches MSVC behavior. + if (Dtor && Dtor->isVirtual() && Dtor->hasAttr<DLLExportAttr>()) + return true; - return MaybeRequireVectorDeletingDtor.count(RD); + return RequireVectorDeletingDtor.count(RD); } -void ASTContext::setClassMaybeNeedsVectorDeletingDestructor( +void ASTContext::setClassNeedsVectorDeletingDestructor( const CXXRecordDecl *RD) { if (!getTargetInfo().emitVectorDeletingDtors(getLangOpts())) return; - - MaybeRequireVectorDeletingDtor.insert(RD); + RequireVectorDeletingDtor.insert(RD); } MangleNumberingContext & diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 82300c3ede183..93ac0305df38f 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1201,12 +1201,6 @@ void CodeGenFunction::EmitNewArrayInitializer( EmitCXXAggrConstructorCall(Ctor, NumElements, CurPtr, CCE, /*NewPointerIsChecked*/ true, CCE->requiresZeroInitialization()); - if (getContext().getTargetInfo().emitVectorDeletingDtors( - getContext().getLangOpts())) { - CXXDestructorDecl *Dtor = Ctor->getParent()->getDestructor(); - if (Dtor && Dtor->isVirtual()) - CGM.requireVectorDestructorDefinition(Ctor->getParent()); - } return; } diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index daaa846bf42bc..3658d84f55256 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -8561,55 +8561,3 @@ std::string CodeGenModule::getPFPFieldName(const FieldDecl *FD) { Out << "." << FD->getName(); return OutName; } - -bool CodeGenModule::classNeedsVectorDestructor(const CXXRecordDecl *RD) { - if (!Context.getTargetInfo().emitVectorDeletingDtors(Context.getLangOpts())) - return false; - CXXDestructorDecl *Dtor = RD->getDestructor(); - // The compiler can't know if new[]/delete[] will be used outside of the DLL, - // so just force vector deleting destructor emission if dllexport is present. - // This matches MSVC behavior. - if (Dtor && Dtor->isVirtual() && Dtor->hasAttr<DLLExportAttr>()) - return true; - - return RequireVectorDeletingDtor.count(RD); -} - -void CodeGenModule::requireVectorDestructorDefinition(const CXXRecordDecl *RD) { - if (!Context.getTargetInfo().emitVectorDeletingDtors(Context.getLangOpts())) - return; - RequireVectorDeletingDtor.insert(RD); - - // To reduce code size in general case we lazily emit scalar deleting - // destructor definition and an alias from vector deleting destructor to - // scalar deleting destructor. It may happen that we first emitted the scalar - // deleting destructor definition and the alias and then discovered that the - // definition of the vector deleting destructor is required. Then we need to - // remove the alias and the scalar deleting destructor and queue vector - // deleting destructor body for emission. Check if that is the case. - CXXDestructorDecl *DtorD = RD->getDestructor(); - GlobalDecl ScalarDtorGD(DtorD, Dtor_Deleting); - StringRef MangledName = getMangledName(ScalarDtorGD); - llvm::GlobalValue *Entry = GetGlobalValue(MangledName); - GlobalDecl VectorDtorGD(DtorD, Dtor_VectorDeleting); - if (Entry && !Entry->isDeclaration()) { - StringRef VDName = getMangledName(VectorDtorGD); - llvm::GlobalValue *VDEntry = GetGlobalValue(VDName); - // It exists and it should be an alias. - assert(VDEntry && isa<llvm::GlobalAlias>(VDEntry)); - auto *NewFn = llvm::Function::Create( - cast<llvm::FunctionType>(VDEntry->getValueType()), - llvm::Function::ExternalLinkage, VDName, &getModule()); - SetFunctionAttributes(VectorDtorGD, NewFn, /*IsIncompleteFunction*/ false, - /*IsThunk*/ false); - NewFn->takeName(VDEntry); - VDEntry->replaceAllUsesWith(NewFn); - VDEntry->eraseFromParent(); - Entry->replaceAllUsesWith(NewFn); - Entry->eraseFromParent(); - } - // Always add a deferred decl to emit once we confirmed that vector deleting - // destructor definition is required. That helps to enforse its generation - // even if destructor is only declared. - addDeferredDeclToEmit(VectorDtorGD); -} diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 0a697c84b66a7..0081bf5c4cf5f 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -529,11 +529,6 @@ class CodeGenModule : public CodeGenTypeCache { /// that we don't re-emit the initializer. llvm::DenseMap<const Decl*, unsigned> DelayedCXXInitPosition; - /// To remember which types did require a vector deleting destructor body. - /// This set basically contains classes that have virtual destructor and new[] - /// was emitted for the class. - llvm::SmallPtrSet<const CXXRecordDecl *, 16> RequireVectorDeletingDtor; - typedef std::pair<OrderGlobalInitsOrStermFinalizers, llvm::Function *> GlobalInitData; @@ -1583,13 +1578,6 @@ class CodeGenModule : public CodeGenTypeCache { /// are emitted lazily. void EmitGlobal(GlobalDecl D); - /// Record that new[] was called for the class, transform vector deleting - /// destructor definition in a form of alias to the actual definition. - void requireVectorDestructorDefinition(const CXXRecordDecl *RD); - - /// Check that class need vector deleting destructor body. - bool classNeedsVectorDestructor(const CXXRecordDecl *RD); - bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D); void EmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target); diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index d959b89f860e4..06fce6171eb28 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -4107,7 +4107,7 @@ void MicrosoftCXXABI::emitCXXStructor(GlobalDecl GD) { return; if (GD.getDtorType() == Dtor_VectorDeleting && - !CGM.classNeedsVectorDestructor(dtor->getParent())) { + !getContext().classNeedsVectorDeletingDestructor(dtor->getParent())) { // Create GlobalDecl object with the correct type for the scalar // deleting destructor. GlobalDecl ScalarDtorGD(dtor, Dtor_Deleting); diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 441df43d3d184..0591d05d361c0 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -11268,7 +11268,6 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) { if (Context.getTargetInfo().emitVectorDeletingDtors( Context.getLangOpts())) { - bool DestructorIsExported = Destructor->hasAttr<DLLExportAttr>(); // Lookup delete[] too in case we have to emit a vector deleting dtor. DeclarationName VDeleteName = Context.DeclarationNames.getCXXOperatorName(OO_Array_Delete); @@ -11282,8 +11281,7 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) { VDeleteName); Destructor->setGlobalOperatorArrayDelete(GlobalArrOperatorDelete); if (GlobalArrOperatorDelete && - (Context.classMaybeNeedsVectorDeletingDestructor(RD) || - DestructorIsExported)) + Context.classNeedsVectorDeletingDestructor(RD)) MarkFunctionReferenced(Loc, GlobalArrOperatorDelete); } else if (!ArrOperatorDelete) { ArrOperatorDelete = FindDeallocationFunctionForDestructor( @@ -11291,9 +11289,7 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) { /*LookForGlobal*/ true, VDeleteName); } Destructor->setOperatorArrayDelete(ArrOperatorDelete); - if (ArrOperatorDelete && - (Context.classMaybeNeedsVectorDeletingDestructor(RD) || - DestructorIsExported)) + if (ArrOperatorDelete && Context.classNeedsVectorDeletingDestructor(RD)) MarkFunctionReferenced(Loc, ArrOperatorDelete); } } @@ -19132,8 +19128,6 @@ void Sema::MarkVTableUsed(SourceLocation Loc, CXXRecordDecl *Class, // delete(). ContextRAII SavedContext(*this, DD); CheckDestructor(DD); - if (!DD->getOperatorDelete()) - DD->setInvalidDecl(); } else { MarkFunctionReferenced(Loc, Class->getDestructor()); } diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 08bc6aa483844..8a095b2a89cb7 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2636,31 +2636,17 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, MarkFunctionReferenced(StartLoc, OperatorDelete); } - // new[] will trigger vector deleting destructor emission if the class has - // virtual destructor for MSVC compatibility. Perform necessary checks. - if (Context.getTargetInfo().emitVectorDeletingDtors(Context.getLangOpts())) { - if (const CXXConstructExpr *CCE = - dyn_cast_or_null<CXXConstructExpr>(Initializer); - CCE && ArraySize) { - CXXRecordDecl *ClassDecl = CCE->getConstructor()->getParent(); - // We probably already did this for another new[] with this class so don't - // do it twice. - if (!Context.classMaybeNeedsVectorDeletingDestructor(ClassDecl)) { - auto *Dtor = ClassDecl->getDestructor(); - if (Dtor && Dtor->isVirtual() && !Dtor->isDeleted()) { - Context.setClassMaybeNeedsVectorDeletingDestructor(ClassDecl); - if (!Dtor->isDefined() && !Dtor->isInvalidDecl()) { - // Call CheckDestructor if destructor is not defined. This is - // needed to find operators delete and delete[] for vector deleting - // destructor body because new[] will trigger emission of vector - // deleting destructor body even if destructor is defined in another - // translation unit. - ContextRAII SavedContext(*this, Dtor); - CheckDestructor(Dtor); - } - } - } - } + // For MSVC vector deleting destructors support we record that for the class + // new[] was called. We try to optimize the code size and only emit vector + // deleting destructors when they are required. Vector deleting destructors + // are required for delete[] call but MSVC triggers emission of them + // whenever new[] is called for an object of the class and we do the same + // for compatibility. + if (const CXXConstructExpr *CCE = + dyn_cast_or_null<CXXConstructExpr>(Initializer); + CCE && ArraySize) { + Context.setClassNeedsVectorDeletingDestructor( + CCE->getConstructor()->getParent()); } return CXXNewExpr::Create(Context, UseGlobal, OperatorNew, OperatorDelete, diff --git a/clang/test/CodeGenCXX/microsoft-vector-deleting-dtors-new-array.cpp b/clang/test/CodeGenCXX/microsoft-vector-deleting-dtors-new-array.cpp deleted file mode 100644 index b8b6e44b6b2f8..0000000000000 --- a/clang/test/CodeGenCXX/microsoft-vector-deleting-dtors-new-array.cpp +++ /dev/null @@ -1,122 +0,0 @@ -// RUN: %clang_cc1 -emit-llvm -fms-extensions %s -triple=x86_64-pc-windows-msvc -o - | FileCheck %s - -// Test that vector deleting destructors are emitted when new[] is used, -// even when the destructor definition is in another translation unit. - -struct ForwardDeclared { - ForwardDeclared(); - virtual ~ForwardDeclared(); -}; - -struct DefinedInTU { - virtual ~DefinedInTU(); -}; - -struct NonVirtualDtor { - ~NonVirtualDtor(); -}; - -struct NoDtor { - virtual void foo(); - int x; -}; - -struct DeclDerived : ForwardDeclared { - ~DeclDerived() override; -}; - -struct InlineDefaulted { - virtual ~InlineDefaulted() = default; -}; - -struct OutOfLineDefaulted { - virtual ~OutOfLineDefaulted(); -}; - -OutOfLineDefaulted::~OutOfLineDefaulted() = default; - -template<typename T> -struct Container { - T data; - virtual ~Container(); -}; - -extern template class Container<int>; -Container<int> *arr = new Container<int>[5]; - -struct ImplicitVDtorDerived : ForwardDeclared{ - int data; -}; - -struct __declspec(dllimport) DllImported { - virtual ~DllImported(); -}; - -struct VirtualDerived : virtual ForwardDeclared { - ~VirtualDerived() override; -}; - -struct DeclaredCtorDefinedDtor { - DeclaredCtorDefinedDtor(); - virtual ~DeclaredCtorDefinedDtor() {} -}; - -struct TemplateNotAllocated { - TemplateNotAllocated(); - virtual ~TemplateNotAllocated(); -}; - -struct TemplateAllocated { - TemplateAllocated(); - virtual ~TemplateAllocated(); -}; - -template <int T> -void allocate() { - TemplateNotAllocated *arr = new TemplateNotAllocated[T]; -} - -template <typename T> -void actuallyAllocate() { - T *arr = new T[10]; - delete[] arr; -} - -void cases() { - ForwardDeclared *arr = new ForwardDeclared[5]; - DefinedInTU *arr1 = new DefinedInTU[5]; - NonVirtualDtor *arr2 = new NonVirtualDtor[5]; - NoDtor *arr3 = new NoDtor[5]; - ForwardDeclared *arr4 = new DeclDerived[5]; - InlineDefaulted *arr5 = new InlineDefaulted[5]; - OutOfLineDefaulted *arr6 = new OutOfLineDefaulted[5]; - ImplicitVDtorDerived *arr7 = new ImplicitVDtorDerived[5]; - DllImported *arr8 = new DllImported[5]; - VirtualDerived *arr9 = new VirtualDerived[3]; - DeclaredCtorDefinedDtor *arr10 = new DeclaredCtorDefinedDtor[5]; - actuallyAllocate<TemplateAllocated>(); -} - - -// CHECK-DAG: declare dso_local void @"??1ForwardDeclared@@UEAA@XZ"( -// CHECK-DAG: define weak dso_local noundef ptr @"??_EForwardDeclared@@UEAAPEAXI@Z"( -// CHECK-DAG: define dso_local void @"??1DefinedInTU@@UEAA@XZ"( -// CHECK-DAG: define weak dso_local noundef ptr @"??_EDefinedInTU@@UEAAPEAXI@Z"( -// CHECK-DAG: define weak dso_local noundef ptr @"??_EDeclDerived@@UEAAPEAXI@Z"( -// CHECK-DAG: declare dso_local void @"??1DeclDerived@@UEAA@XZ"( -// CHECK-DAG: define weak dso_local noundef ptr @"??_EInlineDefaulted@@UEAAPEAXI@Z"( -// CHECK-DAG: define weak dso_local noundef ptr @"??_EOutOfLineDefaulted@@UEAAPEAXI@Z"( -// CHECK-DAG: declare dso_local void @"??1?$Container@H@@UEAA@XZ"( -// CHECK-DAG: define weak dso_local noundef ptr @"??_E?$Container@H@@UEAAPEAXI@Z"( -// CHECK-DAG: define weak dso_local noundef ptr @"??_EImplicitVDtorDerived@@UEAAPEAXI@Z"( -// CHECK-DAG: declare dllimport void @"??1DllImported@@UEAA@XZ"( -// CHECK-DAG: define weak dso_local noundef ptr @"??_EDllImported@@UEAAPEAXI@Z"( -// CHECK-DAG: define weak dso_local noundef ptr @"??_EVirtualDerived@@UEAAPEAXI@Z"( -// CHECK-DAG: define weak dso_local noundef ptr @"??_EDeclaredCtorDefinedDtor@@UEAAPEAXI@Z"( -// CHECK-DAG: declare dso_local void @"??1TemplateAllocated@@UEAA@XZ"( -// CHECK-DAG: define weak dso_local noundef ptr @"??_ETemplateAllocated@@UEAAPEAXI@Z"( -// CHECK-NOT: @"??_ETemplateNotAllocated@@ -// CHECK-NOT: @"??_ENonVirtualDtor@@ -// CHECK-NOT: @"??_ENoDtor@@ - -DefinedInTU::~DefinedInTU() {} diff --git a/clang/test/SemaCXX/gh134265.cpp b/clang/test/SemaCXX/gh134265.cpp index 421197cf3d7f7..790165411c938 100644 --- a/clang/test/SemaCXX/gh134265.cpp +++ b/clang/test/SemaCXX/gh134265.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 %s -verify=expected,noms -fsyntax-only -triple=x86_64-unknown-linux-gnu -// RUN: %clang_cc1 %s -verify=expected,noms -fsyntax-only -triple=x86_64-unknown-linux-gnu -std=c++20 +// RUN: %clang_cc1 %s -verify=expected -fsyntax-only -triple=x86_64-unknown-linux-gnu +// RUN: %clang_cc1 %s -verify=expected -fsyntax-only -triple=x86_64-unknown-linux-gnu -std=c++20 // RUN: %clang_cc1 %s -verify=expected,ms -fms-extensions -fms-compatibility -triple=x86_64-pc-windows-msvc -DMS // Verify that clang doesn't emit additional errors when searching for @@ -56,34 +56,7 @@ struct Final1 : BaseDelete1, BaseDelete2, BaseDestructor { }; #endif // MS -// Make sure there is no double diagnosing for declared-only destructors and -// new[]. -struct DeclaredOnly { - virtual ~DeclaredOnly(); // ms-error {{attempt to use a deleted function}} - static void operator delete(void* ptr) = delete; // ms-note {{explicitly marked deleted here}} -}; - -struct DeclaredOnlyArr { - virtual ~DeclaredOnlyArr(); - static void operator delete[](void* ptr) = delete; -}; - void foo() { Final* a = new Final[10](); FinalExplicit* b = new FinalExplicit[10](); - DeclaredOnly *d = new DeclaredOnly[5](); - DeclaredOnlyArr *e = new DeclaredOnlyArr[5](); } - -// Make sure there is no double diagnosing for forward declared destructors -// and new[]. -namespace std { struct destroying_delete_t {}; } -struct A { - void operator delete( - A*, //expected-error {{cannot cast 'D' to its private base class 'A'}} - std::destroying_delete_t); -}; -struct B : private A { using A::operator delete; }; //expected-note {{declared private here}} -struct D : B { virtual ~D(); }; //ms-note {{while checking implicit 'delete this' for virtual destructor}} -void f() { new D[5]; } -D::~D() {} // noms-note {{while checking implicit 'delete this' for virtual destructor}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
