https://github.com/Fznamznon updated https://github.com/llvm/llvm-project/pull/135041
>From 9e923b1fe683dea499f7557a48d97aa6d2469c07 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <mariya.podchishcha...@intel.com> Date: Wed, 9 Apr 2025 09:02:17 -0700 Subject: [PATCH 1/5] [MS][clang] Error about ambigous operator delete[] only when required And issue was reported in https://github.com/llvm/llvm-project/pull/133950 . Since we don't always emit vector deleting dtors, only error out about ambigous operator delete[] when it will be required for vector delering dtor emission. --- clang/lib/Sema/SemaDeclCXX.cpp | 21 ++++++++++-------- clang/lib/Sema/SemaExprCXX.cpp | 18 +++++++++++++-- clang/test/SemaCXX/gh134265.cpp | 39 ++++++++++++++++++++++++++++++--- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index b86f7118e0b34..6732062965aef 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -11045,15 +11045,18 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) { DiagnoseUseOfDecl(OperatorDelete, Loc); MarkFunctionReferenced(Loc, OperatorDelete); Destructor->setOperatorDelete(OperatorDelete, ThisArg); - // Lookup delete[] too in case we have to emit a vector deleting dtor; - DeclarationName VDeleteName = - Context.DeclarationNames.getCXXOperatorName(OO_Array_Delete); - FunctionDecl *ArrOperatorDelete = FindDeallocationFunctionForDestructor( - Loc, RD, VDeleteName, /*Diagnose=*/false); - // delete[] in the TU will make sure the operator is referenced and its - // uses diagnosed, otherwise vector deleting dtor won't be called anyway, - // so just record it in the destructor. - Destructor->setOperatorArrayDelete(ArrOperatorDelete); + if (Context.getTargetInfo().getCXXABI().isMicrosoft()) { + // Lookup delete[] too in case we have to emit a vector deleting dtor; + DeclarationName VDeleteName = + Context.DeclarationNames.getCXXOperatorName(OO_Array_Delete); + FunctionDecl *ArrOperatorDelete = FindDeallocationFunctionForDestructor( + Loc, RD, VDeleteName, + Destructor->hasAttr<DLLExportAttr>() && Destructor->isDefined()); + // delete[] in the TU will make sure the operator is referenced and its + // uses diagnosed, otherwise vector deleting dtor won't be called + // anyway, so just record it in the destructor. + Destructor->setOperatorArrayDelete(ArrOperatorDelete); + } } } diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 42bc4db20e818..247cd02b23522 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2789,8 +2789,19 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range, return true; } + // new[] will force emission of vector deleting dtor which needs delete[]. + bool MaybeVectorDeletingDtor = false; + if (Context.getTargetInfo().getCXXABI().isMicrosoft()) { + if (AllocElemType->isRecordType() && IsArray) { + auto *RD = + cast<CXXRecordDecl>(AllocElemType->castAs<RecordType>()->getDecl()); + CXXDestructorDecl *DD = RD->getDestructor(); + MaybeVectorDeletingDtor = DD && DD->isVirtual() && !DD->isDeleted(); + } + } + // We don't need an operator delete if we're running under -fno-exceptions. - if (!getLangOpts().Exceptions) { + if (!getLangOpts().Exceptions && !MaybeVectorDeletingDtor) { OperatorDelete = nullptr; return false; } @@ -3290,8 +3301,11 @@ bool Sema::FindDeallocationFunction(SourceLocation StartLoc, CXXRecordDecl *RD, // Try to find operator delete/operator delete[] in class scope. LookupQualifiedName(Found, RD); - if (Found.isAmbiguous()) + if (Found.isAmbiguous()) { + if (!Diagnose) + Found.suppressDiagnostics(); return true; + } Found.suppressDiagnostics(); diff --git a/clang/test/SemaCXX/gh134265.cpp b/clang/test/SemaCXX/gh134265.cpp index c7bdeb2add0cc..a2d360d7ba02a 100644 --- a/clang/test/SemaCXX/gh134265.cpp +++ b/clang/test/SemaCXX/gh134265.cpp @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 %s -verify -fsyntax-only +// RUN: %clang_cc1 %s -verify=expected -fsyntax-only +// RUN: %clang_cc1 %s -verify=expected -fsyntax-only -std=c++20 +// RUN: %clang_cc1 %s -verify=expected,ms -fms-extensions -fms-compatibility -triple=x86_64-pc-windows-msvc -DMS struct Foo { virtual ~Foo() {} // expected-error {{attempt to use a deleted function}} @@ -13,10 +15,41 @@ struct Bar { struct Baz { virtual ~Baz() {} - static void operator delete[](void* ptr) = delete; // expected-note {{explicitly marked deleted here}} + static void operator delete[](void* ptr) = delete; // expected-note {{explicitly marked deleted here}}\ + ms-note{{explicitly marked deleted here}}} +}; + +struct BarBaz { + ~BarBaz() {} + static void operator delete[](void* ptr) = delete; }; void foobar() { - Baz *B = new Baz[10](); + Baz *B = new Baz[10](); // ms-error {{attempt to use a deleted function}} delete [] B; // expected-error {{attempt to use a deleted function}} + BarBaz *BB = new BarBaz[10](); +} + +struct BaseDelete1 { + void operator delete[](void *); //ms-note 2{{member found by ambiguous name lookup}} +}; +struct BaseDelete2 { + void operator delete[](void *); //ms-note 2{{member found by ambiguous name lookup}} +}; +struct BaseDestructor { + BaseDestructor() {} + virtual ~BaseDestructor() = default; +}; +struct Final : BaseDelete1, BaseDelete2, BaseDestructor { + Final() {} +}; + +#ifdef MS +struct Final1 : BaseDelete1, BaseDelete2, BaseDestructor { + __declspec(dllexport) ~Final1() {} // ms-error {{member 'operator delete[]' found in multiple base classes of different types}} +}; +#endif // MS + +void foo() { + Final* a = new Final[10](); // ms-error {{member 'operator delete[]' found in multiple base classes of different types}} } >From a66bc6c0c4934a95d233e82b3e5a0705dde74467 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <mariya.podchishcha...@intel.com> Date: Thu, 10 Apr 2025 01:06:28 -0700 Subject: [PATCH 2/5] Correct the test --- clang/test/SemaCXX/gh134265.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/SemaCXX/gh134265.cpp b/clang/test/SemaCXX/gh134265.cpp index a2d360d7ba02a..d5673ece97eba 100644 --- a/clang/test/SemaCXX/gh134265.cpp +++ b/clang/test/SemaCXX/gh134265.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 %s -verify=expected -fsyntax-only -// RUN: %clang_cc1 %s -verify=expected -fsyntax-only -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 struct Foo { >From d0478334528ce6769faf2329ebce02b69c68f355 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <mariya.podchishcha...@intel.com> Date: Thu, 10 Apr 2025 03:39:42 -0700 Subject: [PATCH 3/5] Add a test with explicit dtor --- clang/test/SemaCXX/gh134265.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/clang/test/SemaCXX/gh134265.cpp b/clang/test/SemaCXX/gh134265.cpp index d5673ece97eba..baef10bec8bff 100644 --- a/clang/test/SemaCXX/gh134265.cpp +++ b/clang/test/SemaCXX/gh134265.cpp @@ -31,10 +31,10 @@ void foobar() { } struct BaseDelete1 { - void operator delete[](void *); //ms-note 2{{member found by ambiguous name lookup}} + void operator delete[](void *); //ms-note 3{{member found by ambiguous name lookup}} }; struct BaseDelete2 { - void operator delete[](void *); //ms-note 2{{member found by ambiguous name lookup}} + void operator delete[](void *); //ms-note 3{{member found by ambiguous name lookup}} }; struct BaseDestructor { BaseDestructor() {} @@ -43,6 +43,10 @@ struct BaseDestructor { struct Final : BaseDelete1, BaseDelete2, BaseDestructor { Final() {} }; +struct FinalExplicit : BaseDelete1, BaseDelete2, BaseDestructor { + FinalExplicit() {} + inline ~FinalExplicit() {} +}; #ifdef MS struct Final1 : BaseDelete1, BaseDelete2, BaseDestructor { @@ -52,4 +56,5 @@ struct Final1 : BaseDelete1, BaseDelete2, BaseDestructor { void foo() { Final* a = new Final[10](); // ms-error {{member 'operator delete[]' found in multiple base classes of different types}} + FinalExplicit* b = new FinalExplicit[10](); // ms-error {{member 'operator delete[]' found in multiple base classes of different types}} } >From d441be3fa84d1f48a9981670bcd202131c729a56 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <mariya.podchishcha...@intel.com> Date: Thu, 10 Apr 2025 04:02:55 -0700 Subject: [PATCH 4/5] Clarify why we diagnose for dllexpr --- clang/lib/Sema/SemaDeclCXX.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 6732062965aef..0ee58fbfd9067 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -11049,12 +11049,16 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) { // Lookup delete[] too in case we have to emit a vector deleting dtor; DeclarationName VDeleteName = Context.DeclarationNames.getCXXOperatorName(OO_Array_Delete); + // Diagnose if there is no available operator delete[] found and the + // destructor is exported. Vector deleting dtor body emission requires + // operator delete[] to be present. Whenever the destructor is exported, + // we just always emit vector deleting dtor body, because we don't know + // if new[] will be used with the type outside of the library. Otherwise + // when the dtor is not exported then new[]/delete[] in the TU will make + // sure the operator is referenced and its uses diagnosed. + bool Diagnose = Destructor->hasAttr<DLLExportAttr>() && Destructor->isDefined(); FunctionDecl *ArrOperatorDelete = FindDeallocationFunctionForDestructor( - Loc, RD, VDeleteName, - Destructor->hasAttr<DLLExportAttr>() && Destructor->isDefined()); - // delete[] in the TU will make sure the operator is referenced and its - // uses diagnosed, otherwise vector deleting dtor won't be called - // anyway, so just record it in the destructor. + Loc, RD, VDeleteName, Diagnose); Destructor->setOperatorArrayDelete(ArrOperatorDelete); } } >From cda9b30be2e201f3546a09aa375dd8ea9f353294 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <mariya.podchishcha...@intel.com> Date: Thu, 10 Apr 2025 04:06:56 -0700 Subject: [PATCH 5/5] Format --- clang/lib/Sema/SemaDeclCXX.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 0ee58fbfd9067..721f61ca409ab 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -11056,7 +11056,8 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) { // if new[] will be used with the type outside of the library. Otherwise // when the dtor is not exported then new[]/delete[] in the TU will make // sure the operator is referenced and its uses diagnosed. - bool Diagnose = Destructor->hasAttr<DLLExportAttr>() && Destructor->isDefined(); + bool Diagnose = + Destructor->hasAttr<DLLExportAttr>() && Destructor->isDefined(); FunctionDecl *ArrOperatorDelete = FindDeallocationFunctionForDestructor( Loc, RD, VDeleteName, Diagnose); Destructor->setOperatorArrayDelete(ArrOperatorDelete); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
