c++: Handle COMPOUND_EXPRs in ocp_convert [PR94339]

Message ID 20200326191325.GM2156@tucnak
State New
Headers show
Series
  • c++: Handle COMPOUND_EXPRs in ocp_convert [PR94339]
Related show

Commit Message

Christophe Lyon via Gcc-patches March 26, 2020, 7:13 p.m.
Hi!

My recent change to get_narrower/warnings_for_convert_and_check broke
the following testcase, warnings_for_convert_and_check is upset that
expr is a COMPOUND_EXPR with INTEGER_CST at the rightmost operand, while
result is a COMPOUND_EXPR with a NOP_EXPR of INTEGER_CST at the rightmost
operand, it expects such conversions to be simplified.

The easiest fix seems to be to handle COMPOUND_EXPRs in ocp_convert too,
by converting the rightmost operand and recreating COMPOUND_EXPR(s) if that
changed.

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

The attr-copy-2.C change is a workaround for PR94346, where we now ICE on
the testcase, while previously we'd ICE only if it contained a comma
expression at the outer level rather than cast of a COMPOUND_EXPR to
something.  I'll defer that to Martin.

2020-03-26  Jakub Jelinek  <jakub@redhat.com>

	PR c++/94339
	* cvt.c (ocp_convert): Handle COMPOUND_EXPR by recursion on the second
	operand and creating a new COMPOUND_EXPR if anything changed.

	* g++.dg/other/pr94339.C: New test.
	* g++.dg/ext/attr-copy-2.C: Comment out failing tests due to PR94346.


	Jakub

Comments

Christophe Lyon via Gcc-patches March 27, 2020, 2:12 a.m. | #1
On 3/26/20 3:13 PM, Jakub Jelinek wrote:
> Hi!

> 

> My recent change to get_narrower/warnings_for_convert_and_check broke

> the following testcase, warnings_for_convert_and_check is upset that

> expr is a COMPOUND_EXPR with INTEGER_CST at the rightmost operand, while

> result is a COMPOUND_EXPR with a NOP_EXPR of INTEGER_CST at the rightmost

> operand, it expects such conversions to be simplified.

> 

> The easiest fix seems to be to handle COMPOUND_EXPRs in ocp_convert too,

> by converting the rightmost operand and recreating COMPOUND_EXPR(s) if that

> changed.

> 

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


OK.

> The attr-copy-2.C change is a workaround for PR94346, where we now ICE on

> the testcase, while previously we'd ICE only if it contained a comma

> expression at the outer level rather than cast of a COMPOUND_EXPR to

> something.  I'll defer that to Martin.

> 

> 2020-03-26  Jakub Jelinek  <jakub@redhat.com>

> 

> 	PR c++/94339

> 	* cvt.c (ocp_convert): Handle COMPOUND_EXPR by recursion on the second

> 	operand and creating a new COMPOUND_EXPR if anything changed.

> 

> 	* g++.dg/other/pr94339.C: New test.

> 	* g++.dg/ext/attr-copy-2.C: Comment out failing tests due to PR94346.

> 

> --- gcc/cp/cvt.c.jj	2020-01-12 11:54:36.000000000 +0100

> +++ gcc/cp/cvt.c	2020-03-26 12:30:52.312170071 +0100

> @@ -697,6 +697,17 @@ ocp_convert (tree type, tree expr, int c

>     if (error_operand_p (e) || type == error_mark_node)

>       return error_mark_node;

>   

> +  if (TREE_CODE (e) == COMPOUND_EXPR)

> +    {

> +      e = ocp_convert (type, TREE_OPERAND (e, 1), convtype, flags, complain);

> +      if (e == error_mark_node)

> +	return error_mark_node;

> +      if (e == TREE_OPERAND (expr, 1))

> +	return expr;

> +      return build2_loc (EXPR_LOCATION (expr), COMPOUND_EXPR, TREE_TYPE (e),

> +			 TREE_OPERAND (expr, 0), e);

> +    }

> +

>     complete_type (type);

>     complete_type (TREE_TYPE (expr));

>   

> --- gcc/testsuite/g++.dg/other/pr94339.C.jj	2020-03-26 12:37:01.645664573 +0100

> +++ gcc/testsuite/g++.dg/other/pr94339.C	2020-03-26 12:32:36.217621191 +0100

> @@ -0,0 +1,11 @@

> +// PR c++/94339

> +// { dg-do compile }

> +

> +unsigned a;

> +void bar ();

> +

> +unsigned

> +foo (bool x)

> +{

> +  return x ? bar (), -1 : a;

> +}

> --- gcc/testsuite/g++.dg/ext/attr-copy-2.C.jj	2020-01-12 11:54:37.000000000 +0100

> +++ gcc/testsuite/g++.dg/ext/attr-copy-2.C	2020-03-26 20:05:34.464882638 +0100

> @@ -36,8 +36,8 @@ typedef struct C

>     ATTR (copy (*(struct A *)(0, 1))) long m_xpa_0_1;

>   

>     ATTR (copy (((struct A *)0)[0])) short m_arpa_0;

> -  ATTR (copy (((struct A *)(1, 0))[0])) int m_arpa_1_0;

> -  ATTR (copy (((struct A *)(0, 1))[0])) long m_arpa_0_1;

> +//  ATTR (copy (((struct A *)(1, 0))[0])) int m_arpa_1_0;

> +//  ATTR (copy (((struct A *)(0, 1))[0])) long m_arpa_0_1;

>   

>     ATTR (copy (a)) short m_a;

>     ATTR (copy (b.a)) int m_b_a;

> @@ -86,8 +86,8 @@ static_assert (__builtin_has_attribute (

>   static_assert (__builtin_has_attribute (((C*)0)->m_ra, packed));

>   

>   static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0, packed));

> -static_assert (__builtin_has_attribute (((C*)0)->m_arpa_1_0, packed));

> -static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0_1, packed));

