diff mbox

c++/78771 ICE with inheriting ctor

Message ID 0adea123-1e7d-293c-25a2-ee290fddf678@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Jan. 11, 2017, 3:53 p.m. UTC 
On 01/04/2017 12:53 AM, Jason Merrill wrote:

> Hmm, that seems like where the problem is.  We shouldn't try to
> instantiate the inheriting constructor until we've already chosen the
> base constructor; in the new model the inheriting constructor is just an
> implementation detail.

Oh what fun.  This testcase behaves differently for C++17, C++11 
-fnew-inheriting-ctors and C++11 -fno-new-inheriting-ctors compilation 
modes.

Firstly, unpatched G++ is fine in C++17 mode, because:
   /* In C++17, "If the initializer expression is a prvalue and the
      cv-unqualified version of the source type is the same class as the 
class
      of the destination, the initializer expression is used to 
initialize the
      destination object."  Handle that here to avoid doing overload
      resolution.  */
and inside that we have:

       /* FIXME P0135 doesn't say how to handle direct initialization from a
	 type with a suitable conversion operator.  Let's handle it like
	 copy-initialization, but allowing explict conversions.  */

That conversion lookup short-circuits the subsequent overload resolution 
that would otherwise explode.

Otherwise, with -fnew-inheriting-ctors, you are indeed correct.  There 
needs to be a call to strip_inheriting_ctors in deduce_inheriting_ctor.

With -fno-new-inheriting-ctors we need the original patch I posted 
(included herein).  I suppose we might be able to remove the assert from 
strip_inheriting_ctors and always call that from deduce_inheriting_ctor, 
but that seems a bad idea to me.

I was unable to produce a c++17 testcase that triggered this problem by 
avoiding the above-mentioned overload resolution short-circuiting.

As -fnew-inheriting-ctors is a mangling-affecting flag, I guess we're 
stuck with it for the foreseable future.

ok?

nathan

Comments

Nathan Sidwell Jan. 20, 2017, 4:18 p.m. UTC | #1 
ping?

On 01/11/2017 10:53 AM, Nathan Sidwell wrote:
> On 01/04/2017 12:53 AM, Jason Merrill wrote:
>
>> Hmm, that seems like where the problem is.  We shouldn't try to
>> instantiate the inheriting constructor until we've already chosen the
>> base constructor; in the new model the inheriting constructor is just an
>> implementation detail.
>
> Oh what fun.  This testcase behaves differently for C++17, C++11
> -fnew-inheriting-ctors and C++11 -fno-new-inheriting-ctors compilation
> modes.
>
> Firstly, unpatched G++ is fine in C++17 mode, because:
>   /* In C++17, "If the initializer expression is a prvalue and the
>      cv-unqualified version of the source type is the same class as the
> class
>      of the destination, the initializer expression is used to
> initialize the
>      destination object."  Handle that here to avoid doing overload
>      resolution.  */
> and inside that we have:
>
>       /* FIXME P0135 doesn't say how to handle direct initialization from a
>      type with a suitable conversion operator.  Let's handle it like
>      copy-initialization, but allowing explict conversions.  */
>
> That conversion lookup short-circuits the subsequent overload resolution
> that would otherwise explode.
>
> Otherwise, with -fnew-inheriting-ctors, you are indeed correct.  There
> needs to be a call to strip_inheriting_ctors in deduce_inheriting_ctor.
>
> With -fno-new-inheriting-ctors we need the original patch I posted
> (included herein).  I suppose we might be able to remove the assert from
> strip_inheriting_ctors and always call that from deduce_inheriting_ctor,
> but that seems a bad idea to me.
>
> I was unable to produce a c++17 testcase that triggered this problem by
> avoiding the above-mentioned overload resolution short-circuiting.
>
> As -fnew-inheriting-ctors is a mangling-affecting flag, I guess we're
> stuck with it for the foreseable future.
>
> ok?
>
> nathan
>
diff mbox

Patch

2017-01-11  Nathan Sidwell  <nathan@acm.org>

	PR c++/78771
	* pt.c (instantiate_template_1): Check for recursive instantiation
	of inheriting constructor when not new-inheriting-ctor.
	* method.c (deduce_inheriting_ctor): Use originating ctor when
	new-inheriting-ctor.

	PR c++/78771
	* g++.dg/cpp0x/pr78771-old.C: New.
	* g++.dg/cpp0x/pr78771-new.C: New.
	* g++.dg/cpp1z/pr78771.C: New.

