diff mbox

[C++] Weffc++/Wnon-virtual-dtor confusion

Message ID 533967BE.1020301@acm.org
State New
Headers show

Commit Message

Nathan Sidwell March 31, 2014, 1:03 p.m. UTC
This patch fixes some unexpected behaviour of the Weffc++ and Wnon-virtual-dtor 
flags.  The documentation for the latter says it's also enabled by Weffc++, but 
that's untrue.

The current behaviour of Weffc++ is to warn about any non-virtual dtor in a 
non-polymorphic base class (relying on the below described -Wnon-virtual-dtor 
case to catch polymorphic classes).  Since Edition 3 of Scott Meyer's book, the 
rule has been that it only applies to polymorphic classes.  I.e.  a polymorphic 
class should not have an accessible non-virtual destructor and all of its bases 
should be likewise. (it's possible earlier editions stated this rule too, but 
3rd ed was what I could find).

The current behaviour of Wnon-virtual-dtor is to complain about accessible 
non-virtual dtors in polymorphic classes.  This is what one expects.

One surprising outcome of the current implementation is that -Weffc++ 
-Wno-non-virtual-dtor still complains about the lack of virtual dtors in base 
classes.

This patch:
1) Clarifies the documentation for -Wnon-virtual-dtor to be for polymorphic 
classes and bases of such classes. (i.e. move the 3rd ed Weffc++ behaviour into 
this flag).

2) As different editions of Scott's book renumber the rules, I removed the 
numbering in Weffc++ and merged the two separate lists.

3) Made -Wnon-virtual-dtor get set from -Weffc++.

4) Moved the check for nonvirtual dtors in base classes to after we determine 
the class is polymorphic.  Warn for all bases, not just the non-polymorphic ones.

5) Made that check trigger on -Wnon-virtual-dtor, not Weffc++.

I removed the 3 scattered tests for non-virtual dtor checking and updated and 
cloned the warn/Wnvdtor.C test to check for the interactions between the two flags.

built & tested on i686-pc-linux-gnu.  I'll commit in a few days, unless there 
are comments.

nathan
2014-03-31  Nathan Sidwell  <nathan@codesourcery.com>

	doc/
	* invoke.texi (Wnon-virtual-dtor): Adjust documentation.
	(Weffc++): Remove Scott's numbering, merge lists and reference
	Wnon-virtual-dtor.

	c-family/
	* c.opt (Wnon-virtual-dtor): Auto set when Weffc++.

	cp/
	* class.c (accessible_nvdtor_p): New.
	(check_bases): Don't check base destructor here ...
	(check_bases_and_members): ... check them here.  Trigger on
	Wnon-virtual-dtor flag/
	(finish_struct_1): Use accessible_nvdtor_p.

	testsuite/
	* g++.dg/warn/Wnvdtor.C: Add non-polymorphic case.
	* g++.dg/warn/Wnvdtor-2.C: New.
	* g++.dg/warn/Wnvdtor-3.C: New.
	* g++.dg/warn/Wnvdtor-4.C: New.
	* g++.dg/warn/Weff1.C: Delete.
	* g++.old-deja/g++.benjamin/15309-1.C: Delete.
	* g++.old-deja/g++.benjamin/15309-2.C: Delete.

Comments

Markus Trippelsdorf April 4, 2014, 4:38 p.m. UTC | #1
On 2014.03.31 at 14:03 +0100, Nathan Sidwell wrote:
> This patch fixes some unexpected behaviour of the Weffc++ and Wnon-virtual-dtor 
> flags.  The documentation for the latter says it's also enabled by Weffc++, but 
> that's untrue.
> 
> The current behaviour of Weffc++ is to warn about any non-virtual dtor in a 
> non-polymorphic base class (relying on the below described -Wnon-virtual-dtor 
> case to catch polymorphic classes).  Since Edition 3 of Scott Meyer's book, the 
> rule has been that it only applies to polymorphic classes.  I.e.  a polymorphic 
> class should not have an accessible non-virtual destructor and all of its bases 
> should be likewise. (it's possible earlier editions stated this rule too, but 
> 3rd ed was what I could find).
> 
> The current behaviour of Wnon-virtual-dtor is to complain about accessible 
> non-virtual dtors in polymorphic classes.  This is what one expects.
> 
> One surprising outcome of the current implementation is that -Weffc++ 
> -Wno-non-virtual-dtor still complains about the lack of virtual dtors in base 
> classes.
> 
> This patch:
> 1) Clarifies the documentation for -Wnon-virtual-dtor to be for polymorphic 
> classes and bases of such classes. (i.e. move the 3rd ed Weffc++ behaviour into 
> this flag).

