Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11045,15 +11045,23 @@ 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);
// 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, Diagnose);
Destructor->setOperatorArrayDelete(ArrOperatorDelete);
}
}
}

Expand Down
18 changes: 16 additions & 2 deletions clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();

Expand Down
44 changes: 41 additions & 3 deletions clang/test/SemaCXX/gh134265.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// RUN: %clang_cc1 %s -verify -fsyntax-only
// 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 {
virtual ~Foo() {} // expected-error {{attempt to use a deleted function}}
Expand All @@ -13,10 +15,46 @@ 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 3{{member found by ambiguous name lookup}}
};
struct BaseDelete2 {
void operator delete[](void *); //ms-note 3{{member found by ambiguous name lookup}}
};
struct BaseDestructor {
BaseDestructor() {}
virtual ~BaseDestructor() = default;
};
struct Final : BaseDelete1, BaseDelete2, BaseDestructor {
Final() {}
};
struct FinalExplicit : BaseDelete1, BaseDelete2, BaseDestructor {
FinalExplicit() {}
inline ~FinalExplicit() {}
};

#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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe throw in a third case for good measure: an explicit, inline, non-exported destructor, with ambiguous lookup. I'm guessing we want the same diagnostics we get for Final, i.e. we emit the diagnostic only when array-new gets called, as in the case with the implicit destructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, thanks!


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}}
}