diff mbox

[C++/66443] virtual base of abstract class

Message ID 55BD56D0.601@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Aug. 1, 2015, 11:31 p.m. UTC
On 07/17/15 15:59, Nathan Sidwell wrote:
> On 07/17/15 15:42, Jason Merrill wrote:
>> On 07/08/2015 10:50 AM, Nathan Sidwell wrote:
>>> On 06/30/15 19:21, Nathan Sidwell wrote:
>>>> On 06/30/15 00:19, Jason Merrill wrote:
>>>>> On 06/29/2015 06:57 PM, Nathan Sidwell wrote:
>>>>>>     * method.c (synthesized_method_walk): Skip virtual bases of
>>>>>>     abstract classes in C++14 mode.
>>>>>
>>>>> Let's not limit this to C++14 mode; most DRs apply to earlier
>>>>> standards as well.
>>>
>>> curiously opening it up leads to some test failures related to
>>> determining the exception specifier for implicit ctors and dtors.  Not
>>> had time to investigate that yet ...
>>
>> If C++98 mode is problematic, we can limit this to C++11 and up.
>
> I'm not yet sure.  The failure mode I saw surprised me, and suggests there's
> something wrong with the patch.  Sadly, I've got interrupted by other stuff.

Ok, this patch fixes things up.  The previous version was a little too lax, 
extending the logic of DR1611 to all synthesized functions.  However, this broke 
virtual synthesized dtors, in that an abstract class's synthesized dtor's 
exception specification would not take account of any virtual base dtor 
exception specs.  This would mean that a non-abstract derived class's 
synthesized dtor might end up with a throwing exception spec (because the 
virtual base's dtor did), and that would be looser than the exception spec on 
the abstract base's non-callable synthesized dtor.  And that fails the virtual 
overriding checks.

So this restricts the skipping to exactly what DR 1611 discusses -- default 
constructors of virtual bases of an abstract class.  Those can never be virtual, 
so we don't end up with the above problem in that case.

Jason, WDYT?

nathan

Comments

Jason Merrill Aug. 3, 2015, 3:44 a.m. UTC | #1
On 08/01/2015 07:31 PM, Nathan Sidwell wrote:
> Ok, this patch fixes things up.  The previous version was a little too
> lax, extending the logic of DR1611 to all synthesized functions.
> However, this broke virtual synthesized dtors, in that an abstract
> class's synthesized dtor's exception specification would not take
> account of any virtual base dtor exception specs.  This would mean that
> a non-abstract derived class's synthesized dtor might end up with a
> throwing exception spec (because the virtual base's dtor did), and that
> would be looser than the exception spec on the abstract base's
> non-callable synthesized dtor.  And that fails the virtual overriding
> checks.

It seems to me that DR 1658 ignores vbases of abstract classes for 
determining whether a destructor is deleted, but says nothing about 
exception specifications.

DR 1351 specifically ignores vbases of abstract classes for determining 
the exception specification of a constructor, but only for constructors.

So I think that for destructors we want to walk the base, but pass in a 
fake delete_p.

Why the check for inherited_parms?  I would think that inheriting 
constructors would be handled like other ctors.

Jason
Nathan Sidwell Aug. 3, 2015, 1:20 p.m. UTC | #2
On 08/02/15 23:44, Jason Merrill wrote:

> It seems to me that DR 1658 ignores vbases of abstract classes for determining
> whether a destructor is deleted, but says nothing about exception specifications.
>
> DR 1351 specifically ignores vbases of abstract classes for determining the
> exception specification of a constructor, but only for constructors.
>
> So I think that for destructors we want to walk the base, but pass in a fake
> delete_p.

Ok. that all makes sense.

> Why the check for inherited_parms?  I would think that inheriting constructors
> would be handled like other ctors.

I think I continue to be confused ...

nathan
diff mbox

Patch

2015-08-01  Nathan Sidwell  <nathan@acm.org>

	cp/
	PR c++/66443
	* init.c (emit_mem_initializers): Do not emit initializers for
	virtual bases of abstract classes.
	* method.c (synthesized_method_walk): Skip virtual bases of
	abstract classes in C++14 mode.

	testsuite/
	PR c++/66443
	* cpp1y/pr66443.C: New test.

Index: cp/init.c
===================================================================
--- cp/init.c	(revision 226444)
+++ cp/init.c	(working copy)
@@ -1140,9 +1140,7 @@  emit_mem_initializers (tree mem_inits)
 	}
 
       /* Initialize the base.  */
-      if (BINFO_VIRTUAL_P (subobject))
-	construct_virtual_base (subobject, arguments);
-      else
+      if (!BINFO_VIRTUAL_P (subobject))
 	{
 	  tree base_addr;
 
@@ -1156,6 +1154,8 @@  emit_mem_initializers (tree mem_inits)
                               tf_warning_or_error);
 	  expand_cleanup_for_base (subobject, NULL_TREE);
 	}
+      else if (!ABSTRACT_CLASS_TYPE_P (current_class_type))
+	construct_virtual_base (subobject, arguments);
     }
   in_base_initializer = 0;
 
Index: cp/method.c
===================================================================
--- cp/method.c	(revision 226444)
+++ cp/method.c	(working copy)
@@ -1506,7 +1506,13 @@  synthesized_method_walk (tree ctype, spe
   vbases = CLASSTYPE_VBASECLASSES (ctype);
   if (vec_safe_is_empty (vbases))
     /* No virtual bases to worry about.  */;
-  else if (!assign_p)
+  else if (!assign_p
+	   /* DR 1611 ignores virtual bases of abstract classes for
+	      the default constructor.  Such ctors can never be
+	      directly called.  Thus their deletion should not affect
+	      whether they are deleted in this class.   */
+	   && (!ABSTRACT_CLASS_TYPE_P (ctype) || sfk != sfk_constructor
+	       || inherited_parms))
     {
       if (constexpr_p)
 	*constexpr_p = false;
Index: testsuite/g++.dg/other/pr66443.C
===================================================================
--- testsuite/g++.dg/other/pr66443.C	(revision 0)
+++ testsuite/g++.dg/other/pr66443.C	(working copy)
@@ -0,0 +1,21 @@ 
+// { dg-do compile }
+
+class A {
+public:
+    A( int ) {  }
+};
+
+// B's virtual base is ignored for default ctor determination as B is
+// abstract.  DR1611 &  DR1658
+
+class B: virtual public A {
+public:
+    virtual void do_something() = 0;
+};
+
+class C: public B {
+public:
+    C(): A( 1 ) {  }
+    virtual void do_something() {  }
+};
+