diff mbox

PING: [PATCH] Fix PRs c/52283/37985

Message ID 4F8E7B61.7030701@st.com
State New
Headers show

Commit Message

Christian Bruel April 18, 2012, 8:29 a.m. UTC 
http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00191.html

and discussed in
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52283

I would like to close the associated PRs to fix a few discrepancies with
the folding of constant expressions warnings.

Original patch from Manu was slightly modified to reflect the new
warn_if_unused_value location (moved from stmt.c to c-familly/c-common.c).

Is it OK for trunk, bootstrapped and regtested on x86

Many Thanks

Christian

Comments

Manuel López-Ibáñez April 18, 2012, 9:06 a.m. UTC | #1 
On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com> wrote:
>
> Is it OK for trunk, bootstrapped and regtested on x86

I think Joseph Myers is on vacation, and there are no other C FE
reviewers, but since this is c-common and convert.c, perhaps Jason
and/or Richard can review it?

Thanks,

Manuel.
Richard Biener April 18, 2012, 9:51 a.m. UTC | #2 
On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com> wrote:
>>
>> Is it OK for trunk, bootstrapped and regtested on x86
>
> I think Joseph Myers is on vacation, and there are no other C FE
> reviewers, but since this is c-common and convert.c, perhaps Jason
> and/or Richard can review it?

The patch is ok if you put the PR52283 properly into a separate testcase,
not by amending gcc.dg/case-const-2.c.

Thanks,
Richard.

> Thanks,
>
> Manuel.
Christian Bruel April 19, 2012, 9:11 a.m. UTC | #3 
On 04/18/2012 11:51 AM, Richard Guenther wrote:
> On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez
> <lopezibanez@gmail.com> wrote:
>> On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com> wrote:
>>>
>>> Is it OK for trunk, bootstrapped and regtested on x86
>>
>> I think Joseph Myers is on vacation, and there are no other C FE
>> reviewers, but since this is c-common and convert.c, perhaps Jason
>> and/or Richard can review it?
> 
> The patch is ok if you put the PR52283 properly into a separate testcase,
> not by amending gcc.dg/case-const-2.c.
> 

Thanks, done at rev #186586. with this change.

> Thanks,
> Richard.
> 
>> Thanks,
>>
>> Manuel.
Manuel López-Ibáñez April 19, 2012, 10:17 a.m. UTC | #4 
On 19 April 2012 11:11, Christian Bruel <christian.bruel@st.com> wrote:
>
>
> On 04/18/2012 11:51 AM, Richard Guenther wrote:
>> On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez
>> <lopezibanez@gmail.com> wrote:
>>> On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com> wrote:
>>>>
>>>> Is it OK for trunk, bootstrapped and regtested on x86
>>>
>>> I think Joseph Myers is on vacation, and there are no other C FE
>>> reviewers, but since this is c-common and convert.c, perhaps Jason
>>> and/or Richard can review it?
>>
>> The patch is ok if you put the PR52283 properly into a separate testcase,
>> not by amending gcc.dg/case-const-2.c.
>>
>
> Thanks, done at rev #186586. with this change.

Great!

Just a minor nit, for future patches. There is the unwritten rule of
adding the Changelogs to the commit log, like follows:

2012-04-19  Christian Bruel  <christian.bruel@st.com>
                  Manuel López-Ibáñez  <manu@gcc.gnu.org>

       PR c/52283
       PR c/37985
       * stmt.c (warn_if_unused_value): Skip NOP_EXPR.
       * convert.c (convert_to_integer): Don't set TREE_NO_WARNING.
testsuite/
       * gcc.dg/pr52283.c: New test.
       * gcc.dg/pr37985.c: New test.


Also, if you add to the commit logs the PR numbers like:
PR whatever/NUMBER
PR whatever2/NUMBER

then bugzilla will show the commit on each PR.

Cheers,

Manuel.
H.J. Lu April 19, 2012, 2:31 p.m. UTC | #5 
On Thu, Apr 19, 2012 at 3:17 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 19 April 2012 11:11, Christian Bruel <christian.bruel@st.com> wrote:
>>
>>
>> On 04/18/2012 11:51 AM, Richard Guenther wrote:
>>> On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez
>>> <lopezibanez@gmail.com> wrote:
>>>> On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com> wrote:
>>>>>
>>>>> Is it OK for trunk, bootstrapped and regtested on x86
>>>>
>>>> I think Joseph Myers is on vacation, and there are no other C FE
>>>> reviewers, but since this is c-common and convert.c, perhaps Jason
>>>> and/or Richard can review it?
>>>
>>> The patch is ok if you put the PR52283 properly into a separate testcase,
>>> not by amending gcc.dg/case-const-2.c.
>>>
>>
>> Thanks, done at rev #186586. with this change.
>
> Great!
>
> Just a minor nit, for future patches. There is the unwritten rule of
> adding the Changelogs to the commit log, like follows:
>
> 2012-04-19  Christian Bruel  <christian.bruel@st.com>
>                  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>       PR c/52283
>       PR c/37985
>       * stmt.c (warn_if_unused_value): Skip NOP_EXPR.
>       * convert.c (convert_to_integer): Don't set TREE_NO_WARNING.
> testsuite/
>       * gcc.dg/pr52283.c: New test.
>       * gcc.dg/pr37985.c: New test.
>
>

