[C++ Patch] PR 70466 ("ICE on invalid code in tree check: expected constructor, have parm_decl in convert_like_real...")

[C++ Patch] PR 70466 ("ICE on invalid code in tree check: expected constructor, have parm_decl in convert_like_real...")

[Paolo Carlini]

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

Hi,

in this ICE during error recovery, the check in convert_like_real:

         if (CONSTRUCTOR_NELTS (expr) == 0
         && FUNCTION_FIRST_USER_PARMTYPE (convfn) != void_list_node)

is reached for a PARM_DECL as expr. I think that the correct way to 
avoid in general such problem is adding (here too, as elsewhere) a check 
that BRACE_ENCLOSED_INITIALIZER_P (expr) is true to the outer 
conditional, for sure because talking about "converting to %qT from 
initializer list would use explicit constructor %qD", which happens 
anyway in the above conditional, otherwise doesn't make sense. Tested 
x86_64-linux.

Thanks,
Paolo.

/////////////////////////////

[-- Attachment #2: CL_70466 --]
[-- Type: text/plain, Size: 279 bytes --]

/cp
2016-05-16  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/70466
	* call.c (convert_like_real): Check that we are actually converting
	from an init list.

/testsuite
2016-05-16  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/70466
	* g++.dg/template/crash122.C: New.

[-- Attachment #3: patch_70466 --]
[-- Type: text/plain, Size: 1222 bytes --]

Index: cp/call.c
===================================================================
--- cp/call.c	(revision 236300)
+++ cp/call.c	(working copy)
@@ -6377,6 +6377,7 @@ convert_like_real (conversion *convs, tree expr, t
 	/* When converting from an init list we consider explicit
 	   constructors, but actually trying to call one is an error.  */
 	if (DECL_NONCONVERTING_P (convfn) && DECL_CONSTRUCTOR_P (convfn)
+	    && BRACE_ENCLOSED_INITIALIZER_P (expr)
 	    /* Unless this is for direct-list-initialization.  */
 	    && !DIRECT_LIST_INIT_P (expr)
 	    /* And in C++98 a default constructor can't be explicit.  */
Index: testsuite/g++.dg/template/crash122.C
===================================================================
--- testsuite/g++.dg/template/crash122.C	(revision 0)
+++ testsuite/g++.dg/template/crash122.C	(working copy)
@@ -0,0 +1,27 @@
+// PR c++/70466
+
+template < class T, class T >  // { dg-error "conflicting" }
+class A
+{
+public:
+  explicit A (T (S::*f) ()) {}  // { dg-error "expected" }
+};
+
+template < class T, class S > 
+A < T, S > foo (T (S::*f) ())
+{
+  return A < T, S > (f);
+}
+
+class B
+{
+public:
+  void bar () {}
+};
+
+int
+main ()
+{
+  foo (&B::bar);
+  return 0;
+}

Re: [C++ Patch] PR 70466 ("ICE on invalid code in tree check: expected constructor, have parm_decl in convert_like_real...")

[Paolo Carlini]

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

... alternately, if the substance of my patchlet is right, we could 
simplify a bit the logic per the below.

Thanks,
Paolo.

/////////////////////

[-- Attachment #2: patch_70466_2 --]
[-- Type: text/plain, Size: 1303 bytes --]

Index: cp/call.c
===================================================================
--- cp/call.c	(revision 236309)
+++ cp/call.c	(working copy)
@@ -6377,8 +6377,9 @@ convert_like_real (conversion *convs, tree expr, t
 	/* When converting from an init list we consider explicit
 	   constructors, but actually trying to call one is an error.  */
 	if (DECL_NONCONVERTING_P (convfn) && DECL_CONSTRUCTOR_P (convfn)
+	    && BRACE_ENCLOSED_INITIALIZER_P (expr)
 	    /* Unless this is for direct-list-initialization.  */
-	    && !DIRECT_LIST_INIT_P (expr)
+	    && !CONSTRUCTOR_IS_DIRECT_INIT (expr)
 	    /* And in C++98 a default constructor can't be explicit.  */
 	    && cxx_dialect >= cxx11)
 	  {
Index: testsuite/g++.dg/template/crash122.C
===================================================================
--- testsuite/g++.dg/template/crash122.C	(revision 0)
+++ testsuite/g++.dg/template/crash122.C	(working copy)
@@ -0,0 +1,27 @@
+// PR c++/70466
+
+template < class T, class T >  // { dg-error "conflicting" }
+class A
+{
+public:
+  explicit A (T (S::*f) ()) {}  // { dg-error "expected" }
+};
+
+template < class T, class S > 
+A < T, S > foo (T (S::*f) ())
+{
+  return A < T, S > (f);
+}
+
+class B
+{
+public:
+  void bar () {}
+};
+
+int
+main ()
+{
+  foo (&B::bar);
+  return 0;
+}

Re: [C++ Patch] PR 70466 ("ICE on invalid code in tree check: expected constructor, have parm_decl in convert_like_real...")

[Jason Merrill]

On 05/17/2016 04:47 AM, Paolo Carlini wrote:
> ... alternately, if the substance of my patchlet is right, we could
> simplify a bit the logic per the below.

Here's a well-formed variant that was accepted by 4.5.  Does your patch 
fix it?  I also think with your patch we can drop the C++11 check, since 
list-initialization doesn't exist in C++98.

template < class T, class S >
struct A
{
   explicit A (...) {}
};

template < class T, class S >
A < T, S > foo (T (S::*f) ())
{
   return A < T, S > (f);
}

struct B
{
   void bar () {}
};

int
main ()
{
   foo (&B::bar);
   return 0;
}

Re: [C++ Patch] PR 70466 ("ICE on invalid code in tree check: expected constructor, have parm_decl in convert_like_real...")

[Paolo Carlini]

Hi,

On 17/05/2016 20:15, Jason Merrill wrote:
> On 05/17/2016 04:47 AM, Paolo Carlini wrote:
>> ... alternately, if the substance of my patchlet is right, we could
>> simplify a bit the logic per the below.
>
> Here's a well-formed variant that was accepted by 4.5.  Does your 
> patch fix it?  I also think with your patch we can drop the C++11 
> check, since list-initialization doesn't exist in C++98.
Oh nice, the new testcase indeed passes with my patch. However, removing 
completely C++11 check causes a regression in c++98 mode for 
init/explicit1.C, we start warning for it:

struct A { explicit A(int = 0); };
struct B { A a; };

int main()
{
   B b = {};            // { dg-warning "explicit" "" { target c++11 } }
}

(just checked, apparently clang too accepts init/explicit1.C in c++98)

Thanks,
Paolo.

Re: [C++ Patch] PR 70466 ("ICE on invalid code in tree check: expected constructor, have parm_decl in convert_like_real...")

[Jason Merrill]

On 05/17/2016 05:57 PM, Paolo Carlini wrote:
> On 17/05/2016 20:15, Jason Merrill wrote:
>> On 05/17/2016 04:47 AM, Paolo Carlini wrote:
>>> ... alternately, if the substance of my patchlet is right, we could
>>> simplify a bit the logic per the below.
>>
>> Here's a well-formed variant that was accepted by 4.5.  Does your
>> patch fix it?  I also think with your patch we can drop the C++11
>> check, since list-initialization doesn't exist in C++98.
> Oh nice, the new testcase indeed passes with my patch. However, removing
> completely C++11 check causes a regression in c++98 mode for
> init/explicit1.C, we start warning for it:

Ah, that makes sense.  Your patch is OK, then.

Jason

Re: [C++ Patch] PR 70466 ("ICE on invalid code in tree check: expected constructor, have parm_decl in convert_like_real...")

[Paolo Carlini]

Hi,

On 18/05/2016 16:08, Jason Merrill wrote:
> On 05/17/2016 05:57 PM, Paolo Carlini wrote:
>> On 17/05/2016 20:15, Jason Merrill wrote:
>>> On 05/17/2016 04:47 AM, Paolo Carlini wrote:
>>>> ... alternately, if the substance of my patchlet is right, we could
>>>> simplify a bit the logic per the below.
>>>
>>> Here's a well-formed variant that was accepted by 4.5.  Does your
>>> patch fix it?  I also think with your patch we can drop the C++11
>>> check, since list-initialization doesn't exist in C++98.
>> Oh nice, the new testcase indeed passes with my patch. However, removing
>> completely C++11 check causes a regression in c++98 mode for
>> init/explicit1.C, we start warning for it:
>
> Ah, that makes sense.  Your patch is OK, then.
Committed. Since you noticed that actually this is a regression, please 
let me know in which branches we want to fix it, I would guess at least 
gcc-6-branch too.

Thanks,
Paolo.

Re: [C++ Patch] PR 70466 ("ICE on invalid code in tree check: expected constructor, have parm_decl in convert_like_real...")

[Jason Merrill]

On 05/18/2016 10:22 AM, Paolo Carlini wrote:
> Hi,
>
> On 18/05/2016 16:08, Jason Merrill wrote:
>> On 05/17/2016 05:57 PM, Paolo Carlini wrote:
>>> On 17/05/2016 20:15, Jason Merrill wrote:
>>>> On 05/17/2016 04:47 AM, Paolo Carlini wrote:
>>>>> ... alternately, if the substance of my patchlet is right, we could
>>>>> simplify a bit the logic per the below.
>>>>
>>>> Here's a well-formed variant that was accepted by 4.5.  Does your
>>>> patch fix it?  I also think with your patch we can drop the C++11
>>>> check, since list-initialization doesn't exist in C++98.
>>> Oh nice, the new testcase indeed passes with my patch. However, removing
>>> completely C++11 check causes a regression in c++98 mode for
>>> init/explicit1.C, we start warning for it:
>>
>> Ah, that makes sense.  Your patch is OK, then.
> Committed. Since you noticed that actually this is a regression, please
> let me know in which branches we want to fix it, I would guess at least
> gcc-6-branch too.

All the release branches, I think; it looks very safe.

Jason