I'm not sure that this is a good idea. This changes the existing
behavior of -Wnon-virtual-dtor and causes hundreds of new warnings when
building LLVM. Wouldn't it make more sense to move the 3rd ed Weffc++
behavior to the -Weffc++ flag alone?
Nathan Sidwell April 4, 2014, 4:48 p.m. UTC | #2
On 04/04/14 17:38, Markus Trippelsdorf wrote:

> I'm not sure that this is a good idea. This changes the existing
> behavior of -Wnon-virtual-dtor and causes hundreds of new warnings when
> building LLVM. Wouldn't it make more sense to move the 3rd ed Weffc++
> behavior to the -Weffc++ flag alone?

IIUC you're using -Wnon-virtual-dtor on its own, is that right?  What are the 
class shapes where you're seeing this behaviour?

AFAICT the -Wnon-virtual-dtor warning was an attempt to separate out that 
particular -Weffc++ warning, but it was broken.

nathan
Markus Trippelsdorf April 4, 2014, 4:54 p.m. UTC | #3
On 2014.04.04 at 17:48 +0100, Nathan Sidwell wrote:
> On 04/04/14 17:38, Markus Trippelsdorf wrote:
> 
> > I'm not sure that this is a good idea. This changes the existing
> > behavior of -Wnon-virtual-dtor and causes hundreds of new warnings when
> > building LLVM. Wouldn't it make more sense to move the 3rd ed Weffc++
> > behavior to the -Weffc++ flag alone?
> 
> IIUC you're using -Wnon-virtual-dtor on its own, is that right?  What are the 
> class shapes where you're seeing this behaviour?
> 
> AFAICT the -Wnon-virtual-dtor warning was an attempt to separate out that 
> particular -Weffc++ warning, but it was broken.

For example:

markus@x4 tmp % cat test.ii
class Option
{
public:
  virtual ~Option ();
};
template <int> class opt_storage
{
};
template <int = 0> class A : Option, opt_storage<0>
{
};
template class A<>;

markus@x4 tmp % g++ -Wnon-virtual-dtor -std=c++11 -c test.ii
test.ii: In instantiation of ‘class A<>’:
test.ii:12:16:   required from here
test.ii:9:26: warning: base class ‘class opt_storage<0>’ has accessible non-virtual destructor [-Wnon-virtual-dtor]
 template <int = 0> class A : Option, opt_storage<0>
                          ^
markus@x4 tmp % clang++ -Wnon-virtual-dtor -std=c++11 -c test.ii
markus@x4 tmp %
(Before your commit)
markus@x4 tmp % g++ -Wnon-virtual-dtor -std=c++11 -c test.ii
markus@x4 tmp %
Nathan Sidwell April 4, 2014, 5:04 p.m. UTC | #4
On 04/04/14 17:54, Markus Trippelsdorf wrote:

> markus@x4 tmp % g++ -Wnon-virtual-dtor -std=c++11 -c test.ii
> test.ii: In instantiation of ‘class A<>’:
> test.ii:12:16:   required from here
> test.ii:9:26: warning: base class ‘class opt_storage<0>’ has accessible non-virtual destructor [-Wnon-virtual-dtor]
>   template <int = 0> class A : Option, opt_storage<0>

ah, you've hit on the one case I was unsure about -- IMHO Scott's rule about 
bases is incomplete.  The rule should be for publicly accessible bases only -- 
opt_storage is private.

The aim of the rule is to make sure that random code doing:

   A_Base_type *ptr = ptr_to_derived; // implicit base cast
   ...
   delete ptr;

behaved as one might expect and invoke the final polymorphic dtor.  Such an 
implicit cast to a private base can't happen outside of the class.  Inside the 
class one's expected to know what one's doing.

I'm fine with adding a TREE_PUBLIC (base_binfo) check into the loop in c_b_a_m. 
  Would that work for you?

nathan
Jason Merrill April 4, 2014, 5:19 p.m. UTC | #5
On 04/04/2014 01:04 PM, Nathan Sidwell wrote:
> On 04/04/14 17:54, Markus Trippelsdorf wrote:
>
>> markus@x4 tmp % g++ -Wnon-virtual-dtor -std=c++11 -c test.ii
>> test.ii: In instantiation of ‘class A<>’:
>> test.ii:12:16:   required from here
>> test.ii:9:26: warning: base class ‘class opt_storage<0>’ has
>> accessible non-virtual destructor [-Wnon-virtual-dtor]
>>   template <int = 0> class A : Option, opt_storage<0>
>
> ah, you've hit on the one case I was unsure about -- IMHO Scott's rule
> about bases is incomplete.  The rule should be for publicly accessible
> bases only -- opt_storage is private.

