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 <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
