Bug 15214 - Warning non-virtual-dtor too strict
Summary: Warning non-virtual-dtor too strict
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 3.4.0
: P2 enhancement
Target Milestone: 4.0.0
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, patch
Depends on:
Blocks:
 
Reported: 2004-04-30 00:18 UTC by Tom Marshall
Modified: 2004-05-28 17:13 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2004-04-30 00:26:57


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Marshall 2004-04-30 00:18:05 UTC
It seems to me that it is safe to have virtual functions and a non-virtual dtor
if the dtor can only be invoked by methods within the class.  That is, the dtor
must be private and the class must not declare any friends.

Sample code:

#include <stdio.h>

class A
{
public:
    A() : m_ref(0) { printf("a::ctor\n"); }

    virtual int addref(void);
    virtual int release(void);

private:
    ~A() { printf("a::dtor\n"); }

protected:
    int m_ref;
};

int A::addref(void)
{
    return ++m_ref;
}
int A::release(void)
{
    int ref = --m_ref;
    if (ref == 0) delete this;
    return ref;
}
 
int main(void)
{
    A* a = new A;
    a->addref();
    a->release();
    return 0;
}

Patch:

--- class.c.orig        2004-04-29 16:51:53.000000000 -0700
+++ class.c     2004-04-29 16:51:55.000000000 -0700
@@ -5102,7 +5102,17 @@
 
   if (warn_nonvdtor && TYPE_POLYMORPHIC_P (t) && TYPE_HAS_DESTRUCTOR (t)
       && DECL_VINDEX (TREE_VEC_ELT (CLASSTYPE_METHOD_VEC (t), 1)) == NULL_TREE)
-    warning ("`%#T' has virtual functions but non-virtual destructor", t);
+    {
+      tree dtor = TREE_VEC_ELT (CLASSTYPE_METHOD_VEC (t), 1);
+
+      /* Warn only if the dtor is non-private or the class has friends */
+      if (!TREE_PRIVATE (dtor) ||
+         (CLASSTYPE_FRIEND_CLASSES (t) ||
+          DECL_FRIENDLIST (TYPE_MAIN_DECL (t))))
+       {
+         warning ("%#T' has virtual functions but non-virtual destructor", t);
+       }
+    }
 
   complete_vars (t);
Comment 1 Andrew Pinski 2004-04-30 00:26:56 UTC
Even what you are proposing is too strict, see PR 7302 (I am going to close that one as a dup of this 
one though).
Comment 2 Andrew Pinski 2004-04-30 00:31:09 UTC
*** Bug 7302 has been marked as a duplicate of this bug. ***
Comment 3 Andrew Pinski 2004-04-30 00:35:37 UTC
I think you can just change !TREE_PRIVATE to be TREE_PUBLIC and submit it to gcc-
patches@gcc.gnu.org with a changelog and ___AFTER___ reading <http://gcc.gnu.org/
contribute.html>.  If you cannot make testcases just ask for help with them.
Comment 4 Tom Marshall 2004-04-30 03:45:14 UTC
(In reply to comment #3)
> I think you can just change !TREE_PRIVATE to be TREE_PUBLIC and submit it to gcc-
> patches@gcc.gnu.org with a changelog and ___AFTER___ reading <http://gcc.gnu.org/
> contribute.html>.  If you cannot make testcases just ask for help with them.

I don't think this is correct either.  This would make it possible to declare
class A with virtual functions and a protected non-virtual dtor, then declare
class B that derives from A, also with a protected non-virtual dtor, without
invoking the warning.  I am not familiar enough with the internals of GCC to
know whether there is an easy solution for bug 7302.  In fact, I think that it
is just a corner case that can probably be marked WONTFIX.  If you declare a
class in which all functions are pure virtual (including all base class(es)),
what sense is there in providing an explicit dtor?  If it is to cleanup member
variables, the dtor surely must be virtual to ensure it is properly invoked,
correct?
Comment 5 Wolfgang Bangerth 2004-05-11 18:38:50 UTC
A patch was submitted here: 
  http://gcc.gnu.org/ml/gcc-patches/2004-05/msg00599.html 
Comment 6 Wolfgang Bangerth 2004-05-11 18:53:01 UTC
I don't quite understand the usefulness of the construct you want the  
compiler to accept: if the destructor can't be called from a derived  
class, then you can derive from this class. Why would you want to have  
virtual functions then?  
  
W.  
Comment 7 Tom Marshall 2004-05-11 19:03:55 UTC
(In reply to comment #6)
> I don't quite understand the usefulness of the construct you want the  
> compiler to accept: if the destructor can't be called from a derived  
> class, then you can derive from this class. Why would you want to have  
> virtual functions then?  

The class may derive from base class(es) that have virtual functions.  This is
a common construct in COM programming, for example.  The class implements one or
more interfaces (class definitions that consist of pure virtual functions) and
uses "delete this" in its Release() method.  Making the dtor private helps to
ensure that it is being used properly.
Comment 8 Wolfgang Bangerth 2004-05-11 19:23:18 UTC
Ah, ok. Basically what you do is to mark the class "final", i.e. no 
other class can derive from it. I think your claim then makes sense. 
 
W. 
Comment 9 CVS Commits 2004-05-28 17:01:26 UTC
Subject: Bug 15214

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jason@gcc.gnu.org	2004-05-28 17:01:21

Modified files:
	gcc/cp         : ChangeLog class.c 

Log message:
	PR c++/15214
	* class.c (finish_struct_1): Warn only if the dtor is non-private or
	the class has friends.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4060&r2=1.4061
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/class.c.diff?cvsroot=gcc&r1=1.614&r2=1.615

Comment 10 Andrew Pinski 2004-05-28 17:13:18 UTC
Fixed.