I also notice that the opt_storage destructor is implicitly declared and 
trivial; it seems excessive to warn in that case.  I'm also somewhat 
skeptical about extending the warning to non-polymorphic bases at all, 
at least in the absence of -Weffc++.

Since this change is causing problems, I'm inclined to back it out until 
after 4.9 branches.

Jason
Markus Trippelsdorf April 4, 2014, 5:23 p.m. UTC | #6
On 2014.04.04 at 18:04 +0100, Nathan Sidwell wrote:
> On 04/04/14 17:54, Markus Trippelsdorf wrote:
> 
> > markus@x4 tmp % g++ -Wnon-virtual-dtor -std=c++11 -c test.ii
> > test.ii: In instantiation of ‘class A<>’:
> > test.ii:12:16:   required from here
> > test.ii:9:26: warning: base class ‘class opt_storage<0>’ has accessible non-virtual destructor [-Wnon-virtual-dtor]
> >   template <int = 0> class A : Option, opt_storage<0>
> 
> ah, you've hit on the one case I was unsure about -- IMHO Scott's rule about 
> bases is incomplete.  The rule should be for publicly accessible bases only -- 
> opt_storage is private.
> 
> The aim of the rule is to make sure that random code doing:
> 
>    A_Base_type *ptr = ptr_to_derived; // implicit base cast
>    ...
>    delete ptr;
> 
> behaved as one might expect and invoke the final polymorphic dtor.  Such an 
> implicit cast to a private base can't happen outside of the class.  Inside the 
> class one's expected to know what one's doing.
> 
> I'm fine with adding a TREE_PUBLIC (base_binfo) check into the loop in c_b_a_m. 
>   Would that work for you?

Yes. Thanks.
Nathan Sidwell April 4, 2014, 5:33 p.m. UTC | #7
On 04/04/14 18:19, Jason Merrill wrote:

> I also notice that the opt_storage destructor is implicitly declared and
> trivial; it seems excessive to warn in that case.

I disagree, I think that's exactly a case that should be warned about -- because 
it's easy to overlook.


> I'm also somewhat skeptical
> about extending the warning to non-polymorphic bases at all, at least in the
> absence of -Weffc++.

That's certainly plausible.

> Since this change is causing problems, I'm inclined to back it out until after
> 4.9 branches.

That too.

I'm testing a patch that makes the test in the loop:

	  if (TREE_PUBLIC (base_binfo)
	      && (TYPE_POLYMORPHIC_P (basetype) || warn_ecpp)
	      && accessible_nvdtor_p (basetype))

but I agree about reverting the patch if there's still further discussion 
following that suggested change.

nathan
Nathan Sidwell April 6, 2014, 8:13 a.m. UTC | #8
On 04/04/14 18:33, Nathan Sidwell wrote:

> I'm testing a patch that makes the test in the loop:
>
>        if (TREE_PUBLIC (base_binfo)

Hm, binfo's aren't noted that way, it's encoded in BINFO_ACCESS and 
SET_BINFO_ACCESS in search.c.  We'd need to move those back to cp.h or expose an 
interface in search.c.  This is looking like a rat hole ...

nathan
Markus Trippelsdorf April 6, 2014, 9:50 a.m. UTC | #9
On 2014.04.06 at 09:13 +0100, Nathan Sidwell wrote:
> On 04/04/14 18:33, Nathan Sidwell wrote:
> 
> > I'm testing a patch that makes the test in the loop:
> >
> >        if (TREE_PUBLIC (base_binfo)
> 
> Hm, binfo's aren't noted that way, it's encoded in BINFO_ACCESS and 
> SET_BINFO_ACCESS in search.c.  We'd need to move those back to cp.h or expose an 
> interface in search.c.  This is looking like a rat hole ...

Thus a revert looks like the best option at this point.
diff mbox

Patch

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 208870)
+++ gcc/c-family/c.opt	(working copy)
@@ -569,7 +569,7 @@  C++ ObjC++ Var(warn_nontemplate_friend)
 Warn when non-templatized friend functions are declared within a template
 
 Wnon-virtual-dtor
-C++ ObjC++ Var(warn_nonvdtor) Warning
+C++ ObjC++ Var(warn_nonvdtor) Warning LangEnabledBy(C++ ObjC++,Weffc++)
 Warn about non-virtual destructors
 
 Wnonnull
Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 208870)
+++ gcc/cp/class.c	(working copy)
@@ -149,6 +149,7 @@  static tree *build_base_field (record_la
 static void build_base_fields (record_layout_info, splay_tree, tree *);
 static void check_methods (tree);
 static void remove_zero_width_bit_fields (tree);
+static bool accessible_nvdtor_p (tree);
 static void check_bases (tree, int *, int *);
 static void check_bases_and_members (tree);
 static tree create_vtable_ptr (tree, tree *);
@@ -1476,6 +1477,33 @@  inherit_targ_abi_tags (tree t)
   mark_type_abi_tags (t, false);
 }
 
