From 1b71a22b28326b04bbdafcdb53c0a5f3917a11d8 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Tue, 26 Jul 2011 23:27:24 +0000 Subject: [PATCH] Re-fix r136172 so it isn't an error; apparently, some people are fond of their undefined behavior. llvm-svn: 136183 --- .../clang/Basic/DiagnosticSemaKinds.td | 5 ++-- clang/lib/Sema/SemaExprCXX.cpp | 29 +++++++++---------- clang/test/SemaCXX/new-delete.cpp | 2 +- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 41aaebaa607e..7ee93568e335 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3421,8 +3421,9 @@ def warn_non_virtual_dtor : Warning< def warn_delete_non_virtual_dtor : Warning< "delete called on %0 that has virtual functions but non-virtual destructor">, InGroup, DefaultIgnore; -def err_delete_abstract_non_virtual_dtor : Error< - "cannot delete %0, which is abstract and does not have a virtual destructor">; +def warn_delete_abstract_non_virtual_dtor : Warning< + "delete called on %0 that is abstract but has non-virtual destructor">, + InGroup; def warn_overloaded_virtual : Warning< "%q0 hides overloaded virtual %select{function|functions}1">, InGroup, DefaultIgnore; diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 86a4ecc1fc52..94a5bafa7c7d 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -1913,18 +1913,6 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal, DiagnoseUseOfDecl(Dtor, StartLoc); } - // Deleting an abstract class with a non-virtual destructor is always - // undefined per [expr.delete]p3, and leads to strange-looking - // linker errors. - if (PointeeRD->isAbstract()) { - CXXDestructorDecl *dtor = PointeeRD->getDestructor(); - if (dtor && !dtor->isVirtual()) { - Diag(StartLoc, diag::err_delete_abstract_non_virtual_dtor) - << PointeeElem; - 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 @@ -1933,11 +1921,20 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal, // behavior is undefined. // // Note: a final class cannot be derived from, no issue there - if (!ArrayForm && PointeeRD->isPolymorphic() && - !PointeeRD->hasAttr()) { + if (PointeeRD->isPolymorphic() && !PointeeRD->hasAttr()) { CXXDestructorDecl *dtor = PointeeRD->getDestructor(); - if (dtor && !dtor->isVirtual()) - Diag(StartLoc, diag::warn_delete_non_virtual_dtor) << PointeeElem; + 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; + } + } } } else if (getLangOptions().ObjCAutoRefCount && diff --git a/clang/test/SemaCXX/new-delete.cpp b/clang/test/SemaCXX/new-delete.cpp index 7545b7c136d1..748ce7770097 100644 --- a/clang/test/SemaCXX/new-delete.cpp +++ b/clang/test/SemaCXX/new-delete.cpp @@ -414,5 +414,5 @@ namespace PR10504 { struct A { virtual void foo() = 0; }; - void f(A *x) { delete x; } // expected-error {{cannot delete 'PR10504::A', which is abstract and does not have a virtual destructor}} + void f(A *x) { delete x; } // expected-warning {{delete called on 'PR10504::A' that is abstract but has non-virtual destructor}} }