<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On 29 Jan 2013, at 03:30, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=us-ascii"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jan 27, 2013, at 13:40 , Richard <<a href="mailto:tarka.t.otter@googlemail.com">tarka.t.otter@googlemail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=us-ascii"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On 25 Jan 2013, at 18:51, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=us-ascii"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>I see what you mean about duplicated checks. Here's an idea: put all the checks in evalCastFromLoc under a special case for casting to bool. Then in evalBinOpLL:</div><div><br></div><div><font face="Courier">if (rInt->isZeroConstant()) {</font></div><div><font face="Courier"> if (op == BO_Sub)</font></div><div><font face="Courier"> return evalCastFromLoc(lhs, resultTy);</font></div><div><font face="Courier"> if (BinaryOperator::isComparisonOp(op))</font></div><div><font face="Courier"> return evalBinOpNN(state, op,</font></div><div><font face="Courier"> evalCastFromLoc(lhs, getContext().BoolTy),</font></div><div><font face="Courier"> makeTruthVal(false, getContext().BoolTy));</font></div><div><span style="font-family: Courier; ">}</span></div><div><br></div><div>That way you only have the ugly comparison once, and it's in SValBuilder rather than ConstraintManager.</div></div></blockquote><br></div><div>This is a nice idea, but I don't think it will work. The problem is that evalCastFromLoc will then expect different return values for the same inputs depending on whether it is being called from evalBinOpLL or evalCast. I gave it a quick go, but it caused many tests to fail for varying reasons. </div><div></div></div></blockquote><div><br></div><div>I'm confused by this. When is a cast of a weak function pointer to bool going to be different from the symbolic value cast to bool? (Or the symbolic value itself, if you type it as 'bool' for now.) And a strong function pointer casted to bool should just be true, always.</div><div><br></div></div></div></blockquote><div><br></div><div>I believe the problem was trying to match the default behaviour of the switch in evalBinOpLL, which returns true for any MemRegionVal, but evalCast returns UnknownVal in some cases. The tests failing were mostly c++ casting that expected unknown values but were getting trues instead. In other cases checkers were getting null pointers unexpectedly, which caused lots of malloc and retain / release checkers to fail. eg</div><div><br></div><div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "><span style="color: #bb2ca2">void</span> testNewArrayNoThrow() {</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> clang_analyzer_eval(<span style="color: #bb2ca2">new</span> (<span style="color: #272ad8">1</span>) NoThrow[<span style="color: #272ad8">2</span>]); <span style="color: #008400">// expected-warning{{UNKNOWN}}, but got TRUE</span></div><div style="margin: 0px; font-size: 14px; font-family: Monaco; ">}</div></div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">How about the attached diff instead? The function could be named more helpfully, but it does succeed at putting the ugly code in a single place, while changing the architecture as little as possible. </div></blockquote><div><br></div><div>One thing on the tests -- the analyzer treats (!foo) as (foo != 0), so that's not testing the same thing as (foo).</div></div></div></blockquote></div><br><div>Updated.</div><div><br></div><div></div></body></html>