This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFC: Generating C++ warnings for non-virtual destructors at delete


This patch was inspired from http://gcc.gnu.org/bugzilla/show_bug.cgi?id=7302
which qualifies the warning about a class having "virtual functions but
non-virtual destructor"



The -Wnon-virtual-dtor option has been problematic for me in that it generates warnings in my code base for cases that I don't really need to worry about.



After some consideration I've concluded that for my purposes the declaration time check for -Wnon-virtual-dtor is not useful because there are cases where it is perfectly acceptable to have a non-virtual protected dtor, even in the presence of friends. For example:

class Base
{
   friend class IteratorBase; // Does not delete Base

protected:
   Base();
   ~Base();

private:
   virtual void f();
};


In this case the recent changes to -Wnon-virtual-dtor generate a what I consider to be gratuitous warning.


I think that instead of attempting to determine failure at declaration time, I get more useful warnings if a check is made when delete is invoked. As an example, the above code for Base is perfectly acceptable so long as there is no attempt to delete a pointer to Base.


The patch introduces -Wbase-class-dtor and will issue a warning if:


a. The dtor is non-virtual and either public or private, the class
   is polymorphic and there is at least one protected ctor.

b. The dtor is non-virtual and protected.


I'm looking for some feedback as to whether a change in this direction would be useful for the community before investing additional time in working the patch up into shape for inclusion in 4.x code base.


Earl



--- gcc-3.4.2/gcc/c-common.c.orig 2008-11-05 09:40:41.946166300 -0800 +++ gcc-3.4.2/gcc/c-common.c 2008-11-05 12:28:40.947121100 -0800 @@ -628,6 +628,11 @@

int warn_nonvdtor;

+/* Nonzero means warn when calling a non virtual destructor in a polymorphic
+ class, when it really should have a virtual one. */
+
+int warn_baseclassdtor;
+
/* Nonzero means warn when the compiler will reorder code. */


 int warn_reorder;
--- gcc-3.4.2/gcc/c-common.h.orig	2008-11-05 09:43:47.586969700 -0800
+++ gcc-3.4.2/gcc/c-common.h	2008-11-05 12:28:41.025263100 -0800
@@ -789,6 +789,11 @@

extern int warn_nonvdtor;

+/* Nonzero means warn when calling a non virtual destructor in a polymorphic
+ class, when it really should have a virtual one. */
+
+extern int warn_baseclassdtor;
+
/* Nonzero means warn when the compiler will reorder code. */


 extern int warn_reorder;
--- gcc-3.4.2/gcc/c-opts.c.orig	2008-11-05 09:39:58.387290700 -0800
+++ gcc-3.4.2/gcc/c-opts.c	2008-11-05 12:28:41.025263100 -0800
@@ -383,6 +383,7 @@
 	{
 	  /* C++-specific warnings.  */
 	  warn_nonvdtor = value;
+          warn_baseclassdtor = value;
 	  warn_reorder = value;
 	  warn_nontemplate_friend = value;
 	}
@@ -550,6 +551,10 @@
       warn_nonvdtor = value;
       break;

+    case OPT_Wbase_class_dtor:
+      warn_baseclassdtor = value;
+      break;
+
     case OPT_Wnonnull:
       warn_nonnull = value;
       break;
--- gcc-3.4.2/gcc/c.opt.orig	2008-11-05 09:37:29.071589800 -0800
+++ gcc-3.4.2/gcc/c.opt	2008-11-05 12:28:41.025263100 -0800
@@ -305,6 +305,10 @@
 C++ ObjC++
 Warn about non-virtual destructors

+Wbase-class-dtor
+C++ ObjC++
+Warn about base class non-virtual destructors
+
 Wnonnull
 C ObjC

--- gcc-3.4.2/gcc/cp/decl2.c.orig	2008-11-05 06:57:31.026463600 -0800
+++ gcc-3.4.2/gcc/cp/decl2.c	2008-11-05 12:57:05.286654700 -0800
@@ -487,6 +487,50 @@
       doing_vec = 0;
     }

+ /* Deleting a polymorphic type lacking a virtual dtor may warn. */
+ if (warn_baseclassdtor)
+ {
+ tree classtype;
+
+ classtype = TREE_TYPE (type);
+
+ if (IS_AGGR_TYPE (classtype))
+ {
+ tree dtor;
+
+ dtor = CLASSTYPE_DESTRUCTORS (classtype);
+
+ /* Check for polymorphic type with non-protected dtor and
+ at least one protected ctor. */
+ if (TYPE_POLYMORPHIC_P (classtype)
+ && (!dtor
+ || ( !DECL_VIRTUAL_P (dtor) && !TREE_PROTECTED (dtor))))
+ {
+ tree ctor;
+
+ for (ctor = CLASSTYPE_CONSTRUCTORS (classtype);
+ ctor; ctor = OVL_NEXT (ctor))
+ {
+ if (TREE_PROTECTED (OVL_CURRENT (ctor)))
+ {
+ warning ("deleting %#T with virtual functions,"
+ " protected constructor and %s"
+ " non-virtual destructor",
+ classtype,
+ (dtor && TREE_PRIVATE (dtor)
+ ? "private" : "public"));
+ break;
+ }
+ }
+ }
+
+ /* Check for protected non-virtual dtor. */
+ else if (dtor && !DECL_VIRTUAL_P (dtor) && TREE_PROTECTED (dtor))
+ warning ("deleting %#T with protected"
+ " non-virtual destructor", classtype);
+ }
+ }
+
/* Deleting a pointer with the value zero is valid and has no effect. */
if (integer_zerop (t))
return build1 (NOP_EXPR, void_type_node, t);



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]