On 09/29/16 20:52, Bernd Edlinger wrote: > On 09/29/16 20:03, Jason Merrill wrote: >> >> What do you think about dropping the TYPE_UNSIGNED exception as well? >> I don't see what difference that makes. >> > > > If I drop that exception, then I could also drop the check for > INTEGER_TYPE and the whole if, because I think other types can not > happen, but if they are allowed they are as well bogus here. > > I can try a bootstrap and see if there are false positives. > > But I can do that as well in a follow-up patch, this should probably > be done step by step, especially when it may trigger some false > positives. > > I think I could also add more stuff, like unary + or - ? > or maybe also binary +, -, * and / ? > > We already discussed making this a multi-level option, > and maybe enabling the higher level explicitly in the > boot-strap. > > As long as the warning continues to find more bugs than false > positives, it is probably worth extending it to more cases. > > However unsigned integer shift are not undefined if they overflow. > > It is possible that this warning will then trigger also on valid > code that does loop termination with unsigned int left shifting. > I dont have a real example, but maybe like this hypothetical C-code: > > unsigned int x=1, bits=0; > while (x << bits) bits++; > printf("bits=%d\n", bits); > > > Is it OK for everybody to warn for this on -Wall, or maybe only > when -Wextra or for instance -Wint-in-bool-context=2 is used ? > >
Unfortunately, without that exception there is a false positive: In file included from ../../gcc-trunk/gcc/ada/gcc-interface/decl.c:30:0: ../../gcc-trunk/gcc/ada/gcc-interface/decl.c: In function 'int adjust_packed(tree, tree, int)': ../../gcc-trunk/gcc/tree.h:1874:22: error: << on signed integer in boolean context [-Werror=int-in-bool-context] ? ((unsigned)1) << ((NODE)->type_common.align - 1) : 0) ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../gcc-trunk/gcc/ada/gcc-interface/decl.c:6928:7: note: in expansion of macro 'TYPE_ALIGN' if (TYPE_ALIGN (record_type) ^~~~~~~~~~ But that did not happen with this version: Index: c-common.c =================================================================== --- c-common.c (revision 240571) +++ c-common.c (working copy) @@ -4655,6 +4655,14 @@ c_common_truthvalue_conversion (location_t locatio return c_common_truthvalue_conversion (location, TREE_OPERAND (expr, 0)); + case LSHIFT_EXPR: + /* Warn on signed integer left shift. */ + if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE + && !TYPE_UNSIGNED (TREE_TYPE (expr))) + warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, + "<< on signed integer in boolean context"); + break; + case COND_EXPR: if (warn_int_in_bool_context && !from_macro_definition_at (EXPR_LOCATION (expr))) Is that version OK for you? Bernd.