C++ PATCH for c++/89212 - ICE converting nullptr to pointer-to-member-function

C++ PATCH for c++/89212 - ICE converting nullptr to pointer-to-member-function

[Marek Polacek]

r256999 removed early bailout for pointer-to-member-function types, so we
now try to tsubst each element of a pointer-to-member-function CONSTRUCTOR.

That's fine but the problem here is that we end up converting a null pointer
to pointer-to-member-function type and that crashes in fold_convert:

16035     case INTEGER_CST:
16039       {
16040         /* Instantiate any typedefs in the type.  */
16041         tree type = tsubst (TREE_TYPE (t), args, complain, in_decl);
16042         r = fold_convert (type, t);

It seems obvious to use cp_fold_convert which handles TYPE_PTRMEM_P, but
that then ICEs too, the infamous "canonical types differ for identical types":
type is

struct
{
  void A::<T344> (struct A *) * __pfn;
  long int __delta;
}

and the type of "0" is "void A::<T344> (struct A *) *".  These types are 
structurally equivalent but have different canonical types.  (What's up
with that, anyway?  It seems OK that the canonical type of the struct is
the struct itself and that the canonical type of the pointer is the pointer
itself.)

That could be handled in cp_fold_convert: add code to convert an integer_zerop to
TYPE_PTRMEMFUNC_P.  Unfortunately the 0 is not null_ptr_cst_p because it's got
a pointer type.

Or just don't bother substituting null_member_pointer_value_p and avoid the
above.

Bootstrapped/regtested on x86_64-linux, ok for trunk and 8?

2019-02-08  Marek Polacek  <polacek@redhat.com>

	PR c++/89212 - ICE converting nullptr to pointer-to-member-function.
	* pt.c (tsubst_copy_and_build) <case CONSTRUCTOR>: Return early for
	null member pointer value.

	* g++.dg/cpp0x/nullptr40.C: New test.

diff --git gcc/cp/pt.c gcc/cp/pt.c
index b8fbf4046f0..acc2d8f1feb 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -19251,6 +19251,9 @@ tsubst_copy_and_build (tree t,
 	   looked up by digest_init.  */
 	process_index_p = !(type && MAYBE_CLASS_TYPE_P (type));
 
+	if (null_member_pointer_value_p (t))
+	  RETURN (t);
+
 	n = vec_safe_copy (CONSTRUCTOR_ELTS (t));
         newlen = vec_safe_length (n);
 	FOR_EACH_VEC_SAFE_ELT (n, idx, ce)
diff --git gcc/testsuite/g++.dg/cpp0x/nullptr40.C gcc/testsuite/g++.dg/cpp0x/nullptr40.C
new file mode 100644
index 00000000000..21c188bdb5e
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/nullptr40.C
@@ -0,0 +1,19 @@
+// PR c++/89212
+// { dg-do compile { target c++11 } }
+
+template <int, typename T> using enable_if_t = int;
+
+template<class X, void(X::*foo)() = nullptr>
+struct p
+{
+    template<void(X::*fun)() = foo, typename T = enable_if_t<nullptr == fun, int>>
+    p(T) { }
+    p() = default;
+};
+
+struct A
+{
+    p<A> i = 1;
+    void bar();
+    p<A, &A::bar> j;
+};

Re: C++ PATCH for c++/89212 - ICE converting nullptr to pointer-to-member-function

[Jason Merrill]

On 2/8/19 12:21 PM, Marek Polacek wrote:
> r256999 removed early bailout for pointer-to-member-function types, so we
> now try to tsubst each element of a pointer-to-member-function CONSTRUCTOR.
> 
> That's fine but the problem here is that we end up converting a null pointer
> to pointer-to-member-function type and that crashes in fold_convert:
> 
> 16035     case INTEGER_CST:
> 16039       {
> 16040         /* Instantiate any typedefs in the type.  */
> 16041         tree type = tsubst (TREE_TYPE (t), args, complain, in_decl);
> 16042         r = fold_convert (type, t);
> 
> It seems obvious to use cp_fold_convert which handles TYPE_PTRMEM_P, but
> that then ICEs too, the infamous "canonical types differ for identical types":
> type is
> 
> struct
> {
>    void A::<T344> (struct A *) * __pfn;
>    long int __delta;
> }
> 
> and the type of "0" is "void A::<T344> (struct A *) *".  These types are
> structurally equivalent but have different canonical types.  (What's up
> with that, anyway?  It seems OK that the canonical type of the struct is
> the struct itself and that the canonical type of the pointer is the pointer
> itself.)
> 
> That could be handled in cp_fold_convert: add code to convert an integer_zerop to
> TYPE_PTRMEMFUNC_P.  Unfortunately the 0 is not null_ptr_cst_p because it's got
> a pointer type.
> 
> Or just don't bother substituting null_member_pointer_value_p and avoid the
> above.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk and 8?
> 
> 2019-02-08  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c++/89212 - ICE converting nullptr to pointer-to-member-function.
> 	* pt.c (tsubst_copy_and_build) <case CONSTRUCTOR>: Return early for
> 	null member pointer value.
> 
> 	* g++.dg/cpp0x/nullptr40.C: New test.
> 
> diff --git gcc/cp/pt.c gcc/cp/pt.c
> index b8fbf4046f0..acc2d8f1feb 100644
> --- gcc/cp/pt.c
> +++ gcc/cp/pt.c
> @@ -19251,6 +19251,9 @@ tsubst_copy_and_build (tree t,
>   	   looked up by digest_init.  */
>   	process_index_p = !(type && MAYBE_CLASS_TYPE_P (type));
>   
> +	if (null_member_pointer_value_p (t))
> +	  RETURN (t);

I would expect this to do the wrong thing if type is different from 
TREE_TYPE (t).  Can we get here for a dependent PMF type like T 
(A::*)()?  If not, let's assert that they're the same.  Otherwise, maybe 
cp_convert (type, nullptr_node)?

Jason

Re: C++ PATCH for c++/89212 - ICE converting nullptr to pointer-to-member-function

[Marek Polacek]

On Fri, Feb 08, 2019 at 05:37:00PM -0500, Jason Merrill wrote:
> On 2/8/19 12:21 PM, Marek Polacek wrote:
> > r256999 removed early bailout for pointer-to-member-function types, so we
> > now try to tsubst each element of a pointer-to-member-function CONSTRUCTOR.
> > 
> > That's fine but the problem here is that we end up converting a null pointer
> > to pointer-to-member-function type and that crashes in fold_convert:
> > 
> > 16035     case INTEGER_CST:
> > 16039       {
> > 16040         /* Instantiate any typedefs in the type.  */
> > 16041         tree type = tsubst (TREE_TYPE (t), args, complain, in_decl);
> > 16042         r = fold_convert (type, t);
> > 
> > It seems obvious to use cp_fold_convert which handles TYPE_PTRMEM_P, but
> > that then ICEs too, the infamous "canonical types differ for identical types":
> > type is
> > 
> > struct
> > {
> >    void A::<T344> (struct A *) * __pfn;
> >    long int __delta;
> > }
> > 
> > and the type of "0" is "void A::<T344> (struct A *) *".  These types are
> > structurally equivalent but have different canonical types.  (What's up
> > with that, anyway?  It seems OK that the canonical type of the struct is
> > the struct itself and that the canonical type of the pointer is the pointer
> > itself.)
> > 
> > That could be handled in cp_fold_convert: add code to convert an integer_zerop to
> > TYPE_PTRMEMFUNC_P.  Unfortunately the 0 is not null_ptr_cst_p because it's got
> > a pointer type.
> > 
> > Or just don't bother substituting null_member_pointer_value_p and avoid the
> > above.
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk and 8?
> > 
> > 2019-02-08  Marek Polacek  <polacek@redhat.com>
> > 
> > 	PR c++/89212 - ICE converting nullptr to pointer-to-member-function.
> > 	* pt.c (tsubst_copy_and_build) <case CONSTRUCTOR>: Return early for
> > 	null member pointer value.
> > 
> > 	* g++.dg/cpp0x/nullptr40.C: New test.
> > 
> > diff --git gcc/cp/pt.c gcc/cp/pt.c
> > index b8fbf4046f0..acc2d8f1feb 100644
> > --- gcc/cp/pt.c
> > +++ gcc/cp/pt.c
> > @@ -19251,6 +19251,9 @@ tsubst_copy_and_build (tree t,
> >   	   looked up by digest_init.  */
> >   	process_index_p = !(type && MAYBE_CLASS_TYPE_P (type));
> > +	if (null_member_pointer_value_p (t))
> > +	  RETURN (t);
> 
> I would expect this to do the wrong thing if type is different from
> TREE_TYPE (t).  Can we get here for a dependent PMF type like T (A::*)()?
> If not, let's assert that they're the same.  Otherwise, maybe cp_convert
> (type, nullptr_node)?

Yup, that's a concern.  But I'm not seeing any ICEs with the assert added and a
dependent PMF as in the new testcase.  And it seems we get a conversion error
if the types of the PMFs don't match.  If I'm wrong, this would be easy to
fix anyway.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2019-02-11  Marek Polacek  <polacek@redhat.com>

	PR c++/89212 - ICE converting nullptr to pointer-to-member-function.
	* pt.c (tsubst_copy_and_build) <case CONSTRUCTOR>: Return early for
	null member pointer value.

	* g++.dg/cpp0x/nullptr40.C: New test.
	* g++.dg/cpp0x/nullptr41.C: New test.

diff --git gcc/cp/pt.c gcc/cp/pt.c
index b8fbf4046f0..2682c68dcfa 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -19251,6 +19251,12 @@ tsubst_copy_and_build (tree t,
 	   looked up by digest_init.  */
 	process_index_p = !(type && MAYBE_CLASS_TYPE_P (type));
 
+	if (null_member_pointer_value_p (t))
+	  {
+	    gcc_assert (same_type_p (type, TREE_TYPE (t)));
+	    RETURN (t);
+	  }
+
 	n = vec_safe_copy (CONSTRUCTOR_ELTS (t));
         newlen = vec_safe_length (n);
 	FOR_EACH_VEC_SAFE_ELT (n, idx, ce)
diff --git gcc/testsuite/g++.dg/cpp0x/nullptr40.C gcc/testsuite/g++.dg/cpp0x/nullptr40.C
new file mode 100644
index 00000000000..21c188bdb5e
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/nullptr40.C
@@ -0,0 +1,19 @@
+// PR c++/89212
+// { dg-do compile { target c++11 } }
+
+template <int, typename T> using enable_if_t = int;
+
+template<class X, void(X::*foo)() = nullptr>
+struct p
+{
+    template<void(X::*fun)() = foo, typename T = enable_if_t<nullptr == fun, int>>
+    p(T) { }
+    p() = default;
+};
+
+struct A
+{
+    p<A> i = 1;
+    void bar();
+    p<A, &A::bar> j;
+};
diff --git gcc/testsuite/g++.dg/cpp0x/nullptr41.C gcc/testsuite/g++.dg/cpp0x/nullptr41.C
new file mode 100644
index 00000000000..54e66af2095
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/nullptr41.C
@@ -0,0 +1,19 @@
+// PR c++/89212
+// { dg-do compile { target c++11 } }
+
+template <int, typename T> using enable_if_t = int;
+
+template<typename U, typename W, typename Y, class X, W(X::*foo)() = nullptr>
+struct p
+{
+    template<U(Y::*fun)() = foo, typename T = enable_if_t<nullptr == fun, int>>
+    p(T) { }
+    p() = default;
+};
+
+struct A
+{
+    p<void, void, A, A> i = 1;
+    void bar();
+    p<void, void, A, A, &A::bar> j;
+};

Re: C++ PATCH for c++/89212 - ICE converting nullptr to pointer-to-member-function

[Jason Merrill]

On 2/11/19 2:21 PM, Marek Polacek wrote:
> On Fri, Feb 08, 2019 at 05:37:00PM -0500, Jason Merrill wrote:
>> On 2/8/19 12:21 PM, Marek Polacek wrote:
>>> r256999 removed early bailout for pointer-to-member-function types, so we
>>> now try to tsubst each element of a pointer-to-member-function CONSTRUCTOR.
>>>
>>> That's fine but the problem here is that we end up converting a null pointer
>>> to pointer-to-member-function type and that crashes in fold_convert:
>>>
>>> 16035     case INTEGER_CST:
>>> 16039       {
>>> 16040         /* Instantiate any typedefs in the type.  */
>>> 16041         tree type = tsubst (TREE_TYPE (t), args, complain, in_decl);
>>> 16042         r = fold_convert (type, t);
>>>
>>> It seems obvious to use cp_fold_convert which handles TYPE_PTRMEM_P, but
>>> that then ICEs too, the infamous "canonical types differ for identical types":
>>> type is
>>>
>>> struct
>>> {
>>>     void A::<T344> (struct A *) * __pfn;
>>>     long int __delta;
>>> }
>>>
>>> and the type of "0" is "void A::<T344> (struct A *) *".  These types are
>>> structurally equivalent but have different canonical types.  (What's up
>>> with that, anyway?  It seems OK that the canonical type of the struct is
>>> the struct itself and that the canonical type of the pointer is the pointer
>>> itself.)
>>>
>>> That could be handled in cp_fold_convert: add code to convert an integer_zerop to
>>> TYPE_PTRMEMFUNC_P.  Unfortunately the 0 is not null_ptr_cst_p because it's got
>>> a pointer type.
>>>
>>> Or just don't bother substituting null_member_pointer_value_p and avoid the
>>> above.
>>>
>>> Bootstrapped/regtested on x86_64-linux, ok for trunk and 8?
>>>
>>> 2019-02-08  Marek Polacek  <polacek@redhat.com>
>>>
>>> 	PR c++/89212 - ICE converting nullptr to pointer-to-member-function.
>>> 	* pt.c (tsubst_copy_and_build) <case CONSTRUCTOR>: Return early for
>>> 	null member pointer value.
>>>
>>> 	* g++.dg/cpp0x/nullptr40.C: New test.
>>>
>>> diff --git gcc/cp/pt.c gcc/cp/pt.c
>>> index b8fbf4046f0..acc2d8f1feb 100644
>>> --- gcc/cp/pt.c
>>> +++ gcc/cp/pt.c
>>> @@ -19251,6 +19251,9 @@ tsubst_copy_and_build (tree t,
>>>    	   looked up by digest_init.  */
>>>    	process_index_p = !(type && MAYBE_CLASS_TYPE_P (type));
>>> +	if (null_member_pointer_value_p (t))
>>> +	  RETURN (t);
>>
>> I would expect this to do the wrong thing if type is different from
>> TREE_TYPE (t).  Can we get here for a dependent PMF type like T (A::*)()?
>> If not, let's assert that they're the same.  Otherwise, maybe cp_convert
>> (type, nullptr_node)?
> 
> Yup, that's a concern.  But I'm not seeing any ICEs with the assert added and a
> dependent PMF as in the new testcase.  And it seems we get a conversion error
> if the types of the PMFs don't match.  If I'm wrong, this would be easy to
> fix anyway.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

OK.

Jason