Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -5802,12 +5802,14 @@ "%0 has virtual functions but non-virtual destructor">, InGroup, DefaultIgnore; def warn_delete_non_virtual_dtor : Warning< - "delete called on non-final %0 that has virtual functions " - "but non-virtual destructor">, + "%select{delete|destructor}0 called on non-final %1 that has " + "virtual functions but non-virtual destructor">, InGroup, DefaultIgnore; +def note_delete_non_virtual : Note< + "qualify call to silence this warning">; def warn_delete_abstract_non_virtual_dtor : Warning< - "delete called on %0 that is abstract but has non-virtual destructor">, - InGroup; + "%select{delete|destructor}0 called on %1 that is abstract but has " + "non-virtual destructor">, InGroup; def warn_overloaded_virtual : Warning< "%q0 hides overloaded virtual %select{function|functions}1">, InGroup, DefaultIgnore; Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -4698,6 +4698,10 @@ ExprResult ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal, bool ArrayForm, Expr *Operand); + void CheckVirtualDtorCall(CXXDestructorDecl *dtor, SourceLocation Loc, + bool IsDelete, bool CallCanBeVirtual, + bool WarnOnNonAbstractTypes, + SourceLocation DtorLoc); DeclResult ActOnCXXConditionDeclaration(Scope *S, Declarator &D); ExprResult CheckConditionVariable(VarDecl *ConditionVar, Index: lib/Sema/SemaExprCXX.cpp =================================================================== --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -2765,30 +2765,10 @@ return ExprError(); } - // C++ [expr.delete]p3: - // In the first alternative (delete object), if the static type of the - // object to be deleted is different from its dynamic type, the static - // type shall be a base class of the dynamic type of the object to be - // deleted and the static type shall have a virtual destructor or the - // behavior is undefined. - // - // Note: a final class cannot be derived from, no issue there - if (PointeeRD->isPolymorphic() && !PointeeRD->hasAttr()) { - CXXDestructorDecl *dtor = PointeeRD->getDestructor(); - if (dtor && !dtor->isVirtual()) { - if (PointeeRD->isAbstract()) { - // If the class is abstract, we warn by default, because we're - // sure the code has undefined behavior. - Diag(StartLoc, diag::warn_delete_abstract_non_virtual_dtor) - << PointeeElem; - } else if (!ArrayForm) { - // Otherwise, if this is not an array delete, it's a bit suspect, - // but not necessarily wrong. - Diag(StartLoc, diag::warn_delete_non_virtual_dtor) << PointeeElem; - } - } - } - + CheckVirtualDtorCall(PointeeRD->getDestructor(), StartLoc, + /*IsDelete=*/true, /*CallCanBeVirtual=*/true, + /*WarnOnNonAbstractTypes=*/!ArrayForm, + SourceLocation()); } if (!OperatorDelete) @@ -2817,6 +2797,45 @@ return Result; } +void Sema::CheckVirtualDtorCall(CXXDestructorDecl *dtor, SourceLocation Loc, + bool IsDelete, bool CallCanBeVirtual, + bool WarnOnNonAbstractTypes, + SourceLocation DtorLoc) { + if (!dtor || dtor->isVirtual() || !CallCanBeVirtual) + return; + + // C++ [expr.delete]p3: + // In the first alternative (delete object), if the static type of the + // object to be deleted is different from its dynamic type, the static + // type shall be a base class of the dynamic type of the object to be + // deleted and the static type shall have a virtual destructor or the + // behavior is undefined. + // + const CXXRecordDecl *PointeeRD = dtor->getParent(); + // Note: a final class cannot be derived from, no issue there + if (!PointeeRD->isPolymorphic() || PointeeRD->hasAttr()) + return; + + QualType ClassType = dtor->getThisType(Context)->getPointeeType(); + if (PointeeRD->isAbstract()) { + // If the class is abstract, we warn by default, because we're + // sure the code has undefined behavior. + Diag(Loc, diag::warn_delete_abstract_non_virtual_dtor) << (IsDelete ? 0 : 1) + << ClassType; + } else if (WarnOnNonAbstractTypes) { + // Otherwise, if this is not an array delete, it's a bit suspect, + // but not necessarily wrong. + Diag(Loc, diag::warn_delete_non_virtual_dtor) << (IsDelete ? 0 : 1) + << ClassType; + } + if (!IsDelete) { + std::string TypeStr; + ClassType.getAsStringInternal(TypeStr, getPrintingPolicy()); + Diag(DtorLoc, diag::note_delete_non_virtual) + << FixItHint::CreateInsertion(DtorLoc, TypeStr + "::"); + } +} + /// \brief Check the use of the given variable as a C++ condition in an if, /// while, do-while, or switch statement. ExprResult Sema::CheckConditionVariable(VarDecl *ConditionVar, Index: lib/Sema/SemaOverload.cpp =================================================================== --- lib/Sema/SemaOverload.cpp +++ lib/Sema/SemaOverload.cpp @@ -12236,6 +12236,17 @@ << MD->getDeclName(); } } + + if (CXXDestructorDecl *DD = + dyn_cast(TheCall->getMethodDecl())) { + // a->A::f() doesn't go through the vtable, except in AppleKext mode. + bool CallCanBeVirtual = !cast(NakedMemExpr)->hasQualifier() || + getLangOpts().AppleKext; + CheckVirtualDtorCall(DD, MemExpr->getLocStart(), /*IsDelete=*/false, + CallCanBeVirtual, /*WarnOnNonAbstractTypes=*/true, + MemExpr->getMemberLoc()); + } + return MaybeBindToTemporary(TheCall); } Index: test/SemaCXX/destructor.cpp =================================================================== --- test/SemaCXX/destructor.cpp +++ test/SemaCXX/destructor.cpp @@ -257,6 +257,7 @@ } } +// FIXME: Why are these supposed to not warn? void nowarnarray() { { B* b = new B[4]; @@ -311,6 +312,14 @@ } } +void nowarn0_explicit_dtor(F* f, VB* vb, VD* vd, VF* vf) { + f->~F(); + f->~F(); + vb->~VB(); + vd->~VD(); + vf->~VF(); +} + void warn0() { { B* b = new B(); @@ -326,6 +335,17 @@ } } +void warn0_explicit_dtor(B* b, B& br, D* d) { + b->~B(); // expected-warning {{destructor called on non-final 'dnvd::B' that has virtual functions but non-virtual destructor}} expected-note{{qualify call to silence this warning}} + b->B::~B(); // No warning when the call isn't virtual. + + br.~B(); // expected-warning {{destructor called on non-final 'dnvd::B' that has virtual functions but non-virtual destructor}} expected-note{{qualify call to silence this warning}} + br.B::~B(); + + d->~D(); // expected-warning {{destructor called on non-final 'dnvd::D' that has virtual functions but non-virtual destructor}} expected-note{{qualify call to silence this warning}} + d->D::~D(); +} + void nowarn1() { { simple_ptr f(new F());