diff mbox series

c++: ICE in is_really_empty_class [PR95497]

Message ID 20200707133327.1377955-1-ppalka@redhat.com
State New
Headers show
Series c++: ICE in is_really_empty_class [PR95497] | expand

Commit Message

Patrick Palka July 7, 2020, 1:33 p.m. UTC
We are ICEing in the testcase below because we pass the
yet-uninstantiated class type A<int> of the PARM_DECL b to
is_really_empty_class from potential_rvalue_constant_expression when
parsing the requirement t += b.

This patch fixes the ICE by guarding the problematic call to
is_really_empty_class with a COMPLETE_TYPE_P check, which should also
subsume the existing dependent_type_p check.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
commit to trunk and to the 10 branch?

gcc/cp/ChangeLog:

	PR c++/95497
	* constexpr.c (potential_constant_expression_1): When
	processing_template_decl, check COMPLETE_TYPE_P before calling
	is_really_empty_class.

gcc/testsuite/ChangeLog:

	PR c++/95497
	* g++.dg/cpp2a/concepts-pr95497.C: New test.
---
 gcc/cp/constexpr.c                            |  2 +-
 gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C

Comments

Patrick Palka July 7, 2020, 7:07 p.m. UTC | #1
On Tue, 7 Jul 2020, Patrick Palka wrote:

> We are ICEing in the testcase below because we pass the
> yet-uninstantiated class type A<int> of the PARM_DECL b to
> is_really_empty_class from potential_rvalue_constant_expression when
> parsing the requirement t += b.
> 
> This patch fixes the ICE by guarding the problematic call to
> is_really_empty_class with a COMPLETE_TYPE_P check, which should also
> subsume the existing dependent_type_p check.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
> commit to trunk and to the 10 branch?

Oops, the regression is not present on the 10 branch so this fix wouldn't
need backporting.

> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/95497
> 	* constexpr.c (potential_constant_expression_1): When
> 	processing_template_decl, check COMPLETE_TYPE_P before calling
> 	is_really_empty_class.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/95497
> 	* g++.dg/cpp2a/concepts-pr95497.C: New test.
> ---
>  gcc/cp/constexpr.c                            |  2 +-
>  gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C | 12 ++++++++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 1939166e907..ff78ebda2dc 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -7443,7 +7443,7 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
>        if (now && want_rval)
>  	{
>  	  tree type = TREE_TYPE (t);
> -	  if (dependent_type_p (type)
> +	  if ((processing_template_decl && !COMPLETE_TYPE_P (type))
>  	      || is_really_empty_class (type, /*ignore_vptr*/false))
>  	    /* An empty class has no data to read.  */
>  	    return true;
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
> new file mode 100644
> index 00000000000..4d7718ad5e8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
> @@ -0,0 +1,12 @@
> +// PR c++/95497
> +// { dg-do compile { target c++20 } }
> +
> +template <typename T>
> +struct A{};
> +
> +template <typename T>
> +concept c =
> +    requires(T t, A<int> b) // note that A<int> is independent of T
> +    {
> +        { t += b };
> +    };
> -- 
> 2.27.0.203.gf402ea6816
> 
>
Jason Merrill July 7, 2020, 7:21 p.m. UTC | #2
On 7/7/20 9:33 AM, Patrick Palka wrote:
> We are ICEing in the testcase below because we pass the
> yet-uninstantiated class type A<int> of the PARM_DECL b to
> is_really_empty_class from potential_rvalue_constant_expression when
> parsing the requirement t += b.

Why are we getting to potential_rvalue_constant_expression?  My guess is 
from build_non_dependent_expr because processing_constraint isn't set?

> This patch fixes the ICE by guarding the problematic call to
> is_really_empty_class with a COMPLETE_TYPE_P check, which should also
> subsume the existing dependent_type_p check.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
> commit to trunk and to the 10 branch?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/95497
> 	* constexpr.c (potential_constant_expression_1): When
> 	processing_template_decl, check COMPLETE_TYPE_P before calling
> 	is_really_empty_class.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/95497
> 	* g++.dg/cpp2a/concepts-pr95497.C: New test.
> ---
>   gcc/cp/constexpr.c                            |  2 +-
>   gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C | 12 ++++++++++++
>   2 files changed, 13 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 1939166e907..ff78ebda2dc 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -7443,7 +7443,7 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
>         if (now && want_rval)
>   	{
>   	  tree type = TREE_TYPE (t);
> -	  if (dependent_type_p (type)
> +	  if ((processing_template_decl && !COMPLETE_TYPE_P (type))
>   	      || is_really_empty_class (type, /*ignore_vptr*/false))
>   	    /* An empty class has no data to read.  */
>   	    return true;
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
> new file mode 100644
> index 00000000000..4d7718ad5e8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
> @@ -0,0 +1,12 @@
> +// PR c++/95497
> +// { dg-do compile { target c++20 } }
> +
> +template <typename T>
> +struct A{};
> +
> +template <typename T>
> +concept c =
> +    requires(T t, A<int> b) // note that A<int> is independent of T
> +    {
> +        { t += b };
> +    };
>
Patrick Palka July 7, 2020, 7:36 p.m. UTC | #3
On Tue, 7 Jul 2020, Jason Merrill wrote:

> On 7/7/20 9:33 AM, Patrick Palka wrote:
> > We are ICEing in the testcase below because we pass the
> > yet-uninstantiated class type A<int> of the PARM_DECL b to
> > is_really_empty_class from potential_rvalue_constant_expression when
> > parsing the requirement t += b.
> 
> Why are we getting to potential_rvalue_constant_expression?  My guess is from
> build_non_dependent_expr because processing_constraint isn't set?

My mistake, I meant to write that we are calling is_really_empty_class
from is_rvalue_constant_expression, not from
potential_rvalue_constant_expression.  And we're getting to
is_rvalue_constant_expression from cp_parser_constant_expression when
parsing the RHS of the assignment.

> 
> > This patch fixes the ICE by guarding the problematic call to
> > is_really_empty_class with a COMPLETE_TYPE_P check, which should also
> > subsume the existing dependent_type_p check.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
> > commit to trunk and to the 10 branch?
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	PR c++/95497
> > 	* constexpr.c (potential_constant_expression_1): When
> > 	processing_template_decl, check COMPLETE_TYPE_P before calling
> > 	is_really_empty_class.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	PR c++/95497
> > 	* g++.dg/cpp2a/concepts-pr95497.C: New test.
> > ---
> >   gcc/cp/constexpr.c                            |  2 +-
> >   gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C | 12 ++++++++++++
> >   2 files changed, 13 insertions(+), 1 deletion(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
> > 
> > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> > index 1939166e907..ff78ebda2dc 100644
> > --- a/gcc/cp/constexpr.c
> > +++ b/gcc/cp/constexpr.c
> > @@ -7443,7 +7443,7 @@ potential_constant_expression_1 (tree t, bool
> > want_rval, bool strict, bool now,
> >         if (now && want_rval)
> >   	{
> >   	  tree type = TREE_TYPE (t);
> > -	  if (dependent_type_p (type)
> > +	  if ((processing_template_decl && !COMPLETE_TYPE_P (type))
> >   	      || is_really_empty_class (type, /*ignore_vptr*/false))
> >   	    /* An empty class has no data to read.  */
> >   	    return true;
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
> > new file mode 100644
> > index 00000000000..4d7718ad5e8
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
> > @@ -0,0 +1,12 @@
> > +// PR c++/95497
> > +// { dg-do compile { target c++20 } }
> > +
> > +template <typename T>
> > +struct A{};
> > +
> > +template <typename T>
> > +concept c =
> > +    requires(T t, A<int> b) // note that A<int> is independent of T
> > +    {
> > +        { t += b };
> > +    };
> > 
> 
>
Jason Merrill July 8, 2020, 4:33 a.m. UTC | #4
On 7/7/20 3:36 PM, Patrick Palka wrote:
> On Tue, 7 Jul 2020, Jason Merrill wrote:
> 
>> On 7/7/20 9:33 AM, Patrick Palka wrote:
>>> We are ICEing in the testcase below because we pass the
>>> yet-uninstantiated class type A<int> of the PARM_DECL b to
>>> is_really_empty_class from potential_rvalue_constant_expression when
>>> parsing the requirement t += b.
>>
>> Why are we getting to potential_rvalue_constant_expression?  My guess is from
>> build_non_dependent_expr because processing_constraint isn't set?
> 
> My mistake, I meant to write that we are calling is_really_empty_class
> from is_rvalue_constant_expression, not from
> potential_rvalue_constant_expression.  And we're getting to
> is_rvalue_constant_expression from cp_parser_constant_expression when
> parsing the RHS of the assignment.

Most of the parser constant-expression stuff is a relic of C++98; I 
tried changing cp_parser_constant_expression to only check constancy 
when we're requiring a constant expression, but that broke some library 
tests for some reason.

>>> This patch fixes the ICE by guarding the problematic call to
>>> is_really_empty_class with a COMPLETE_TYPE_P check, which should also
>>> subsume the existing dependent_type_p check.

OK.

>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
>>> commit to trunk and to the 10 branch?
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	PR c++/95497
>>> 	* constexpr.c (potential_constant_expression_1): When
>>> 	processing_template_decl, check COMPLETE_TYPE_P before calling
>>> 	is_really_empty_class.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	PR c++/95497
>>> 	* g++.dg/cpp2a/concepts-pr95497.C: New test.
>>> ---
>>>    gcc/cp/constexpr.c                            |  2 +-
>>>    gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C | 12 ++++++++++++
>>>    2 files changed, 13 insertions(+), 1 deletion(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
>>>
>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
>>> index 1939166e907..ff78ebda2dc 100644
>>> --- a/gcc/cp/constexpr.c
>>> +++ b/gcc/cp/constexpr.c
>>> @@ -7443,7 +7443,7 @@ potential_constant_expression_1 (tree t, bool
>>> want_rval, bool strict, bool now,
>>>          if (now && want_rval)
>>>    	{
>>>    	  tree type = TREE_TYPE (t);
>>> -	  if (dependent_type_p (type)
>>> +	  if ((processing_template_decl && !COMPLETE_TYPE_P (type))
>>>    	      || is_really_empty_class (type, /*ignore_vptr*/false))
>>>    	    /* An empty class has no data to read.  */
>>>    	    return true;
>>> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
>>> b/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
>>> new file mode 100644
>>> index 00000000000..4d7718ad5e8
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
>>> @@ -0,0 +1,12 @@
>>> +// PR c++/95497
>>> +// { dg-do compile { target c++20 } }
>>> +
>>> +template <typename T>
>>> +struct A{};
>>> +
>>> +template <typename T>
>>> +concept c =
>>> +    requires(T t, A<int> b) // note that A<int> is independent of T
>>> +    {
>>> +        { t += b };
>>> +    };
>>>
>>
>>
>
diff mbox series

Patch

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 1939166e907..ff78ebda2dc 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -7443,7 +7443,7 @@  potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
       if (now && want_rval)
 	{
 	  tree type = TREE_TYPE (t);
-	  if (dependent_type_p (type)
+	  if ((processing_template_decl && !COMPLETE_TYPE_P (type))
 	      || is_really_empty_class (type, /*ignore_vptr*/false))
 	    /* An empty class has no data to read.  */
 	    return true;
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
new file mode 100644
index 00000000000..4d7718ad5e8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr95497.C
@@ -0,0 +1,12 @@ 
+// PR c++/95497
+// { dg-do compile { target c++20 } }
+
+template <typename T>
+struct A{};
+
+template <typename T>
+concept c =
+    requires(T t, A<int> b) // note that A<int> is independent of T
+    {
+        { t += b };
+    };