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]
Related show

Commit Message

Jason Merrill via Gcc-patches July 7, 2020, 1:33 p.m.
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

-- 
2.27.0.203.gf402ea6816

Comments

Jason Merrill via Gcc-patches July 7, 2020, 7:07 p.m. | #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 via Gcc-patches July 7, 2020, 7:21 p.m. | #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 };

> +    };

>
Jason Merrill via Gcc-patches July 7, 2020, 7:36 p.m. | #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 via Gcc-patches July 8, 2020, 4:33 a.m. | #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 };

>>> +    };

>>>

>>

>>

>

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 };
+    };