Index: cp/method.c
===================================================================
--- cp/method.c	(revision 244314)
+++ cp/method.c	(working copy)
@@ -1858,11 +1858,15 @@  deduce_inheriting_ctor (tree decl)
   gcc_assert (DECL_INHERITED_CTOR (decl));
   tree spec;
   bool trivial, constexpr_, deleted;
+
+  tree inherited = DECL_INHERITED_CTOR (decl);
+  if (flag_new_inheriting_ctors)
+    inherited = strip_inheriting_ctors (inherited);
+
   synthesized_method_walk (DECL_CONTEXT (decl), sfk_inheriting_constructor,
 			   false, &spec, &trivial, &deleted, &constexpr_,
-			   /*diag*/false,
-			   DECL_INHERITED_CTOR (decl),
-			   FUNCTION_FIRST_USER_PARMTYPE (decl));
+			   /*diag=*/false,
+			   inherited, FUNCTION_FIRST_USER_PARMTYPE (decl));
   if (TREE_CODE (inherited_ctor_binfo (decl)) != TREE_BINFO)
     /* Inherited the same constructor from different base subobjects.  */
     deleted = true;
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 244314)
+++ cp/pt.c	(working copy)
@@ -17963,10 +17963,22 @@  instantiate_template_1 (tree tmpl, tree
       if (spec == error_mark_node)
 	return error_mark_node;
 
+      /* If this is an inherited ctor, we can recursively clone it
+	 when deducing the validity of the ctor.  But we won't have
+	 cloned the function yet, so do it now.  We'll redo this
+	 later, but any recursive information learnt here can't
+	 change the validity.  */
+      if (!TREE_CHAIN (spec))
+	{
+	  gcc_assert (!flag_new_inheriting_ctors
+		      && DECL_INHERITED_CTOR (spec));
+	  clone_function_decl (spec, /*update_method_vec_p=*/0);
+	}
       /* Look for the clone.  */
       FOR_EACH_CLONE (clone, spec)
 	if (DECL_NAME (clone) == DECL_NAME (tmpl))
 	  return clone;
+
       /* We should always have found the clone by now.  */
       gcc_unreachable ();
       return NULL_TREE;
Index: testsuite/g++.dg/cpp0x/pr78771-new.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr78771-new.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr78771-new.C	(working copy)
@@ -0,0 +1,28 @@ 
+// PR c++/78771
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-fnew-inheriting-ctors" }
+
+// ICE instantiating a deleted inherited ctor
+
+struct Base
+{
+  template <typename U> Base (U);
+
+  Base (int);
+};
+
+struct Derived;
+
+struct Middle : Base
+{
+  using Base::Base;
+
+  Middle (Derived);
+};
+
+struct Derived : Middle
+{
+  using Middle::Middle;
+};
+
+Middle::Middle (Derived) : Middle (0) {}
Index: testsuite/g++.dg/cpp0x/pr78771-old.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr78771-old.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr78771-old.C	(working copy)
@@ -0,0 +1,28 @@ 
+// PR c++/78771
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-fno-new-inheriting-ctors" }
+
+// ICE instantiating a deleted inherited ctor
+
+struct Base
+{
+  template <typename U> Base (U);
+
+  Base (int);
+};
+
+struct Derived;
+
+struct Middle : Base
+{
+  using Base::Base;
+
+  Middle (Derived);
+};
+
+struct Derived : Middle
+{
+  using Middle::Middle;
+};
+
+Middle::Middle (Derived) : Middle (0) {}
Index: testsuite/g++.dg/cpp1z/pr78771.C
===================================================================
--- testsuite/g++.dg/cpp1z/pr78771.C	(revision 0)
+++ testsuite/g++.dg/cpp1z/pr78771.C	(working copy)
@@ -0,0 +1,27 @@ 
+// PR c++/78771
+// { dg-options -std=c++1z }
+
+// ICE instantiating a deleted inherited ctor
+
+struct Base
+{
+  template <typename U> Base (U);
+
+  Base (int);
+};
+
+struct Derived;
+
+struct Middle : Base
+{
+  using Base::Base;
+
+  Middle (Derived);
+};
+
+struct Derived : Middle
+{
+  using Middle::Middle;
+};
+
+Middle::Middle (Derived) : Middle (0) {}