-Wnon-virtual-dtor warns even when the destructor is protected and the class has only pure virtual members and no friends. In that case, there's no way the destructor can be called incorrectly. The class can't call it, since there are no methods other than the destructor itself. There are no friends who could call it. And if you inherit from it without making the derived class' destructor virtual, it would complain about it in the derived class, since it has virtual functions and a non-virtual destructor (unless it also has no non-pure members, no friends and a non-public destructor). Release: 3.1.1 20020703 (Debian prerelease) Environment: System: Linux flower 2.4.18-preempt #1 Mon Jun 17 14:21:46 BRT 2002 i686 unknown Architecture: i686 host: i386-pc-linux-gnu build: i386-pc-linux-gnu target: i386-pc-linux-gnu configured with: /mnt/data/gcc-3.1/gcc-3.1-3.1.1ds2/src/configure -v --enable-languages=c,c++,java,f77,proto,objc,ada --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-gxx-include-dir=/usr/include/g++-v3-3.1 --enable-shared --with-system-zlib --enable-long-long --enable-nls --without-included-gettext --enable-clocale=gnu --enable-__cxa_atexit --enable-threads=posix --enable-java-gc=boehm --enable-objc-gc i386-linux How-To-Repeat: Input file: === x.cc === class x { public: virtual void y () = 0; protected: ~x () { } }; void f (x* p) { p->y(); delete p; } ====== Command line: ====== g++-3.1 -Wnon-virtual-dtor -O2 -v -save-temps x.cc -o x ====== Compiler output: ====== Reading specs from /usr/lib/gcc-lib/i386-linux/3.1.1/specs Configured with: /mnt/data/gcc-3.1/gcc-3.1-3.1.1ds2/src/configure -v --enable-languages=c,c++,java,f77,proto,objc,ada --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-gxx-include-dir=/usr/include/g++-v3-3.1 --enable-shared --with-system-zlib --enable-long-long --enable-nls --without-included-gettext --enable-clocale=gnu --enable-__cxa_atexit --enable-threads=posix --enable-java-gc=boehm --enable-objc-gc i386-linux Thread model: posix gcc version 3.1.1 20020703 (Debian prerelease) /usr/lib/gcc-lib/i386-linux/3.1.1/cpp0 -lang-c++ -D__GNUG__=3 -D__DEPRECATED -D__EXCEPTIONS -D__GXX_ABI_VERSION=100 -v -D__GNUC__=3 -D__GNUC_MINOR__=1 -D__GNUC_PATCHLEVEL__=1 -D__ELF__ -Dunix -D__gnu_linux__ -Dlinux -D__ELF__ -D__unix__ -D__gnu_linux__ -D__linux__ -D__unix -D__linux -Asystem=posix -D__OPTIMIZE__ -D__STDC_HOSTED__=1 -D_GNU_SOURCE -Acpu=i386 -Amachine=i386 -Di386 -D__i386 -D__i386__ -D__tune_i386__ x.cc -Wnon-virtual-dtor x.ii GNU CPP version 3.1.1 20020703 (Debian prerelease) (cpplib) (i386 Linux/ELF) ignoring nonexistent directory "/usr/i386-linux/include" #include "..." search starts here: #include <...> search starts here: /usr/include/g++-v3-3.1 /usr/include/g++-v3-3.1/i386-linux /usr/include/g++-v3-3.1/backward /usr/local/include /usr/lib/gcc-lib/i386-linux/3.1.1/include /usr/include End of search list. /usr/lib/gcc-lib/i386-linux/3.1.1/cc1plus -fpreprocessed x.ii -quiet -dumpbase x.cc -O2 -Wnon-virtual-dtor -version -o x.s GNU CPP version 3.1.1 20020703 (Debian prerelease) (cpplib) (i386 Linux/ELF) GNU C++ version 3.1.1 20020703 (Debian prerelease) (i386-linux) compiled by GNU C version 3.1.1 20020703 (Debian prerelease). x.cc:2: warning: `class x' has virtual functions but non-virtual destructor x.cc: In function `void f(x*)': x.cc:7: `x::~x()' is protected x.cc:13: within this context ====== The warning at line 2 is the bogus one. The error at line 13 is on purpose, to show you can't delete it (and so that the warning is bogus).
Fix: If a class has only pure virtual members besides the destructor and has no friends, and the destructor is protected or private, then the warning shouldn't be shown.
State-Changed-From-To: open->analyzed State-Changed-Why: Confirmed. Low priority.
Closing as a dup of bug 15214 because that one has a start on the patch but it also describes another situation. *** This bug has been marked as a duplicate of 15214 ***
This is indeed not a duplicate of PR 15214 though the two reports certainly go into the same direction. Let's keep it open then. W.
A pure virtual member function can still be defined (and must be, if it is the destructor).
*** Bug 20570 has been marked as a duplicate of this bug. ***
Created attachment 11068 [details] proposed patch patch, proposed at http://bugs.debian.org/356316
Created attachment 11520 [details] proposed patch (with doc and test changes)
I'd like to point out that structures containing only pure virtual functions should not trigger this diagnostic either. Consider the following: struct IfacFoo { virtual int a() = 0; virtual int b() = 0; }; There is no destructor declared nor do any members exist to necessitate the implicit creation of one. As of version "4.1.1 20060525 (Red Hat 4.1.1-1)", the diagnostic message is still emitted in both this case and those with a protected "dummy" destructor included.
Lawrence: Every class has a destructor. You're talking about classes that have trivial destructors. Whether a non-virtual destructor is trivial or not has no bearing on the fact that if an instance of a derived class is destroyed through a pointer or reference to a base class with a non-virtual destructor, the result is undefined behaviour.
(In reply to comment #8) > Created an attachment (id=11520) [edit] > proposed patch (with doc and test changes) > Thanks for the patch. However, formal submissions are sent to gcc-patches@gcc.gnu.org. An attachment in bugzilla is not considered a formal submission and it may go unnoticed. Please, consider submitting to gcc-patches@gcc.gnu.org (and perhaps also adding it to the patch queue: http://www.dberlin.org/patchdirections.html )
Already posted as <http://gcc.gnu.org/ml/gcc-patches/2006-04/msg00885.html>, with no response.
(In reply to comment #12) > Already posted as <http://gcc.gnu.org/ml/gcc-patches/2006-04/msg00885.html>, > with no response. > You need to insist. A week is normally considered an acceptable interval between pings. You need to bootstrap + regression test. http://gcc.gnu.org/contribute.html#testing Short version: make && make -k check Then, compare with the output without your patch. What I normally do is, for an unpatched tree: make && (make -k check &> pristine) then patch and do the same saving the output in another file. Compare the two outputs (there is a compare tests script in the contrib/ directory). Of course, this is a very dirty and rough explanation. It is better to read the docs.
(In reply to comment #12) > Already posted as <http://gcc.gnu.org/ml/gcc-patches/2006-04/msg00885.html>, > with no response. this patch doesn't cover one situation: struct D; struct C { virtual void f() = 0; protected: // or private friend class D; ~C(); }; in such case compiler should generate warning, because class D can delete derived object through the pointer to class C.
Pawel: Yes, any friend class or function can call a protected or private destructor wrongly. So can members of the class - in fact, even pure virtual members can, since they may still have definitions! The current implementation warns whenever any function might be able to call a non-virtual destructor in a polymorphic class; this results in many false positives. False warnings result in programmers disabling the warning, or paying less attention to warnings, or (in this case) introducing a virtual destructor for no good reason. That is why I consider the behaviour a bug.
quite better ( modulo coding style ) patch is: --- class.c.orig 2006-10-12 22:02:53.000000000 +0200 +++ class.c 2007-02-22 02:54:11.888652367 +0100 @@ -5105,15 +5105,15 @@ tree dtor; dtor = CLASSTYPE_DESTRUCTORS (t); - /* Warn only if the dtor is non-private or the class has - friends. */ if (/* An implicitly declared destructor is always public. And, if it were virtual, we would have created it by now. */ !dtor || (!DECL_VINDEX (dtor) - && (!TREE_PRIVATE (dtor) - || CLASSTYPE_FRIEND_CLASSES (t) - || DECL_FRIENDLIST (TYPE_MAIN_DECL (t))))) + && (/* public non-virtual */ + (!TREE_PRIVATE (dtor) && !TREE_PROTECTED (dtor)) + || (/* or non-public non-virtual with friends */ + (TREE_PRIVATE (dtor) || TREE_PROTECTED (dtor)) + && (CLASSTYPE_FRIEND_CLASSES (t) || DECL_FRIENDLIST (TYPE_MAIN_DECL (t))))))) warning (0, "%q#T has virtual functions but non-virtual destructor", t); } it correctly warns on B, C and D classes: struct A { virtual void f() = 0; protected: ~A(); // ok. }; struct B { virtual void f() = 0; ~B(); // warn! public non-virtual dtor. }; struct C { virtual void f() = 0; // warn! implicit public non-virtual dtor. }; struct D { virtual void f() = 0; private: friend class C; ~D(); // warn! can be called from class C. };
Created attachment 13214 [details] extended patch against gcc-4.2
(In reply to comment #17) > Created an attachment (id=13214) [edit] > extended patch against gcc-4.2 > Hi Pawel, if the bug exists in mainline, the patch should be against mainline (I think this patch will apply to mainline anyway). The patch needs testcases, and it should be sent to gcc-patches@gcc.gnu.org. Also, + warning (0, "%q#T has virtual functions and accessible" + " non-virtual destructor", t) should be: + warning (OPT_Wnon_virtual_dtor, + "%q#T has virtual functions and accessible" + " non-virtual destructor", t) Also + || DECL_FRIENDLIST( TYPE_MAIN_DECL (t))))))) should be + || DECL_FRIENDLIST (TYPE_MAIN_DECL (t))))))) Thanks for the patch.
(In reply to comment #18) > The patch needs testcases, i have a testcase but my tcl/autogen/dejagnu crashes with magic `spawn failed' message :/ e.g.: (...) Executing on host: /home/users/pluto/rpm/BUILD/gcc-4.2-20070307/builddir/gcc/xgcc -B/home/users/pluto/rpm/BUILD/gcc-4.2-20070307/builddir/gcc/ -O0 -w -fno-show-column -c -o 20000105-1.o /home/users/pluto/rpm/BUILD/gcc-4.2-20070307/gcc/testsuite/gcc.c-torture/compile/20000105-1.c (timeout = 300) compiler exited with status -1 output is: spawn failed FAIL: gcc.c-torture/compile/20000105-1.c -O0 (test for excess errors) Excess errors: spawn failed
(In reply to comment #19) > (In reply to comment #18) > > > The patch needs testcases, > > i have a testcase but my tcl/autogen/dejagnu crashes > with magic `spawn failed' message :/ > No idea. Ask in the gcc list, perhaps someone could help you. I would just remove completely and reinstall DejaGNU. Or just use the GCC Compile Farm for testing. I have a script there that you just say PATCH=/path/to/patch ./gccfarming bootstrap And it bootstraps and regression tests the patch and sends you the results by email. It also has a "patchqueue" mode where filenames of patches are read from a file and bootstrapped+tested in sequence, and you can add more names at the end of the file on the fly.
http://gcc.gnu.org/ml/gcc-patches/2007-03/msg01343.html
(In reply to comment #21) > http://gcc.gnu.org/ml/gcc-patches/2007-03/msg01343.html > Hint, if you use the patch queue[1], it takes care of adding a comment pointing to the patch. Also, your patch lacks a Changelog [2]. See also an example [3]. [1] http://www.dberlin.org/patchdirections.html [2] http://gcc.gnu.org/codingconventions.html [3] http://gcc.gnu.org/ml/gcc-patches/2006-12/msg00008.html
Subject: Bug number PR7302 A patch for this bug has been added to the patch tracker. The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2007-03/msg01347.html
Subject: Bug 7302 Author: jason Date: Mon Aug 20 15:08:24 2007 New Revision: 127649 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127649 Log: PR c++/7302 * cp/class.c (finish_struct_1): Warn when a class has virtual functions and accessible non-virtual destructor. * doc/invoke.texi (-Wnon-virtual-dtor): Update documentation. * g++.dg/warn/Wnvdtor-2.C: New testcase. Added: trunk/gcc/testsuite/g++.dg/warn/Wnvdtor-2.C Modified: trunk/gcc/ChangeLog trunk/gcc/cp/ChangeLog trunk/gcc/cp/class.c trunk/gcc/doc/invoke.texi trunk/gcc/testsuite/ChangeLog
The testcase seems to be missing one case where it should warn: class H { protected: ~H(); public: virtual void deleteme() = 0; }; H::~H() { } void H::deleteme() { delete this; } class I : public H { protected: ~I(); public: virtual void deleteme() { H::deleteme(); } void oops(); }; I::~I() { } void I::oops() { deleteme(); } Here, H must have a virtual destructor. The point where it can know it should warn is the "delete this;" line.
(can this bug be un-ASSIGNED?) (In reply to comment #25) > Here, H must have a virtual destructor. The point where it can know it should > warn is the "delete this;" line. I've posted a patch to http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00191.html which gives this for the code in comment 25 7302.cc:16:12: warning: deleting object of abstract class type ‘H’ which has non-virtual destructor will cause undefined behaviour [-Wdelete-non-virtual-dtor]
Assigned to you Jonathan! Feel free to close it when your patch goes in. If there is something else left to do, it is always better to open a new PR.
(In reply to comment #26) > I've posted a patch to http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00191.html > which gives this for the code in comment 25 BTW, I would suggest that you use warning_at, but I guess this is a lost battle.
For G++ 4.7 I've added -Wdelete-non-virtual-dtor, included in -Wall, to handle the case in comment 25, so closing this as fixed now