Re: suggest parentheses around assignment used as truth value

Re: suggest parentheses around assignment used as truth value

[Brad M. Garcia]

> I'd just like to register my opiniont that code like
> 
> if (x = p()) {...}
> 
> shouldn't generate a warning under gcc -Wall.  Even if
> you feel that code like this is obscure, the recommended
> 
> if ((x = p())) {...}
> 
> is just bizarre.

In 80% (don't you love making up statistics?) of cases where "=" is
used inside an "if" expression, the programmer meant to use "==".  So
the warning is helpful.

Now, it "suggests" using parentheses as a means to bypass the generation
of this warning.  You don't need to follow the suggestion, but the
suggestion results in the least amount of change to the source code.  
Other methods include moving the assignment outside of the "if"
expression, or using -Wno-parentheses in addition to -Wall.

Brad Garcia
   ___/  __ /  __ /  ___/ "Being the Linux of digital media
  __/   /  /  / _/  __/    would be a very good life."
_/    ____/ _/ _| ____/      - Jean-Louis Gassee, CEO of Be, Inc.

Re: suggest parentheses around assignment used as truth value

[Tim Hollebeek]

Brad M. Garcia writes ...
> 
> > I'd just like to register my opiniont that code like
> > 
> > if (x = p()) {...}
> > 
> > shouldn't generate a warning under gcc -Wall.  Even if
> > you feel that code like this is obscure, the recommended
> > 
> > if ((x = p())) {...}
> > 
> > is just bizarre.
> 
> In 80% (don't you love making up statistics?) of cases where "=" is
> used inside an "if" expression, the programmer meant to use "==".  So
> the warning is helpful.

Ah, that 80% statistic reminds me of something:

---
% cat >case.c
int main() {
    int x;
    
    switch (1) {
    case 0:
	x = 1;
    case 1:
	x = 2;
    }

    return x;
}
% gcc -Wall case.c
%
---

falling through a case is an error even more often (I've heard the
number 95% of the time tossed around).

I suggest a warning for it.  consecutive cases with no intervening
statements should not generate the warning.

Of course, we would need a syntactic way to specify when fallthrough
was desired, much like the /*FALLTHROUGH*/ lint comment.

I suggest:

    switch (1) {
    case 0:
	x = 1;
	;                  /* it is visually clear this one doesn't */
    case 1:
	x = 2;
	break;             /* this case has a break */
    }

I'm going to adopt this style myself, I think; I'd love it if someone
could add the warning for it to egcs.

---------------------------------------------------------------------------
Tim Hollebeek                           | "Everything above is a true
email: tim@wfn-shop.princeton.edu       |  statement, for sufficiently
URL: http://wfn-shop.princeton.edu/~tim |  false values of true."

Re: suggest parentheses around assignment used as truth value

[Harvey J. Stein]

"Brad M. Garcia" <bgarcia@fore.com> writes:

 > > I'd just like to register my opiniont that code like
 > > 
 > > if (x = p()) {...}
 > > 
 > > shouldn't generate a warning under gcc -Wall.  Even if
 > > you feel that code like this is obscure, the recommended
 > > 
 > > if ((x = p())) {...}
 > > 
 > > is just bizarre.
 > 
 > In 80% (don't you love making up statistics?) of cases where "=" is
 > used inside an "if" expression, the programmer meant to use "==".  So
 > the warning is helpful.

I believe the original poster might be making a distinction btw

   if (x = p()) ...

and

   if (x = y) ...

The second case (where the lvalue and rvalue are variables) is the one
where comparison is typically meant instead of assignment, and which
the gcc warning should apply to.

In the first case (where the rvalue is a function call) it's not at
all clear to me that comparison

   if (x == p()) ...

is more common than assignment

   if (x = p()) ...

or even that assignment is a common mistake when the programmer means
comparison.

There are bad things about both constructs.  The first is doing a
comparision inside the if test instead of an assignment, but is also
losing the return value of the function.  The second is keeping the
return value, which is a good thing, but at the cost of doing an
assignment inside of the if test.

It's not clear that gcc should value one over the other.

Maybe there should be a different independently settable warning for
the case where the rvalue is a function return?

-- 
Harvey J. Stein
BFM Financial Research
hjstein@bfr.co.il

Re: suggest parentheses around assignment used as truth value

[Kamil Iskra]

On Wed, 1 Jul 1998, Carlo Wood wrote:

> It is a very common mistake to type '=' where '==' was intended.
> Using extra parentheses is a very logical way to make the code
> more clear in what you mean.  Not only for the author itself,
> but also for others that read the code later and wonder if this
> is a typo/bug, or if it was intended (something not always clear
> from the context).
> 
> My personal opinion is that this warning is so useful that I'd
> even object to move it to -pedantic warnings.

I don't think that's a good idea. -pedantic serves completely different
pupose:


@item -pedantic
Issue all the warnings demanded by strict ANSI standard C; reject
all programs that use forbidden extensions.


AFAIK, ANSI doesn't require any diagnostic in this case.

/ Kamil Iskra    AmigaOS  Linux/i386  Linux/m68k               \
| GeekGadgets GCC maintainer   UNIX system administrator       |
| iskra@student.uci.agh.edu.pl  kiskra@ernie.icslab.agh.edu.pl |
\ kamil@dwd.interkom.pl   http://student.uci.agh.edu.pl/~iskra /

Re: suggest parentheses around assignment used as truth value

[Richard Earnshaw]

> 
> It's rather hard to double the parentheses in error, and the form does
> visually stand out.
> 

It's very easy to double the parentheses in error.  You just have to have 
the erroneous code inside a macro.

Somewhat simple I know, but:

#define COMPARE(X,Y) ((X)=(Y)) /* Should be == */

...

if (COMPARE(p,q)) {...}  /* Should get a warning, but doesn't */

Richard.

Re: suggest parentheses around assignment used as truth value

[Nathan Sidwell]

Richard Earnshaw wrote:
> 
> Of course the parentheses are necessary in that case, but the meaning is
> obvious, and the compiler can tell that a warning is unnecessary.
> 
> In my opinion, it's a bug if gcc doesn't emit a warning for
> 
>   if ((x = p())) {...}
> 
> but does for
> 
>   if (x = p()) {...}
> 
> since apart from an extra set of parentheses they are identical.  Perhaps
> the warning should be changed to "suggest explicit comparison of
> assignment used as truth value".
I think it'd be a mistake to change the behaviour of this warning and
its inhibiting syntax. It's been that way on gcc for too long. I for one
would be annoyed -- and you can tell how picky I can be about warnings
from some of my other posts :-)

It's rather hard to double the parentheses in error, and the form does
visually stand out.

nathan

-- 
Dr Nathan Sidwell :: Computer Science Department :: Bristol University
      You can up the bandwidth, but you can't up the speed of light      
nathan@acm.org  http://www.cs.bris.ac.uk/~nathan/  nathan@cs.bris.ac.uk

Re: suggest parentheses around assignment used as truth value

[Carlo Wood]

| I'd just like to register my opiniont that code like
| 
| if (x = p()) {...}
| 
| shouldn't generate a warning under gcc -Wall.  Even if
| you feel that code like this is obscure, the recommended
| 
| if ((x = p())) {...}
| 
| is just bizarre.
| 
| ---
| Marc

It is a very common mistake to type '=' where '==' was intended.
Using extra parentheses is a very logical way to make the code
more clear in what you mean.  Not only for the author itself,
but also for others that read the code later and wonder if this
is a typo/bug, or if it was intended (something not always clear
from the context).

My personal opinion is that this warning is so useful that I'd
even object to move it to -pedantic warnings.

-- 
 Carlo Wood  <carlo@runaway.xs4all.nl>

Re: suggest parentheses around assignment used as truth value

[Richard Earnshaw]

> Richard Earnshaw wrote:
> >         if ((x = p()) != 0) {...}
> > 
> > Then it becomes clear why the extra parentheses are needed.
> 
> Yes, because without the parentheses, it will be evaluated as:
> 
> 	if (x = (p() != 0)) {...}
> 
> And you're right back where you started from with a `suggest parentheses
> around assignment used as truth value' warning.
> 

Of course the parentheses are necessary in that case, but the meaning is 
obvious, and the compiler can tell that a warning is unnecessary.

In my opinion, it's a bug if gcc doesn't emit a warning for 

  if ((x = p())) {...}

but does for

  if (x = p()) {...}

since apart from an extra set of parentheses they are identical.  Perhaps 
the warning should be changed to "suggest explicit comparison of 
assignment used as truth value".


Richard.

Re: suggest parentheses around assignment used as truth value

[Bill Currie]

Richard Earnshaw wrote:
>         if ((x = p()) != 0) {...}
> 
> Then it becomes clear why the extra parentheses are needed.

Yes, because without the parentheses, it will be evaluated as:

	if (x = (p() != 0)) {...}

And you're right back where you started from with a `suggest parentheses
around assignment used as truth value' warning.

Bill
-- 
Leave others their otherness

Re: suggest parentheses around assignment used as truth value

[Richard Earnshaw]

> "Marc Rouaix" <rouaix@my-dejanews.com> writes:
> 
> |> I'd just like to register my opiniont that code like
> |> 
> |> if (x = p()) {...}
> |> 
> |> shouldn't generate a warning under gcc -Wall.
> 
> What if you have a typo here?
> 
> |>  Even if
> |> you feel that code like this is obscure, the recommended
> |> 
> |> if ((x = p())) {...}
> |> 
> |> is just bizarre.
> 
> Then write it as:
> 
> 	x = p();
> 	if (x) {...}
> 

Or,

	if ((x = p()) != 0) {...}

Then it becomes clear why the extra parentheses are needed.

Re: suggest parentheses around assignment used as truth value

[Branko Cibej]

Marc Rouaix wrote:

> I'd just like to register my opiniont that code like
>
> if (x = p()) {...}
>
> shouldn't generate a warning under gcc -Wall.

What if you meant to write "if (x == p()) {...}"? Such bugs are
usually very hard to find.

> Even if you feel that code like this is obscure,

It is _not_ obscure, it is in fact used quite often; therefore the
possibility of a typo is large enough to warrant a warning.

> the recommended
>
> if ((x = p())) {...}
>
> is just bizarre.

Can you recommend a better way of telling the compiler that you know
what you're doing? I mean, apart from the obvious

     x = p(); if (x) {...}

    Brane

--
Branko Cibej   <branko.cibej@hermes.si>
HERMES SoftLab, Litijska 51, 1000 Ljubljana, Slovenia
phone: (++386 61) 186 53 49  fax: (++386 61) 186 52 70

Re: suggest parentheses around assignment used as truth value

[Andreas Schwab]

"Marc Rouaix" <rouaix@my-dejanews.com> writes:

|> I'd just like to register my opiniont that code like
|> 
|> if (x = p()) {...}
|> 
|> shouldn't generate a warning under gcc -Wall.

What if you have a typo here?

|>  Even if
|> you feel that code like this is obscure, the recommended
|> 
|> if ((x = p())) {...}
|> 
|> is just bizarre.

Then write it as:

	x = p();
	if (x) {...}

-- 
Andreas Schwab                                      "And now for something
schwab@issan.informatik.uni-dortmund.de              completely different"
schwab@gnu.org

suggest parentheses around assignment used as truth value

[Marc Rouaix]

I'd just like to register my opiniont that code like

if (x = p()) {...}

shouldn't generate a warning under gcc -Wall.  Even if
you feel that code like this is obscure, the recommended

if ((x = p())) {...}

is just bizarre.

---
Marc



-----== Sent via Deja News, The Discussion Network ==-----
http://www.dejanews.com/  Easy access to 50,000+ discussion forums