+/* Return true, iff class T has a non-virtual destructor that is
+   accessible from outside the class heirarchy (i.e. is public, or
+   there's a suitable friend.  */
+
+static bool
+accessible_nvdtor_p (tree t)
+{
+  tree dtor = CLASSTYPE_DESTRUCTORS (t);
+
+  /* An implicitly declared destructor is always public.  And,
+     if it were virtual, we would have created it by now.  */
+  if (!dtor)
+    return true;
+
+  if (DECL_VINDEX (dtor))
+    return false; /* Virtual */
+  
+  if (!TREE_PRIVATE (dtor) && !TREE_PROTECTED (dtor))
+    return true;  /* Public */
+
+  if (CLASSTYPE_FRIEND_CLASSES (t)
+      || DECL_FRIENDLIST (TYPE_MAIN_DECL (t)))
+    return true;   /* Has friends */
+
+  return false;
+}
+
 /* Run through the base classes of T, updating CANT_HAVE_CONST_CTOR_P,
    and NO_CONST_ASN_REF_P.  Also set flag bits in T based on
    properties of the bases.  */
@@ -1512,13 +1540,6 @@  check_bases (tree t,
       if (!CLASSTYPE_LITERAL_P (basetype))
         CLASSTYPE_LITERAL_P (t) = false;
 
-      /* Effective C++ rule 14.  We only need to check TYPE_POLYMORPHIC_P
-	 here because the case of virtual functions but non-virtual
-	 dtor is handled in finish_struct_1.  */
-      if (!TYPE_POLYMORPHIC_P (basetype))
-	warning (OPT_Weffc__,
-		 "base class %q#T has a non-virtual destructor", basetype);
-
       /* If the base class doesn't have copy constructors or
 	 assignment operators that take const references, then the
 	 derived class cannot have such a member automatically
@@ -5547,6 +5568,27 @@  check_bases_and_members (tree t)
   TYPE_HAS_COMPLEX_MOVE_ASSIGN (t) |= TYPE_CONTAINS_VPTR_P (t);
   TYPE_HAS_COMPLEX_DFLT (t) |= TYPE_CONTAINS_VPTR_P (t);
 
+  /* Warn if a base of a polymorphic type has an accessible
+     non-virtual destructor.  It is only now that we know the class is
+     polymorphic.  Although a polymorphic base will have a already
+     been diagnosed during its definition, we warn on use too.  */
+  if (TYPE_POLYMORPHIC_P (t) && warn_nonvdtor)
+    {
+      tree binfo, base_binfo;
+      unsigned i;
+      
+      for (binfo = TYPE_BINFO (t), i = 0;
+	   BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
+	{
+	  tree basetype = TREE_TYPE (base_binfo);
+
+	  if (accessible_nvdtor_p (basetype))
+	    warning (OPT_Wnon_virtual_dtor,
+		     "base class %q#T has accessible non-virtual destructor",
+		     basetype);
+	}
+    }
+  
   /* If the class has no user-declared constructor, but does have
      non-static const or reference data members that can never be
      initialized, issue a warning.  */
@@ -6597,25 +6639,11 @@  finish_struct_1 (tree t)
 
   /* This warning does not make sense for Java classes, since they
      cannot have destructors.  */
-  if (!TYPE_FOR_JAVA (t) && warn_nonvdtor && TYPE_POLYMORPHIC_P (t))
-    {
-      tree dtor;
-
-      dtor = CLASSTYPE_DESTRUCTORS (t);
-      if (/* An implicitly declared destructor is always public.  And,
-	     if it were virtual, we would have created it by now.  */
-	  !dtor
-	  || (!DECL_VINDEX (dtor)
-	      && (/* public non-virtual */
-		  (!TREE_PRIVATE (dtor) && !TREE_PROTECTED (dtor))
-		   || (/* non-public non-virtual with friends */
-		       (TREE_PRIVATE (dtor) || TREE_PROTECTED (dtor))
-			&& (CLASSTYPE_FRIEND_CLASSES (t)
-			|| DECL_FRIENDLIST (TYPE_MAIN_DECL (t)))))))
-	warning (OPT_Wnon_virtual_dtor,
-		 "%q#T has virtual functions and accessible"
-		 " non-virtual destructor", t);
-    }
+  if (!TYPE_FOR_JAVA (t) && warn_nonvdtor
+      && TYPE_POLYMORPHIC_P (t) && accessible_nvdtor_p (t))
+    warning (OPT_Wnon_virtual_dtor,
+	     "%q#T has virtual functions and accessible"
+	     " non-virtual destructor", t);
 
   complete_vars (t);
 
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 208870)
+++ gcc/doc/invoke.texi	(working copy)
@@ -2670,9 +2670,10 @@  the compiler to never throw an exception
 @opindex Wnon-virtual-dtor
 @opindex Wno-non-virtual-dtor
 Warn when a class has virtual functions and an accessible non-virtual
-destructor, in which case it is possible but unsafe to delete
-an instance of a derived class through a pointer to the base class.
-This warning is also enabled if @option{-Weffc++} is specified.
+destructor itself or in a base class, or has in which case it is
+possible but unsafe to delete an instance of a derived class through a
+pointer to the base class.  This warning is automatically enabled if
+@option{-Weffc++} is specified.
 
 @item -Wreorder @r{(C++ and Objective-C++ only)}
 @opindex Wreorder
@@ -2716,40 +2717,34 @@  The following @option{-W@dots{}} options
 @opindex Weffc++
 @opindex Wno-effc++
 Warn about violations of the following style guidelines from Scott Meyers'
-@cite{Effective C++, Second Edition} book:
+@cite{Effective C++} series of books:
 
 @itemize @bullet
 @item
-Item 11:  Define a copy constructor and an assignment operator for classes
+Define a copy constructor and an assignment operator for classes
 with dynamically-allocated memory.
 
 @item
-Item 12:  Prefer initialization to assignment in constructors.
+Prefer initialization to assignment in constructors.
 
 @item
-Item 14:  Make destructors virtual in base classes.
+Have @code{operator=} return a reference to @code{*this}.
 
 @item
-Item 15:  Have @code{operator=} return a reference to @code{*this}.
+Don't try to return a reference when you must return an object.
 
 @item
-Item 23:  Don't try to return a reference when you must return an object.
-
-@end itemize
-
-Also warn about violations of the following style guidelines from
-Scott Meyers' @cite{More Effective C++} book:
-
-@itemize @bullet
-@item
-Item 6:  Distinguish between prefix and postfix forms of increment and
+Distinguish between prefix and postfix forms of increment and
 decrement operators.
 
 @item
-Item 7:  Never overload @code{&&}, @code{||}, or @code{,}.
+Never overload @code{&&}, @code{||}, or @code{,}.
 
 @end itemize
 
+This option also enables @option{-Wnon-virtual-dtor}, which is also
+one of the effective C++ recommendations.
+
 When selecting this option, be aware that the standard library
 headers do not obey all of these guidelines; use @samp{grep -v}
 to filter out those warnings.
Index: gcc/testsuite/g++.dg/warn/Weff1.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Weff1.C	(revision 208870)
+++ gcc/testsuite/g++.dg/warn/Weff1.C	(working copy)
@@ -1,5 +0,0 @@ 
-// { dg-options "-Weffc++" }
-
-struct S {};
-/* Base classes should have virtual destructors.  */
-struct T : public S {}; // { dg-warning "" }
Index: gcc/testsuite/g++.dg/warn/Wnvdtor-2.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wnvdtor-2.C	(revision 208870)
+++ gcc/testsuite/g++.dg/warn/Wnvdtor-2.C	(working copy)
@@ -6,18 +6,18 @@ 
 // destructor, in which case it would be possible but unsafe to delete
 // an instance of a derived class through a pointer to the base class.
 
-struct A // { dg-bogus "non-virtual destructor" }
+struct A
 {
 protected:
-  ~A();
+  ~A(); // inaccessible - no warning
 public:
   virtual void f() = 0;
 };
 
-struct B // { dg-bogus "non-virtual destructor" }
+struct B
 {
 private:
-  ~B();
+  ~B(); // inaccessible - no warning
 public:
   virtual void f() = 0;
 };
@@ -52,3 +52,6 @@  private:
 public:
   virtual void f() = 0;
 };
