[C++/66443]

[C++/66443]

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

This patch fixes 66443, a defect introduced by a C++14 change concerning virtual 
bases of abstract classes.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66443 points at DR1611 & 1658, 
which describe the issue.  Essentially, an abstract class type can only exist as 
a base of some other (non-abstract) class.  Any virtual bases of the abstract 
class will be constructed by the containing class itself.  In this specific 
instance, a deleted default ctor of such a virtual base should not delete the 
abstract class's default dtor.

built &  tested on x6-64-linux, ok?

nathan

[-- Attachment #2: 66443.patch --]
[-- Type: text/x-patch, Size: 2259 bytes --]

2015-06-29  Nathan Sidwell  <nathan@acm.org>

	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.

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

Index: cp/init.c
===================================================================
--- cp/init.c	(revision 225103)
+++ cp/init.c	(working copy)
@@ -1139,9 +1139,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;
 
@@ -1155,6 +1153,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 225103)
+++ cp/method.c	(working copy)
@@ -1505,7 +1505,10 @@ 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
+	   /* C++14 ignores virtual bases of abstract classes, as they
+	      can never be directly called.  */
+	   && (cxx_dialect < cxx14 || !ABSTRACT_CLASS_TYPE_P (ctype)))
     {
       if (constexpr_p)
 	*constexpr_p = false;
Index: testsuite/g++.dg/cpp1y/pr66443.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr66443.C	(revision 0)
+++ testsuite/g++.dg/cpp1y/pr66443.C	(working copy)
@@ -0,0 +1,22 @@
+// PR c++/66443
+// { dg-do compile { target c++14 } }
+
+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() {  }
+};
+

Re: [C++/66443]

[Jason Merrill]

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.

Jason

Re: [C++/66443]

[Nathan Sidwell]

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.

ok, works for me.

(sorry for failing  to complete the subject  line.)

nathan

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

[Nathan Sidwell]

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 ...

nathan

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

[Jason Merrill]

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.

Jason

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

[Nathan Sidwell]

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.

nathan

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

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 1775 bytes --]

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

[-- Attachment #2: 66443-2.patch --]
[-- Type: text/x-patch, Size: 2403 bytes --]

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() {  }
+};
+

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

[Jason Merrill]

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

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

[Nathan Sidwell]

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