<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Dec 8, 2011, at 9:21 PM, David Blaikie wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><br><br><div class="gmail_quote">On Thu, Dec 8, 2011 at 9:14 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-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; position: static; z-index: auto; ">
<div class="im"><br>
On Dec 8, 2011, at 6:23 PM, David Blaikie wrote:<br>
<br>> 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>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>
</blockquote><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></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><br></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>+}</div><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><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>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><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 class="im HOEnZb"><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 class="HOEnZb"><div class="h5">> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">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><br></body></html>