[cfe-commits] [Patch] Checker for assignment of non-Boolean value to Boolean variable

[Anna Zaks]

On Dec 8, 2011, at 9:21 PM, David Blaikie wrote:

> 
> 
> On Thu, Dec 8, 2011 at 9:14 PM, Anna Zaks <ganna at apple.com> wrote:
> 
> On Dec 8, 2011, at 6:23 PM, David Blaikie wrote:
> 
> > Yeah - now I understand the problem, I wonder again if this could be
> > implemented (in a simpler but more strict manner) as a front-end
> > warning in the same way that null-conversions are warned about (though
> > it'd have to be a bit smarter - perhaps detecting the macro/typedef
> > that was used to declare a variable or assign a constant to it - not
> > to mention that the current Clang null-conversion support is broken
> > (I've another thread pending some discussion on that)). But in general
> > this should be good for any >2 state value masquerading as an int -
> > for C++ bool or anything that's as good as it, a lesser warning (if
> > any at all) might be appropriate.
> >
> 
> 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.
> 
> I'm certainly not sufficiently informed to make any claim to veto this checker - just tossing around some ideas.
> 
> I assume the "weakness" you're referring to, based on the compiler warning you described is that it wouldn't catch this:
> 
> unsigned i = 3;
> BOOL j = i;

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). 

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:

+void test_cppbool_initialization(int y) {
+  if (y < 0) {
+    bool x = y; // expected-warning {{Assignment of a non-Boolean value}}
+    return;
+  }
+  if (y > 1) {
+    bool x = y; // expected-warning {{Assignment of a non-Boolean value}}
+    return;
+  }
+  bool x = y; // no-warning
+}

> 
> 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.

> 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.
> 

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).

> - David
>  
> 
> Cheers,
> Anna.
> 
> > The usual warning with Win32 BOOL is to never mix BOOL & bool - but
> > with this kind of warning/checking you could be confident that BOOL is
> > only 0 or 1 and so "someBOOL == some_bool" would work sensibly (&
> > never be "2 == 1" => false, for example)
> >
> > - David
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111208/6eec1834/attachment.html>