gcc.dg/pr52283.c failed on Linux/x86:

FAIL: gcc.dg/pr52283.c (test for excess errors)
Richard Biener April 23, 2012, 9:28 a.m. UTC | #6 
On Fri, Apr 20, 2012 at 5:23 PM, Greta Yorsh <Greta.Yorsh@arm.com> wrote:
> Here is a patch to fix the failing test gcc.dg/pr52283.c.
> Adding the missing dg-warning and dg-options.
>
> OK?

Ok.

Thanks,
Richard.

>
> gcc/testsuite/ChangeLog
>
> 2012-04-20  Greta Yorsh  <Greta.Yorsh@arm.com>
>
>        * gcc.dg/pr52283.c: Add missing dg-warning and dg-options.
>
>
> diff --git a/gcc/testsuite/gcc.dg/pr52283.c b/gcc/testsuite/gcc.dg/pr52283.c
> index 33785a5..070e71a 100644
> --- a/gcc/testsuite/gcc.dg/pr52283.c
> +++ b/gcc/testsuite/gcc.dg/pr52283.c
> @@ -1,6 +1,7 @@
>  /* Test for case labels not integer constant expressions but folding
>    to integer constants (used in Linux kernel).  */
>  /* { dg-do compile } */
> +/* { dg-options "-pedantic" } */
>
>  extern unsigned int u;
>
> @@ -9,7 +10,7 @@ b (int c)
>  {
>   switch (c)
>     {
> -    case (int) (2  | ((4 < 8) ? 8 : u)):
> +    case (int) (2  | ((4 < 8) ? 8 : u)): /* { dg-warning "case label is not
> an integer constant expression" } */
>       ;
>     }
>  }
>
>> -----Original Message-----
>> From: H.J. Lu [mailto:hjl.tools@gmail.com]
>> Sent: 19 April 2012 15:32
>> To: Manuel López-Ibáñez
>> Cc: Christian Bruel; Richard Guenther; gcc-patches@gcc.gnu.org; Joseph
>> S. Myers; Jason Merrill
>> Subject: Re: PING: [PATCH] Fix PRs c/52283/37985
>>
>> On Thu, Apr 19, 2012 at 3:17 AM, Manuel López-Ibáñez
>> <lopezibanez@gmail.com> wrote:
>> > On 19 April 2012 11:11, Christian Bruel <christian.bruel@st.com>
>> wrote:
>> >>
>> >>
>> >> On 04/18/2012 11:51 AM, Richard Guenther wrote:
>> >>> On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez
>> >>> <lopezibanez@gmail.com> wrote:
>> >>>> On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com>
>> wrote:
>> >>>>>
>> >>>>> Is it OK for trunk, bootstrapped and regtested on x86
>> >>>>
>> >>>> I think Joseph Myers is on vacation, and there are no other C FE
>> >>>> reviewers, but since this is c-common and convert.c, perhaps Jason
>> >>>> and/or Richard can review it?
>> >>>
>> >>> The patch is ok if you put the PR52283 properly into a separate
>> testcase,
>> >>> not by amending gcc.dg/case-const-2.c.
>> >>>
>> >>
>> >> Thanks, done at rev #186586. with this change.
>> >
>> > Great!
>> >
>> > Just a minor nit, for future patches. There is the unwritten rule of
>> > adding the Changelogs to the commit log, like follows:
>> >
>> > 2012-04-19  Christian Bruel  <christian.bruel@st.com>
>> >                  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>> >
>> >       PR c/52283
>> >       PR c/37985
>> >       * stmt.c (warn_if_unused_value): Skip NOP_EXPR.
>> >       * convert.c (convert_to_integer): Don't set TREE_NO_WARNING.
>> > testsuite/
>> >       * gcc.dg/pr52283.c: New test.
>> >       * gcc.dg/pr37985.c: New test.
>> >
>> >
>>
>> gcc.dg/pr52283.c failed on Linux/x86:
>>
>> FAIL: gcc.dg/pr52283.c (test for excess errors)
>>
>> --
>> H.J.
>
>
>
diff mbox

Patch

gcc/testsuite/ChangeLog

2010-02-15  Christian Bruel  <christian.bruel@st.com>

	* gcc.dg/case-const-1.c: Test constant expression.
	* gcc.dg/case-const-2.c: Likewise.
	* gcc.dg/case-const-3.c: Likewise.

