<div dir="ltr">Clang already has a lot of checking code for conversions, which should be used for this checking.  To that end, this is how I think it should be laid out:<div><br></div><div>Define two new functions in SemaChecking.cpp:</div><div>void Sema::CheckBoolLikeConversion(Expr *E, SourceLocation CC)</div><div>void CheckBoolLikeConversion(Sema &S, Expr *E, SourceLocation CC)</div><div><br></div><div>The first function should be a private Sema function that serves as a forward to the second function so that SemaExpr.cpp can access it.  The second function should check the LangOpts and if bool is not present, call CheckImplicitConversion.  The arguments to this function should be the Sema reference, the Expr stripped of implicit casts, the bool type (S.Context.BoolTy), and the source location.</div><div><br></div><div>In SemaExpr.cpp, remove the changes to CheckLogicalOperands.  In CheckBooleanCondition, call CheckBoolLikeConversion where it is currently calling CheckAlwaysNonNullPointer.  This will catch the bool conversions in if(...), while(...), etc.</div><div><br></div><div>Catching !.. and ... && ... can be done within in the checking functions in SemaChecking.cpp, specifically, at the end of AnalyzeImplicitConversions.  If E is a unary expression with a logical not, call the CheckBoolLikeConversion on the sub-expression.  If E is 1 binary expression with a logical operator, call the function on both the LHS and RHS.</div><div><br></div><div>As for the testing, if the warning fires in an Analysis test, just disable the warning in the run line.  Also, it looks like some test cases were pulled in from the patch on nonnull.<br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 29, 2014 at 9:47 AM, jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@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"><div style="word-wrap:break-word">- Ping.<div><br></div><div>- Thanks, Fairborz</div><div><br><div><br><blockquote type="cite"><div>Begin forwarded message:</div><br><div style="margin:0px"><span style="font-family:-webkit-system-font,'Helvetica Neue',Helvetica,sans-serif;color:rgb(0,0,0)"><b>Subject: </b></span><span style="font-family:-webkit-system-font,'Helvetica Neue',Helvetica,sans-serif"><b>Re: patch for warning on logical negation with known result</b><br></span></div><div style="margin:0px"><span style="font-family:-webkit-system-font,'Helvetica Neue',Helvetica,sans-serif;color:rgb(0,0,0)"><b>From: </b></span><span style="font-family:-webkit-system-font,'Helvetica Neue',Helvetica,sans-serif">jahanian <<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>><br></span></div><div style="margin:0px"><span style="font-family:-webkit-system-font,'Helvetica Neue',Helvetica,sans-serif;color:rgb(0,0,0)"><b>Date: </b></span><span style="font-family:-webkit-system-font,'Helvetica Neue',Helvetica,sans-serif">October 24, 2014 at 2:08:10 PM PDT<br></span></div><div style="margin:0px"><span style="font-family:-webkit-system-font,'Helvetica Neue',Helvetica,sans-serif;color:rgb(0,0,0)"><b>To: </b></span><span style="font-family:-webkit-system-font,'Helvetica Neue',Helvetica,sans-serif">Richard Trieu <<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>><br></span></div><div style="margin:0px"><span style="font-family:-webkit-system-font,'Helvetica Neue',Helvetica,sans-serif;color:rgb(0,0,0)"><b>Cc: </b></span><span style="font-family:-webkit-system-font,'Helvetica Neue',Helvetica,sans-serif">cfe commits <<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a>><br></span></div><div><div class="h5"><br><div><div style="word-wrap:break-word">Minor change. I should not bail out early when types of implicit cast and its sub expression are identical as<div>implicit cast may be for lvalue to rvalue conversion. This showed up for this test case (with old patch warning is not being issued).</div><div><br></div><div><div style="margin:0px;font-size:11px;font-family:Menlo">__attribute__((__nonnull__))</div><div style="margin:0px;font-size:11px;font-family:Menlo">void test1(void *nonnull) {</div><div style="margin:0px;font-size:11px;font-family:Menlo">   if (nonnull) {}</div><div style="margin:0px;font-size:11px;font-family:Menlo">}</div><div style="margin:0px;font-size:11px;font-family:Menlo"></div></div></div></div></div></div></blockquote></div></div></div><br><div style="word-wrap:break-word"><div><div><blockquote type="cite"><div><div style="word-wrap:break-word"><div><div style="margin:0px;font-size:11px;font-family:Menlo"></div><div style="margin:0px;font-size:11px;font-family:Menlo"><br></div><div style="margin:0px;font-size:11px;font-family:Menlo">- Fariborz</div><div style="margin:0px;font-size:11px;font-family:Menlo"><br></div><div><blockquote type="cite"><div>On Oct 24, 2014, at 1:23 PM, jahanian <<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>> wrote:</div><br><div><div style="word-wrap:break-word">Thanks for the review. Here is the updated patch.<div><br></div><div><span style="white-space:pre-wrap"> </span></div></div><span><nonnull-patch.txt></span><div style="word-wrap:break-word"><div></div><div><br></div><div>- Fariborz</div><div><br></div></div></div></blockquote></div><br></div></div>_______________________________________________<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></div></blockquote></div><br></div></div><br></blockquote></div><br></div></div></div>