PATCH RFA: Fix -Wparentheses fallout

PATCH RFA: Fix -Wparentheses fallout

[Ian Lance Taylor]

The -Wparentheses patch I recently checked in accidentally warns about
    a = b = c;
where all variables have type bool.  That is not intended.  This patch
fixes the problem, and adds test cases.

Thanks to Ben Elliston for noticing the problem.

Bootstrapped and tested on i686-pc-linux-gnu.

OK for mainline?

Ian


cp/ChangeLog:
2007-01-07  Ian Lance Taylor  <iant@google.com>

	* typeck.c (convert_for_assignment): Only warn about a = b = c
	when converting to bool.

testsuite/ChangeLog:
2007-01-07  Ian Lance Taylor  <iant@google.com>

	* g++.dg/warn/Wparentheses-24.C: New test.


Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 120526)
+++ gcc/cp/typeck.c	(working copy)
@@ -6378,11 +6378,13 @@ convert_for_assignment (tree type, tree 
 		 errtype);
     }
 
-  /* If -Wparentheses, warn about a = b = c when a has type bool.  */
+  /* If -Wparentheses, warn about a = b = c when a has type bool and b
+     does not.  */
   if (warn_parentheses
       && type == boolean_type_node
       && TREE_CODE (rhs) == MODIFY_EXPR
-      && !TREE_NO_WARNING (rhs))
+      && !TREE_NO_WARNING (rhs)
+      && TREE_TYPE (rhs) != boolean_type_node)
     {
       warning (OPT_Wparentheses,
 	       "suggest parentheses around assignment used as truth value");
Index: gcc/testsuite/g++.dg/warn/Wparentheses-24.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wparentheses-24.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wparentheses-24.C	(revision 0)
@@ -0,0 +1,14 @@
+// { dg-do compile }
+// { dg-options "-Wparentheses" }
+
+extern int foo (int);
+
+bool a, b, c;
+
+bool
+bar ()
+{
+  c = a = b;
+  foo (0);
+  return a = b;
+}

Re: PATCH RFA: Fix -Wparentheses fallout

[Daniel Jacobowitz]

On Sun, Jan 07, 2007 at 09:16:37PM -0800, Ian Lance Taylor wrote:
> +bool
> +bar ()
> +{
> +  c = a = b;
> +  foo (0);
> +  return a = b;
> +}

Maybe it's just me, but I think that second one deserves a warning.
If it's possible to distinguish the two, anyway.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH RFA: Fix -Wparentheses fallout

[Ian Lance Taylor]

Daniel Jacobowitz <drow@false.org> writes:

> On Sun, Jan 07, 2007 at 09:16:37PM -0800, Ian Lance Taylor wrote:
> > +bool
> > +bar ()
> > +{
> > +  c = a = b;
> > +  foo (0);
> > +  return a = b;
> > +}
> 
> Maybe it's just me, but I think that second one deserves a warning.
> If it's possible to distinguish the two, anyway.

Hmmm.  I guess I don't think it deserves a warning.  Consider

int glob;
int
foo (int i)
{
  return glob = i;
}

It seems to me that we shouldn't warn about that.  And it is
isomorphic to the example with bool (in the code quoted above, all
variables have type bool).

Any other opinions?

Ian

Re: PATCH RFA: Fix -Wparentheses fallout

[Gabriel Dos Reis]

Ian Lance Taylor <iant@google.com> writes:

| Daniel Jacobowitz <drow@false.org> writes:
| 
| > On Sun, Jan 07, 2007 at 09:16:37PM -0800, Ian Lance Taylor wrote:
| > > +bool
| > > +bar ()
| > > +{
| > > +  c = a = b;
| > > +  foo (0);
| > > +  return a = b;
| > > +}
| > 
| > Maybe it's just me, but I think that second one deserves a warning.
| > If it's possible to distinguish the two, anyway.
| 
| Hmmm.  I guess I don't think it deserves a warning.  Consider
| 
| int glob;
| int
| foo (int i)
| {
|   return glob = i;
| }
| 
| It seems to me that we shouldn't warn about that.  And it is
| isomorphic to the example with bool (in the code quoted above, all
| variables have type bool).
| 
| Any other opinions?

The above, e.g. return lhs = rhs, is an idiomatic construct in C++, to
the point that it deserved a departure from C.
But, I'm unclear about the audience of -Wparenthesis, so that may not
count. 

-- Gaby

Re: PATCH RFA: Fix -Wparentheses fallout

[Daniel Jacobowitz]

On Mon, Jan 08, 2007 at 09:13:07PM -0800, Ian Lance Taylor wrote:
> Hmmm.  I guess I don't think it deserves a warning.  Consider
> 
> int glob;
> int
> foo (int i)
> {
>   return glob = i;
> }
> 
> It seems to me that we shouldn't warn about that.  And it is
> isomorphic to the example with bool (in the code quoted above, all
> variables have type bool).

I would actually have warned about that too, though not if the function
returned float.  But I'm more of a C than C++ programmer so I'll take
Gaby's word for the idioms :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH RFA: Fix -Wparentheses fallout

[Gerald Pfeifer]

On Tue, 8 Jan 2007, Ian Lance Taylor wrote:
> 
> Hmmm.  I guess I don't think it deserves a warning.  Consider
> 
> int glob;
> int
> foo (int i)
> {
>   return glob = i;
> }
> 
> It seems to me that we shouldn't warn about that.  And it is
> isomorphic to the example with bool (in the code quoted above, all
> variables have type bool).
> 
> Any other opinions?

How about only issuing the warning if the LHS (glob in this case) is
a local variable?

Gerald

Re: PATCH RFA: Fix -Wparentheses fallout

[Ian Lance Taylor]

Gerald Pfeifer <gerald@pfeifer.com> writes:

> On Tue, 8 Jan 2007, Ian Lance Taylor wrote:
> > 
> > Hmmm.  I guess I don't think it deserves a warning.  Consider
> > 
> > int glob;
> > int
> > foo (int i)
> > {
> >   return glob = i;
> > }
> > 
> > It seems to me that we shouldn't warn about that.  And it is
> > isomorphic to the example with bool (in the code quoted above, all
> > variables have type bool).
> > 
> > Any other opinions?
> 
> How about only issuing the warning if the LHS (glob in this case) is
> a local variable?

That seems like a different warning to me, one about useless code, not
one about converting an assignment to a truth value.

Ian

Re: PATCH RFA: Fix -Wparentheses fallout

[Mark Mitchell]

Ian Lance Taylor wrote:

> cp/ChangeLog:
> 2007-01-07  Ian Lance Taylor  <iant@google.com>
> 
> 	* typeck.c (convert_for_assignment): Only warn about a = b = c
> 	when converting to bool.
> 
> testsuite/ChangeLog:
> 2007-01-07  Ian Lance Taylor  <iant@google.com>
> 
> 	* g++.dg/warn/Wparentheses-24.C: New test.

OK.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: PATCH RFA: Fix -Wparentheses fallout

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 12 bytes --]

