<html dir="ltr">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
<style id="owaParaStyle" type="text/css">
<!--
p
{margin-top:0;
margin-bottom:0}
-->
P {margin-top:0;margin-bottom:0;}</style>
</head>
<body ocsi="0" fpstyle="1">
<div style="direction: ltr;font-family: Tahoma;color: #000000;font-size: 10pt;"><br>
ping<br>
<div><br>
<div style="font-size:13px; font-family:Tahoma">
<p class="MsoNormal"><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US">.......................................................................................................................</span><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:black" lang="EN-US"><br>
Per Viberg </span><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US">Senior Engineer</span><span style="font-size:8.5pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US"><br>
Evidente ES East</span><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US"> AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
</span><span style="font-size:10pt; font-family:'Tahoma','sans-serif'; color:black" lang="EN-US"></span></p>
<p class="MsoNormal"><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-GB">Phone: +46 (0)8 402 79 00<br>
Mobile: +46 (0)70 912 42 52<br>
E-mail: <a href="mailto:Per.Viberg@evidente.se" target="_blank"><font color="#0000ff">Per.Viberg@evidente.se</font></a>
</span><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:black" lang="EN-GB"><br>
<br>
<a href="http://www.evidente.se" target="_blank"><font color="#0000ff">www.evidente.se</font></a></span></p>
<p class="MsoNormal"><span style="font-size:6pt; font-family:'Arial','sans-serif'" lang="EN-GB">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.</span></p>
</div>
</div>
<div style="font-family: Times New Roman; color: #000000; font-size: 16px">
<hr tabindex="-1">
<div style="direction: ltr;" id="divRpF630653"><font color="#000000" size="2" face="Tahoma"><b>Från:</b> Per Viberg<br>
<b>Skickat:</b> den 20 december 2013 14:47<br>
<b>Till:</b> cfe-commits@cs.uiuc.edu<br>
<b>Cc:</b> Richard Trieu; Jordan Rose; Anders Rönnholm<br>
<b>Ämne:</b> SV: [PATCH] new check checking comparing of boolean expressions and literals<br>
</font><br>
</div>
<div></div>
<div>
<div style="direction:ltr; font-family:Tahoma; color:#000000; font-size:10pt"><br>
Hi,<br>
<br>
I agree with comments below from Richard Trieu. <br>
<br>
I found an existing check, DiagnoseOutOfRangeComparison in SemaChecking.cpp that was checking for always true/false expressions by range checking variables.
<br>
<br>
It didn't find the special cases comparing boolean expressions with literals 1 and 0 though. Neither did it range check boolean expressions in C. I extended that check to detect always true/false cases of:
<br>
<font face="Courier New"><br>
C++:<br>
bool > 1: always false<br>
bool <= 1: always true<br>
bool < 0: always false<br>
bool >= 0: always true<br>
<br>
0 <= bool: always true<br>
0 > bool: always false<br>
1 < bool: always false<br>
1 >= bool: always true<br>
<br>
C:<br>
boolean expression > 1 or positive values: always false<br>
boolean expression <= 1 or positive values: always true<br>
boolean expression < 0 or negative values: always false<br>
boolean expression >= 0 or negative values: always true<br>
<br>
0 or negative values <= boolean expression: always true<br>
0 or negative values > boolean expression: always false<br>
1 or positive values < boolean expression: always false<br>
1 or positive values >= boolean expression: always true<br>
</font><br>
patch is attached.<br>
<br>
Best regards<br>
/Per
<div><br>
<div style="font-size:13px; font-family:Tahoma">
<p class="MsoNormal"><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US">.......................................................................................................................</span><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:black" lang="EN-US"><br>
Per Viberg </span><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US">Senior Engineer</span><span style="font-size:8.5pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US"><br>
Evidente ES East</span><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US"> AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
</span><span style="font-size:10pt; font-family:'Tahoma','sans-serif'; color:black" lang="EN-US"></span></p>
<p class="MsoNormal"><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-GB">Phone: +46 (0)8 402 79 00<br>
Mobile: +46 (0)70 912 42 52<br>
E-mail: <a href="mailto:Per.Viberg@evidente.se" target="_blank"><font color="#0000ff">Per.Viberg@evidente.se</font></a>
</span><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:black" lang="EN-GB"><br>
<br>
<a href="http://www.evidente.se" target="_blank"><font color="#0000ff">www.evidente.se</font></a></span></p>
<p class="MsoNormal"><span style="font-size:6pt; font-family:'Arial','sans-serif'" lang="EN-GB">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.</span></p>
</div>
</div>
<div style="font-family:Times New Roman; color:#000000; font-size:16px">
<hr tabindex="-1">
<div id="divRpF737714" style="direction:ltr"><font color="#000000" size="2" face="Tahoma"><b>Från:</b> Richard Trieu [rtrieu@google.com]<br>
<b>Skickat:</b> den 12 november 2013 22:57<br>
<b>Till:</b> Jordan Rose<br>
<b>Cc:</b> Per Viberg; cfe-commits@cs.uiuc.edu<br>
<b>Ämne:</b> Re: [PATCH] new check checking comparing of boolean expressions and literals<br>
</font><br>
</div>
<div></div>
<div>
<div dir="ltr">This warning should not be in -Wtautological-compare since not all cases always evaluate to the same value every time. For instance,
<div><br>
</div>
<div>bool test(bool x) {</div>
<div> return x > 0;</div>
<div>}</div>
<div><br>
</div>
<div>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.
<div class="gmail_extra"><br>
<div class="gmail_quote">On Tue, Nov 12, 2013 at 9:13 AM, Jordan Rose <span dir="ltr">
<<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left-width:1px; border-left-color:rgb(204,204,204); border-left-style:solid; padding-left:1ex">
Oops, two pings and still managed to miss this. Richard, can you take a look as well?<br>
<br>
Some comments from me:<br>
<br>
- 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.<br>
<br>
- 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.<br>
<br>
- 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.<br>
<br>
+ if (z ==(j<y)) {} //<br>
+ if (z<k>y) {} //<br>
+ if (z > (l<y)) {} //<br>
+ if((z<y)>(n<y)) {} //<br>
+ if((z<y)==(o<y)){} //<br>
+ if((z<y)!=(p<y)){} //<br>
+ if((y==z)<(z==x)){} //<br>
+ if(((z==x)<(y==z))!=(q<y)){}//<br>
<br>
- 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).<br>
<br>
- Wow, how did we miss UO_LNot as known-boolean?<br>
</blockquote>
<div>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. </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left-width:1px; border-left-color:rgb(204,204,204); border-left-style:solid; padding-left:1ex">
<span class=""><font color="#888888"><br>
Jordan<br>
</font></span>
<div class="">
<div class="h5"><br>
<br>
On Oct 25, 2013, at 8:27 , Per Viberg <<a href="mailto:Per.Viberg@evidente.se" target="_blank">Per.Viberg@evidente.se</a>> wrote:<br>
<br>
> Hi,<br>
><br>
> Haven't received any comments, thus resending a patch from last week. Is anyone reviewing it?. it would be highly appreciated.<br>
><br>
> Best regards,<br>
> Per<br>
><br>
><br>
><br>
> .......................................................................................................................<br>
><br>
> Hello,<br>
><br>
> The patch adds a check that warns for relational comparison of boolean expressions with literals 1 and 0.<br>
> example:<br>
><br>
> bool x;<br>
> int i,e,y;<br>
><br>
> if(x > 1){}<br>
> if(!i < 0){}<br>
> if(1 > (e<y)){}<br>
><br>
> generates warning:<br>
> "relational comparison of boolean expression against literal value '1' or '0' "<br>
><br>
> see test files bool-compare.c and bool-compare.cpp for more details.<br>
><br>
><br>
> Best regards,<br>
> Per<br>
><br>
> .......................................................................................................................<br>
> Per Viberg Senior Engineer<br>
> Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden<br>
> Phone: +46 (0)8 402 79 00<br>
> Mobile: +46 (0)70 912 42 52<br>
> E-mail: <a href="mailto:Per.Viberg@evidente.se" target="_blank">Per.Viberg@evidente.se</a><br>
><br>
> <a href="http://www.evidente.se" target="_blank">www.evidente.se</a><br>
> 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.<br>
</div>
</div>
<br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
<br>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</body>
</html>