[cfe-commits] Support <x>-to-bool warnings for more contexts

[Nico Weber]

Do you have any numbers on bug / false positive ratios before and
after this change?

Nico

On Mon, Apr 2, 2012 at 6:03 PM, David Blaikie <dblaikie at gmail.com> wrote:
> SemaChecking.cpp:3989 currently returns early from checking implicit
> conversions after it tests some specific X-to-boolean cases (including
> string and funciton literals) but before checking various other cases
> later on like NULL-to-X and wide integer literal to narrow integer.
>
> This change removes the early return, fixes the diagnostic (to
> correctly emit the fact that non-zero literals produce a "true"
> boolean value rather than simply truncating the larger integer
> literal), and updates the tests. In some cases the test cases were
> fixed or updated (//expected-warning), in others I simply suppressed
> the diagnostic because there adding the expected-warnings would've
> added a lot of noise to the test cases*.
>
> * This last case is a little bit questionable: in one specific case we
> produce a really good diagnostic about constant integer literals used
> in boolean contexts:
>
> int f1();
> bool f2() {
>  return f1() && 42;
> }
>
> we produce:
>
> conv.cpp:3:15: warning: use of logical '&&' with constant operand
> [-Wconstant-logical-operand]
>  return f1() && 42;
>              ^  ~~
> conv.cpp:3:15: note: use '&' for a bitwise operation
>  return f1() && 42;
>              ^~
>              &
> conv.cpp:3:15: note: remove constant to silence this warning
>  return f1() && 42;
>             ~^~~~~
>
> But then with my patch we get an extra diagnostic after the above warning/notes:
>
> conv.cpp:3:18: warning: implicit conversion from 'int' to 'bool'
> changes value from 42 to true [-Wconstant-conversion]
>  return f1() && 42;
>              ~~ ^~
>
> which isn't great - since we already gave a much more specific
> diagnosis of the problem in the first warning. If there's some nice
> way that we could suppress the second one whenever the first one is
> provided (unless the first one is only a warning and the second is
> -Werror'd?) I'd be happy to implement that.
>
>
>
> Another thing I noticed as I was exploring this. We have a warning for
> float-literal-to-int such as:
>
> conv.cpp:2:9: warning: implicit conversion turns literal
> floating-point number into integer: 'double' to 'int'
> [-Wliteral-conversion]
> int i = 3.1415;
>    ~   ^~~~~~
>
> But this warning is off-by-default. Why is that? It's already
> relatively conservative (allowing things like : "int i = 3.0" because
> 3.0 converts to an int without loss of precision) - though it's not a
> DiagnoseRuntimeBehavior, which it could be changed to (to be
> consistent with similar things for integers like "unsigned char c =
> 256").
>
> Or is it really that common to deliberately use floating point
> literals to initialize integer values?
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>