gcc/ChangeLog
2012-03-29   Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR c/52283
	* c-familly/c-common.c (warn_if_unused_value): Skip NOP_EXPR.
	* convert.c (convert_to_integer): Don't set TREE_NO_WARNING.

Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 186524)
+++ gcc/c-family/c-common.c	(working copy)
@@ -1692,6 +1692,7 @@ 
 
     case SAVE_EXPR:
     case NON_LVALUE_EXPR:
+    case NOP_EXPR:
       exp = TREE_OPERAND (exp, 0);
       goto restart;
 
Index: gcc/convert.c
===================================================================
--- gcc/convert.c	(revision 186524)
+++ gcc/convert.c	(working copy)
@@ -537,7 +537,6 @@ 
       else if (outprec >= inprec)
 	{
 	  enum tree_code code;
-	  tree tem;
 
 	  /* If the precision of the EXPR's type is K bits and the
 	     destination mode has more bits, and the sign is changing,
@@ -555,13 +554,7 @@ 
 	  else
 	    code = NOP_EXPR;
 
-	  tem = fold_unary (code, type, expr);
-	  if (tem)
-	    return tem;
-
-	  tem = build1 (code, type, expr);
-	  TREE_NO_WARNING (tem) = 1;
-	  return tem;
+	  return fold_build1 (code, type, expr);
 	}
 
       /* If TYPE is an enumeral type or a type with a precision less

Index: gcc/testsuite/gcc.dg/pr37985.c
===================================================================
--- gcc/testsuite/gcc.dg/pr37985.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr37985.c	(revision 0)
@@ -0,0 +1,8 @@ 
+/* PR c37985 */
+/* { dg-do compile } */
+/* { dg-options " -Wall -Wextra " } */
+unsigned char foo(unsigned char a)
+{
+  a >> 2; /* { dg-warning "no effect" } */
+  return a;
+}
Index: gcc/testsuite/gcc.dg/case-const-1.c
===================================================================
--- gcc/testsuite/gcc.dg/case-const-1.c	(revision 186524)
+++ gcc/testsuite/gcc.dg/case-const-1.c	(working copy)
@@ -1,9 +1,11 @@ 
 /* Test for case labels not integer constant expressions but folding
-   to integer constants (used in Linux kernel, PR 39613).  */
+   to integer constants (used in Linux kernel, PR 39613, 52283).  */
 /* { dg-do compile } */
 /* { dg-options "" } */
 
 extern int i;
+extern unsigned int u;
+
 void
 f (int c)
 {
@@ -13,3 +15,13 @@ 
       ;
     }
 }
+
+void
+b (int c)
+{
+  switch (c)
+    {
+    case (int) (2  | ((4 < 8) ? 8 : u)):
+      ;
+    }
+}
Index: gcc/testsuite/gcc.dg/case-const-2.c
===================================================================
--- gcc/testsuite/gcc.dg/case-const-2.c	(revision 186524)
+++ gcc/testsuite/gcc.dg/case-const-2.c	(working copy)
@@ -1,9 +1,11 @@ 
 /* Test for case labels not integer constant expressions but folding
-   to integer constants (used in Linux kernel, PR 39613).  */
+   to integer constants (used in Linux kernel, PR 39613, 52283).  */
 /* { dg-do compile } */
 /* { dg-options "-pedantic" } */
 
 extern int i;
+extern unsigned int u;
+
 void
 f (int c)
 {
@@ -13,3 +15,14 @@ 
       ;
     }
 }
+
+void
+b (int c)
+{
+  switch (c)
+    {
+    case (int) (2  | ((4 < 8) ? 8 : u)): /* { dg-warning "case label is not an integer constant expression" } */
+      ;
+    }
+}
+
Index: gcc/testsuite/gcc.dg/case-const-3.c
===================================================================
--- gcc/testsuite/gcc.dg/case-const-3.c	(revision 186524)
+++ gcc/testsuite/gcc.dg/case-const-3.c	(working copy)
@@ -1,9 +1,11 @@ 
 /* Test for case labels not integer constant expressions but folding
-   to integer constants (used in Linux kernel, PR 39613).  */
+   to integer constants (used in Linux kernel, PR 39613, 52283, ).  */
 /* { dg-do compile } */
 /* { dg-options "-pedantic-errors" } */
 
 extern int i;
+extern unsigned int u;
+
 void
 f (int c)
 {
@@ -13,3 +15,16 @@ 
       ;
     }
 }
+
+void
+b (int c)
+{
+  switch (c)
+    {
+    case (int) (2  | ((4 < 8) ? 8 : u)): /* { dg-error "case label is not an integer constant expression" } */
+      ;
+    }
+}
+
+
+