From 1e678b8bdc3c8535fb410efceb5469ba0a1b3c86 Mon Sep 17 00:00:00 2001
From: Nico Weber <nicolasweber@gmx.de>
Date: Wed, 30 Aug 2017 20:25:22 +0000
Subject: [PATCH] Let -Wdelete-non-virtual-dtor fire in system headers too.
Makes the warning useful again in a std::unique_ptr world, PR28460.
Also make the warning not fire in unevaluated contexts, since system libraries
(e.g. libc++) do do that. This would've been a good change before we started
emitting this warning in system headers too, but "normal" code seems to be less
template-heavy, so we didn't notice until now.
https://reviews.llvm.org/D37235
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@312167 91177308-0d34-0410-b5e6-96231b3b80d8
---
include/clang/Basic/DiagnosticSemaKinds.td | 4 +-
lib/Sema/SemaExprCXX.cpp | 2 +-
test/SemaCXX/destructor.cpp | 57 +++++++++++++++++-----
test/SemaCXX/implicit-exception-spec.cpp | 2 +-
4 files changed, 49 insertions(+), 16 deletions(-)
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 18b2bc21871..c41e19dbe4f 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6417,12 +6417,12 @@ def warn_non_virtual_dtor : Warning<
def warn_delete_non_virtual_dtor : Warning<
"%select{delete|destructor}0 called on non-final %1 that has "
"virtual functions but non-virtual destructor">,
- InGroup<DeleteNonVirtualDtor>, DefaultIgnore;
+ InGroup<DeleteNonVirtualDtor>, DefaultIgnore, ShowInSystemHeader;
def note_delete_non_virtual : Note<
"qualify call to silence this warning">;
def warn_delete_abstract_non_virtual_dtor : Warning<
"%select{delete|destructor}0 called on %1 that is abstract but has "
- "non-virtual destructor">, InGroup<DeleteNonVirtualDtor>;
+ "non-virtual destructor">, InGroup<DeleteNonVirtualDtor>, ShowInSystemHeader;
def warn_overloaded_virtual : Warning<
"%q0 hides overloaded virtual %select{function|functions}1">,
InGroup<OverloadedVirtual>, DefaultIgnore;
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 456aff2fe00..8de3d33d085 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -3284,7 +3284,7 @@ void Sema::CheckVirtualDtorCall(CXXDestructorDecl *dtor, SourceLocation Loc,
bool IsDelete, bool CallCanBeVirtual,
bool WarnOnNonAbstractTypes,
SourceLocation DtorLoc) {
- if (!dtor || dtor->isVirtual() || !CallCanBeVirtual)
+ if (!dtor || dtor->isVirtual() || !CallCanBeVirtual || isUnevaluatedContext())
return;
// C++ [expr.delete]p3:
diff --git a/test/SemaCXX/destructor.cpp b/test/SemaCXX/destructor.cpp
index 49cdc50f3c1..31ee60a718a 100644
--- a/test/SemaCXX/destructor.cpp
+++ b/test/SemaCXX/destructor.cpp
@@ -1,5 +1,31 @@
// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -fsyntax-only -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -fcxx-exceptions -verify %s
// RUN: %clang_cc1 -std=c++11 -triple %ms_abi_triple -DMSABI -fsyntax-only -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -verify %s
+
+#if defined(BE_THE_HEADER)
+
+// Wdelete-non-virtual-dtor should warn about the delete from smart pointer
+// classes in system headers (std::unique_ptr...) too.
+
+#pragma clang system_header
+namespace dnvd {
+template <typename T>
+class simple_ptr {
+public:
+ simple_ptr(T* t): _ptr(t) {}
+ ~simple_ptr() { delete _ptr; } // \
+ // expected-warning {{delete called on non-final 'dnvd::B' that has virtual functions but non-virtual destructor}} \
+ // expected-warning {{delete called on non-final 'dnvd::D' that has virtual functions but non-virtual destructor}}
+ T& operator*() const { return *_ptr; }
+private:
+ T* _ptr;
+};
+}
+
+#else
+
+#define BE_THE_HEADER
+#include __FILE__
+
class A {
public:
~A();
@@ -212,18 +238,6 @@ struct VD: VB {};
struct VF final: VB {};
-template <typename T>
-class simple_ptr {
-public:
- simple_ptr(T* t): _ptr(t) {}
- ~simple_ptr() { delete _ptr; } // \
- // expected-warning {{delete called on non-final 'dnvd::B' that has virtual functions but non-virtual destructor}} \
- // expected-warning {{delete called on non-final 'dnvd::D' that has virtual functions but non-virtual destructor}}
- T& operator*() const { return *_ptr; }
-private:
- T* _ptr;
-};
-
template <typename T>
class simple_ptr2 {
public:
@@ -335,10 +349,28 @@ void warn0() {
}
}
+// Taken from libc++, slightly simplified.
+template <class>
+struct __is_destructible_apply { typedef int type; };
+struct __two {char __lx[2];};
+template <typename _Tp>
+struct __is_destructor_wellformed {
+ template <typename _Tp1>
+ static char __test(typename __is_destructible_apply<
+ decltype(_Tp1().~_Tp1())>::type);
+ template <typename _Tp1>
+ static __two __test (...);
+
+ static const bool value = sizeof(__test<_Tp>(12)) == sizeof(char);
+};
+
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.
+ // No warning in unevaluated contexts.
+ (void)__is_destructor_wellformed<B>::value;
+
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();
@@ -451,3 +483,4 @@ void foo1() {
x.foo1();
}
}
+#endif // BE_THE_HEADER
diff --git a/test/SemaCXX/implicit-exception-spec.cpp b/test/SemaCXX/implicit-exception-spec.cpp
index f400c222de8..c21f773e94c 100644
--- a/test/SemaCXX/implicit-exception-spec.cpp
+++ b/test/SemaCXX/implicit-exception-spec.cpp
@@ -121,7 +121,7 @@ namespace PotentiallyConstructed {
T &a = *p;
static_assert(noexcept(a = a) == D, "");
static_assert(noexcept(a = static_cast<T&&>(a)) == E, "");
- static_assert(noexcept(delete &a) == F, ""); // expected-warning 2{{abstract}}
+ static_assert(noexcept(delete &a) == F, "");
// These are last because the first failure here causes instantiation to bail out.
static_assert(noexcept(new (nothrow) T()) == A, ""); // expected-error 2{{abstract}}
--
GitLab