[PATCH] new check checking comparing of boolean expressions and literals

[Richard Trieu]

This warning should not be in -Wtautological-compare since not all cases
always evaluate to the same value every time.  For instance,

bool test(bool x) {
  return x > 0;
}

This function can still return true or false.  Check the other warnings in
-Wtautological-compare which has their messages end with "always evaluates
to " then some value.  Otherwise, this warning basically boil down to users
not writing code in the way we expected them to and then producing a vague
warning.  Possibly, split out the tautological parts to go into
-Wtautological-compare, then add additional notes, maybe with fix-it hints,
to help the user out.

On Tue, Nov 12, 2013 at 9:13 AM, Jordan Rose <jordan_rose at apple.com> wrote:

> Oops, two pings and still managed to miss this. Richard, can you take a
> look as well?
>
> Some comments from me:
>
> - I would check for the IntegerLiteral first, because that’s a simple
> isa<> check, whereas isKnownToHaveBooleanValue has to look at the
> expression in more detail.
>
> - We tend not to include empty else cases in LLVM code, even for
> commenting purposes. “else { return; }” is more okay, but in general we try
> to keep nesting and braces to a minimum.
>
> - String values can be broken up in TableGen, just like in C. Please keep
> the indentation consistent and break up the line into multiple string
> literal chunks.
>
> +  if (z ==(j<y))  {}   //
> +  if (z<k>y)      {}        //
> +  if (z > (l<y))  {}    //
> +  if((z<y)>(n<y)) {}   //
> +  if((z<y)==(o<y)){}  //
> +  if((z<y)!=(p<y)){}  //
> +  if((y==z)<(z==x)){}  //
> +  if(((z==x)<(y==z))!=(q<y)){}//
>
> - Some funky indentation on this set of test comments. I would actually
> just drop all the empty comment lines, or add "no-warning” (which doesn’t
> do anything, but looks like expected-warning).
>
> - Wow, how did we miss UO_LNot as known-boolean?
>
I know I missed it since I usually do getType()->isBooleanType(), which
worked great for C++ which has a boolean type, but not so well for C which
does not.

>
> Jordan
>
>
> On Oct 25, 2013, at 8:27 , Per Viberg <Per.Viberg at evidente.se> wrote:
>
> > Hi,
> >
> > Haven't received any comments, thus resending a patch from last week. Is
> anyone reviewing it?. it would be highly appreciated.
> >
> > Best regards,
> > Per
> >
> >
> >
> >
> .......................................................................................................................
> >
> > Hello,
> >
> > The patch adds a check that warns for relational comparison of boolean
> expressions with literals 1 and 0.
> > example:
> >
> > bool x;
> > int i,e,y;
> >
> > if(x > 1){}
> > if(!i < 0){}
> > if(1 > (e<y)){}
> >
> > generates warning:
> > "relational comparison of boolean expression against literal value '1'
> or '0' "
> >
> > see test files bool-compare.c and bool-compare.cpp for more details.
> >
> >
> > Best regards,
> > Per
> >
> >
> .......................................................................................................................
> > Per Viberg Senior Engineer
> > Evidente ES East AB  Warfvinges väg 34  SE-112 51 Stockholm  Sweden
> > Phone:    +46 (0)8 402 79 00
> > Mobile:    +46 (0)70 912 42 52
> > E-mail:     Per.Viberg at evidente.se
> >
> > www.evidente.se
> > This e-mail, which might contain confidential information, is addressed
> to the above stated person/company. If you are not the correct addressee,
> employee or in any other way the person concerned, please notify the sender
> immediately. At the same time, please delete this e-mail and destroy any
> prints. Thank You.
>
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131112/9908218a/attachment.html>