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

Jordan Rose jordan_rose at apple.com
Tue Nov 12 09:13:15 PST 2013


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?

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: relationalCompare_rev3_7.diff
Type: text/x-patch
Size: 16711 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131112/b7bd4d85/attachment.bin>
-------------- next part --------------
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list