Ping.

Ian


[-- Attachment #2: Type: message/rfc822, Size: 4984 bytes --]

From: Ian Lance Taylor <iant@google.com>
To: gcc-patches@gcc.gnu.org
Subject: PATCH RFA: Fix -Wparentheses fallout
Date: 07 Jan 2007 21:16:37 -0800
Message-ID: <m3r6u65c96.fsf@dhcp-172-18-118-195.corp.google.com>

The -Wparentheses patch I recently checked in accidentally warns about
    a = b = c;
where all variables have type bool.  That is not intended.  This patch
fixes the problem, and adds test cases.

Thanks to Ben Elliston for noticing the problem.

Bootstrapped and tested on i686-pc-linux-gnu.

OK for mainline?

Ian


cp/ChangeLog:
2007-01-07  Ian Lance Taylor  <iant@google.com>

	* typeck.c (convert_for_assignment): Only warn about a = b = c
	when converting to bool.

testsuite/ChangeLog:
2007-01-07  Ian Lance Taylor  <iant@google.com>

	* g++.dg/warn/Wparentheses-24.C: New test.


Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 120526)
+++ gcc/cp/typeck.c	(working copy)
@@ -6378,11 +6378,13 @@ convert_for_assignment (tree type, tree 
 		 errtype);
     }
 
-  /* If -Wparentheses, warn about a = b = c when a has type bool.  */
+  /* If -Wparentheses, warn about a = b = c when a has type bool and b
+     does not.  */
   if (warn_parentheses
       && type == boolean_type_node
       && TREE_CODE (rhs) == MODIFY_EXPR
-      && !TREE_NO_WARNING (rhs))
+      && !TREE_NO_WARNING (rhs)
+      && TREE_TYPE (rhs) != boolean_type_node)
     {
       warning (OPT_Wparentheses,
 	       "suggest parentheses around assignment used as truth value");
Index: gcc/testsuite/g++.dg/warn/Wparentheses-24.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wparentheses-24.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wparentheses-24.C	(revision 0)
@@ -0,0 +1,14 @@
+// { dg-do compile }
+// { dg-options "-Wparentheses" }
+
+extern int foo (int);
+
+bool a, b, c;
+
+bool
+bar ()
+{
+  c = a = b;
+  foo (0);
+  return a = b;
+}