+
+struct H {};
+struct I : H {};
Index: gcc/testsuite/g++.dg/warn/Wnvdtor-3.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wnvdtor-3.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wnvdtor-3.C	(working copy)
@@ -0,0 +1,56 @@ 
+// { dg-do compile }
+// { dg-options "-Weffc++" }
+
+// Warn when a class has virtual functions and accessible non-virtual
+// destructor, in which case it would be possible but unsafe to delete
+// an instance of a derived class through a pointer to the base class.
+
+struct A
+{
+protected:
+  ~A(); // inaccessible - no warning
+public:
+  virtual void f() = 0;
+};
+
+struct B
+{
+private:
+  ~B(); // inaccessible - no warning
+public:
+  virtual void f() = 0;
+};
+
+struct C // { dg-warning "non-virtual destructor" }
+{
+  virtual void f() = 0;
+};
+
+struct D // { dg-warning "non-virtual destructor" }
+{
+  ~D();
+  virtual void f() = 0;
+};
+
+struct E;
+
+struct F // { dg-warning "non-virtual destructor" }
+{
+protected:
+  friend class E;
+  ~F();
+public:
+  virtual void f() = 0;
+};
+
+struct G // { dg-warning "non-virtual destructor" }
+{
+private:
+  friend class E;
+  ~G();
+public:
+  virtual void f() = 0;
+};
+
+struct H {};
+struct I : H {};
Index: gcc/testsuite/g++.dg/warn/Wnvdtor-4.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wnvdtor-4.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wnvdtor-4.C	(working copy)
@@ -0,0 +1,56 @@ 
+// { dg-do compile }
+// { dg-options "-Weffc++ -Wno-non-virtual-dtor" }
+
+// Warn when a class has virtual functions and accessible non-virtual
+// destructor, in which case it would be possible but unsafe to delete
+// an instance of a derived class through a pointer to the base class.
+
+struct A
+{
+protected:
+  ~A();
+public:
+  virtual void f() = 0;
+};
+
+struct B
+{
+private:
+  ~B();
+public:
+  virtual void f() = 0;
+};
+
+struct C
+{
+  virtual void f() = 0;
+};
+
+struct D
+{
+  ~D();
+  virtual void f() = 0;
+};
+
+struct E;
+
+struct F
+{
+protected:
+  friend class E;
+  ~F();
+public:
+  virtual void f() = 0;
+};
+
+struct G
+{
+private:
+  friend class E;
+  ~G();
+public:
+  virtual void f() = 0;
+};
+
+struct H {};
+struct I : H {};
Index: gcc/testsuite/g++.dg/warn/Wnvdtor.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wnvdtor.C	(revision 208870)
+++ gcc/testsuite/g++.dg/warn/Wnvdtor.C	(working copy)
@@ -8,3 +8,4 @@  extern "Java"
     virtual void bar( void);
   };
 }
