<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 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><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>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></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><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div><div>Thoughts?</div><div><br></div><div></div></div><span><weak3.diff></span><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></div></div></blockquote></div><br></body></html>