> +//static_assert (__builtin_has_attribute (((C*)0)->m_arpa_1_0, packed));

> +//static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0_1, packed));

>   

>   static_assert (__builtin_has_attribute (((C*)0)->m_a, packed));

>   static_assert (__builtin_has_attribute (((C*)0)->m_b_a, packed));

> 

> 	Jakub

>

Patch

--- gcc/cp/cvt.c.jj	2020-01-12 11:54:36.000000000 +0100
+++ gcc/cp/cvt.c	2020-03-26 12:30:52.312170071 +0100
@@ -697,6 +697,17 @@  ocp_convert (tree type, tree expr, int c
   if (error_operand_p (e) || type == error_mark_node)
     return error_mark_node;
 
+  if (TREE_CODE (e) == COMPOUND_EXPR)
+    {
+      e = ocp_convert (type, TREE_OPERAND (e, 1), convtype, flags, complain);
+      if (e == error_mark_node)
+	return error_mark_node;
+      if (e == TREE_OPERAND (expr, 1))
+	return expr;
+      return build2_loc (EXPR_LOCATION (expr), COMPOUND_EXPR, TREE_TYPE (e),
+			 TREE_OPERAND (expr, 0), e);
+    }
+
   complete_type (type);
   complete_type (TREE_TYPE (expr));
 
--- gcc/testsuite/g++.dg/other/pr94339.C.jj	2020-03-26 12:37:01.645664573 +0100
+++ gcc/testsuite/g++.dg/other/pr94339.C	2020-03-26 12:32:36.217621191 +0100
@@ -0,0 +1,11 @@ 
+// PR c++/94339
+// { dg-do compile }
+
+unsigned a;
+void bar ();
+
+unsigned
+foo (bool x)
+{
+  return x ? bar (), -1 : a;
+}
--- gcc/testsuite/g++.dg/ext/attr-copy-2.C.jj	2020-01-12 11:54:37.000000000 +0100
+++ gcc/testsuite/g++.dg/ext/attr-copy-2.C	2020-03-26 20:05:34.464882638 +0100
@@ -36,8 +36,8 @@  typedef struct C
   ATTR (copy (*(struct A *)(0, 1))) long m_xpa_0_1;
 
   ATTR (copy (((struct A *)0)[0])) short m_arpa_0;
-  ATTR (copy (((struct A *)(1, 0))[0])) int m_arpa_1_0;
-  ATTR (copy (((struct A *)(0, 1))[0])) long m_arpa_0_1;
+//  ATTR (copy (((struct A *)(1, 0))[0])) int m_arpa_1_0;
+//  ATTR (copy (((struct A *)(0, 1))[0])) long m_arpa_0_1;
 
   ATTR (copy (a)) short m_a;
   ATTR (copy (b.a)) int m_b_a;
@@ -86,8 +86,8 @@  static_assert (__builtin_has_attribute (
 static_assert (__builtin_has_attribute (((C*)0)->m_ra, packed));
 
 static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0, packed));
-static_assert (__builtin_has_attribute (((C*)0)->m_arpa_1_0, packed));
-static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0_1, packed));
+//static_assert (__builtin_has_attribute (((C*)0)->m_arpa_1_0, packed));
+//static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0_1, packed));
 
 static_assert (__builtin_has_attribute (((C*)0)->m_a, packed));
 static_assert (__builtin_has_attribute (((C*)0)->m_b_a, packed));