+
Index: gcc/testsuite/g++.old-deja/g++.benjamin/15309-1.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.benjamin/15309-1.C	(revision 208870)
+++ gcc/testsuite/g++.old-deja/g++.benjamin/15309-1.C	(working copy)
@@ -1,21 +0,0 @@ 
-// { dg-do assemble  }
-// { dg-options "-Wnon-virtual-dtor -Weffc++" }
-// 981203 bkoz
-// g++/15309
-
-class bahamian {
-public:
-  bahamian ();
-  ~bahamian ();  
-};
-
-class miami : public bahamian	// { dg-warning "" } // WARNING -
-{
-public:
-   miami ();
-   ~miami ();
-};
-
-
-
-
Index: gcc/testsuite/g++.old-deja/g++.benjamin/15309-2.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.benjamin/15309-2.C	(revision 208870)
+++ gcc/testsuite/g++.old-deja/g++.benjamin/15309-2.C	(working copy)
@@ -1,10 +0,0 @@ 
-// { dg-do assemble  }
-// { dg-options "-Wnon-virtual-dtor -Weffc++" }
-// 981203 bkoz
-// g++/15309
-
-class bermuda {  // { dg-warning "" } // WARNING -
-public:
-  virtual int func1(int); 
-  ~bermuda();
-};