On Mon, Apr 2, 2012 at 9:44 PM, Nico Weber <[email protected]> wrote: > Do you have any numbers on bug / false positive ratios before and > after this change?
No, I don't have any numbers at the moment - the motivation was just to fill a fairly obvious hole in -Wnull-conversion that lead me to stumble across the other X-to-bool missing cases that seem as legitimate as any other target type for those diagnostics. I'll see what I can find across a broad code base. - David > 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
