<br><br><div class="gmail_quote">On Thu, Dec 8, 2011 at 10:23 PM, Anna Zaks <span dir="ltr"><<a href="mailto:ganna@apple.com">ganna@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div style="word-wrap:break-word"><br><div><div class="im"><div>On Dec 8, 2011, at 9:21 PM, David Blaikie wrote:</div><br></div><blockquote type="cite"><br><br><div class="gmail_quote"><div class="im">On Thu, Dec 8, 2011 at 9:14 PM, Anna Zaks <span dir="ltr"><<a href="mailto:ganna@apple.com" target="_blank">ganna@apple.com</a>></span> wrote:<br>
</div><blockquote class="gmail_quote" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div class="im"><br>
On Dec 8, 2011, at 6:23 PM, David Blaikie wrote:<br>
<br></div><div class="im">> Yeah - now I understand the problem, I wonder again if this could be<br>
> implemented (in a simpler but more strict manner) as a front-end<br>
> warning in the same way that null-conversions are warned about (though<br>
> it'd have to be a bit smarter - perhaps detecting the macro/typedef<br>
> that was used to declare a variable or assign a constant to it - not<br>
> to mention that the current Clang null-conversion support is broken<br>
> (I've another thread pending some discussion on that)). But in general<br>
> this should be good for any >2 state value masquerading as an int -<br>
> for C++ bool or anything that's as good as it, a lesser warning (if<br>
> any at all) might be appropriate.<br>
><br>
<br>
</div></div><div class="im">My understanding is that you are suggesting to add a compiler warning when a constant, which is not 0 nor 1, is being assigned to a boolean variable. Such warning would be weaker then this checker, so the checker should still go in.<br>
</div></blockquote><div class="im"><div><br>I'm certainly not sufficiently informed to make any claim to veto this checker - just tossing around some ideas.<br><br>I assume the "weakness" you're referring to, based on the compiler warning you described is that it wouldn't catch this:<br>
<br>unsigned i = 3;<br>BOOL j = i;<br></div></div></div></blockquote><div><br></div>Not really. I can imagine this case being handled by the compiler. Ex: seems straightforward for constant propagation(though we probably do not have that in the front end). </div>
</div></blockquote><div><br>Sorry, should've provided a more compelling example (& yes, the frontend has some amount of constant propagation, but it's fairly simplistic - doesn't infer through conditions or anything fancy like that, so far as I know).<br>
</div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word;"><div></div><div>The static analyzer can handle much more interesting cases (in addition to this one). Here is how most of the regression tests in the attached patch look like:</div>
<div><br></div><div><div>+void test_cppbool_initialization(int y) {</div><div>+ if (y < 0) {</div><div>+ bool x = y; // expected-warning {{Assignment of a non-Boolean value}}</div><div>+ return;</div><div>+ }</div>
<div>+ if (y > 1) {</div><div>+ bool x = y; // expected-warning {{Assignment of a non-Boolean value}}</div><div>+ return;</div><div>+ }</div><div>+ bool x = y; // no-warning</div><div>+}<br></div></div></div></blockquote>
<div><br>What I was suggesting was something a little, for what of a better term, dumber. Warning for any assignment from non-BOOL to BOOL. As I mentioned, perhaps this would be too noisy for existing code (I don't know enough about how average code uses these types to speak from any particular experience) but plausible for new code (& more immediate than running the static analyzer) - though perhaps even for new code this would be too restrictive, breaking common idioms, etc - I don't know.<br>
<br>But, yes, this seems valuable (higher accuracy than what I was suggesting - at the very least for legacy code it should be helpful, and my suggestion might be even too restrictive for new code) for pseudo-bool types (that are really multibit integer values capable of accidentally storing values other than 0 or 1 quite easily) but probably wrong for C++ true 'bool'.<br>
<br>- David<br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word;"><div><div></div><div class="im"><div>
<br></div><blockquote type="cite"><div class="gmail_quote"><div><br>That could be dealt with by checking that all assignments to/from BOOL (or whatever other pseuod bool types exist) are from other BOOLs - this might be somewhat noisy, I agree - the checker would be more surgical though the general warning might suffice, especially for new code.</div>
</div></blockquote></div></div><div><div class="im"><blockquote type="cite"><div class="gmail_quote"><div>
This might still be too expensive, I don't know - looking up function
& variable declarations whenever a BOOL expression is encountered to
see how the name was spelled. In which case something more like GNU's __null could be used - a custom, hidden type that looks just like char or whatever the intended underlying type is.<br><br></div></div></blockquote>
<br></div><div>I am not sure exactly what you are proposing. Sounds similar to type checking and disallowing implicit casts even from int, which is probably too noisy for a warning (and even for a checker).</div><div class="im">
<div><br></div><blockquote type="cite"><div class="gmail_quote"><div>- David<br> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Cheers,<br>
Anna.<br>
<div><br>
> The usual warning with Win32 BOOL is to never mix BOOL & bool - but<br>
> with this kind of warning/checking you could be confident that BOOL is<br>
> only 0 or 1 and so "someBOOL == some_bool" would work sensibly (&<br>
> never be "2 == 1" => false, for example)<br>
><br>
> - David<br>
><br>
</div><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>
<br>
</div></div></blockquote></div><br>
</blockquote></div></div><br></div></blockquote></div><br>