r312167 - Let -Wdelete-non-virtual-dtor fire in system headers too.

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 30 13:25:22 PDT 2017


Author: nico
Date: Wed Aug 30 13:25:22 2017
New Revision: 312167

URL: http://llvm.org/viewvc/llvm-project?rev=312167&view=rev
Log:
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

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaExprCXX.cpp
    cfe/trunk/test/SemaCXX/destructor.cpp
    cfe/trunk/test/SemaCXX/implicit-exception-spec.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=312167&r1=312166&r2=312167&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Aug 30 13:25:22 2017
@@ -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;

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=312167&r1=312166&r2=312167&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Aug 30 13:25:22 2017
@@ -3284,7 +3284,7 @@ void Sema::CheckVirtualDtorCall(CXXDestr
                                 bool IsDelete, bool CallCanBeVirtual,
                                 bool WarnOnNonAbstractTypes,
                                 SourceLocation DtorLoc) {
-  if (!dtor || dtor->isVirtual() || !CallCanBeVirtual)
+  if (!dtor || dtor->isVirtual() || !CallCanBeVirtual || isUnevaluatedContext())
     return;
 
   // C++ [expr.delete]p3:

Modified: cfe/trunk/test/SemaCXX/destructor.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/destructor.cpp?rev=312167&r1=312166&r2=312167&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/destructor.cpp (original)
+++ cfe/trunk/test/SemaCXX/destructor.cpp Wed Aug 30 13:25:22 2017
@@ -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();
@@ -213,18 +239,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:
   simple_ptr2(T* t): _ptr(t) {}
@@ -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

Modified: cfe/trunk/test/SemaCXX/implicit-exception-spec.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/implicit-exception-spec.cpp?rev=312167&r1=312166&r2=312167&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/implicit-exception-spec.cpp (original)
+++ cfe/trunk/test/SemaCXX/implicit-exception-spec.cpp Wed Aug 30 13:25:22 2017
@@ -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}}




More information about the cfe-commits mailing list