<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>Richard, do you have any more comments for this? I have a few style notes I could give, but I think this is really better for you or someone else more in Sema to look at.</div><div><br></div><div>Jordan</div><div><br></div><br><div><div>On Jan 29, 2014, at 23:23 , Per Viberg <<a href="mailto:Per.Viberg@evidente.se">Per.Viberg@evidente.se</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div ocsi="0" fpstyle="1" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="direction: ltr; font-family: Tahoma; font-size: 10pt;"><br>ping<br><div><br><div style="font-size: 13px; font-family: Tahoma;"><div style="margin-top: 0px; margin-bottom: 0px;"><span lang="EN-US" style="font-size: 8pt; font-family: Arial, sans-serif; color: gray;">.......................................................................................................................</span><span lang="EN-US" style="font-size: 8pt; font-family: Arial, sans-serif;"><br>Per Viberg<span class="Apple-converted-space"> </span></span><span lang="EN-US" style="font-size: 8pt; font-family: Arial, sans-serif; color: gray;">Senior Engineer</span><span lang="EN-US" style="font-size: 8.5pt; font-family: Arial, sans-serif; color: gray;"><br>Evidente ES East</span><span lang="EN-US" style="font-size: 8pt; font-family: Arial, sans-serif; color: gray;"><span class="Apple-converted-space"> </span>AB Warfvinges väg 34 SE-112 51 Stockholm Sweden</span><span lang="EN-US" style="font-size: 10pt; font-family: Tahoma, sans-serif;"></span></div><div style="margin-top: 0px; margin-bottom: 0px;"><span lang="EN-GB" style="font-size: 8pt; font-family: Arial, sans-serif; color: gray;">Phone: +46 (0)8 402 79 00<br>Mobile: +46 (0)70 912 42 52<br>E-mail: <span class="Apple-converted-space"> </span><a href="mailto:Per.Viberg@evidente.se" target="_blank"><font color="#0000ff">Per.Viberg@evidente.se</font></a><span class="Apple-converted-space"> </span></span><span lang="EN-GB" style="font-size: 8pt; font-family: Arial, sans-serif;"><br><br><a href="http://www.evidente.se/" target="_blank"><font color="#0000ff">www.evidente.se</font></a></span></div><div style="margin-top: 0px; margin-bottom: 0px;"><span lang="EN-GB" style="font-size: 6pt; font-family: Arial, sans-serif;">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></div></div></div><div style="font-family: 'Times New Roman'; font-size: 16px;"><hr tabindex="-1"><div id="divRpF630653" style="direction: ltr;"><font size="2" face="Tahoma"><b>Från:</b><span class="Apple-converted-space"> </span>Per Viberg<br><b>Skickat:</b><span class="Apple-converted-space"> </span>den 20 december 2013 14:47<br><b>Till:</b><span class="Apple-converted-space"> </span><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br><b>Cc:</b><span class="Apple-converted-space"> </span>Richard Trieu; Jordan Rose; Anders Rönnholm<br><b>Ämne:</b><span class="Apple-converted-space"> </span>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; font-size: 10pt;"><br>Hi,<br><br>I agree with comments below from Richard Trieu.<span class="Apple-converted-space"> </span><br><br>I found an existing check, DiagnoseOutOfRangeComparison in SemaChecking.cpp that was checking for always true/false expressions by range checking variables.<span class="Apple-converted-space"> </span><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:<span class="Apple-converted-space"> </span><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;"><div style="margin-top: 0px; margin-bottom: 0px;"><span lang="EN-US" style="font-size: 8pt; font-family: Arial, sans-serif; color: gray;">.......................................................................................................................</span><span lang="EN-US" style="font-size: 8pt; font-family: Arial, sans-serif;"><br>Per Viberg<span class="Apple-converted-space"> </span></span><span lang="EN-US" style="font-size: 8pt; font-family: Arial, sans-serif; color: gray;">Senior Engineer</span><span lang="EN-US" style="font-size: 8.5pt; font-family: Arial, sans-serif; color: gray;"><br>Evidente ES East</span><span lang="EN-US" style="font-size: 8pt; font-family: Arial, sans-serif; color: gray;"><span class="Apple-converted-space"> </span>AB Warfvinges väg 34 SE-112 51 Stockholm Sweden</span><span lang="EN-US" style="font-size: 10pt; font-family: Tahoma, sans-serif;"></span></div><div style="margin-top: 0px; margin-bottom: 0px;"><span lang="EN-GB" style="font-size: 8pt; font-family: Arial, sans-serif; color: gray;">Phone: +46 (0)8 402 79 00<br>Mobile: +46 (0)70 912 42 52<br>E-mail: <span class="Apple-converted-space"> </span><a href="mailto:Per.Viberg@evidente.se" target="_blank"><font color="#0000ff">Per.Viberg@evidente.se</font></a><span class="Apple-converted-space"> </span></span><span lang="EN-GB" style="font-size: 8pt; font-family: Arial, sans-serif;"><br><br><a href="http://www.evidente.se/" target="_blank"><font color="#0000ff">www.evidente.se</font></a></span></div><div style="margin-top: 0px; margin-bottom: 0px;"><span lang="EN-GB" style="font-size: 6pt; font-family: Arial, sans-serif;">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></div></div></div><div style="font-family: 'Times New Roman'; font-size: 16px;"><hr tabindex="-1"><div id="divRpF737714" style="direction: ltr;"><font size="2" face="Tahoma"><b>Från:</b><span class="Apple-converted-space"> </span>Richard Trieu [<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>]<br><b>Skickat:</b><span class="Apple-converted-space"> </span>den 12 november 2013 22:57<br><b>Till:</b><span class="Apple-converted-space"> </span>Jordan Rose<br><b>Cc:</b><span class="Apple-converted-space"> </span>Per Viberg;<span class="Apple-converted-space"> </span><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br><b>Ämne:</b><span class="Apple-converted-space"> </span>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 class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span><span class="Apple-converted-space"> </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: <span class="Apple-converted-space"> </span><a href="mailto:Per.Viberg@evidente.se" target="_blank">Per.Viberg@evidente.se</a><br>><br>><span class="Apple-converted-space"> </span><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>><span class="Apple-converted-space"> </span><a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>><span class="Apple-converted-space"> </span><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></div></blockquote></div></body></html>