On 09/02/16 20:52, Bernd Edlinger wrote: > Hi! > > As reported in PR77434 and PR77421 there should be a warning for > suspicious uses of conditional expressions with non-boolean arguments. > > This warning triggers on conditional expressions in boolean context, > when both possible results are non-zero integer constants, so that > the resulting truth value does in fact not depend on the condition > itself. Thus something like "if (a == b ? 1 : 2)" is always bogus, > and was most likely meant to be "if (a == (b ? 1 : 2))". > > > Boot-strap and reg-testing on x86_64-pc-linux-gnu without regressions. > Is it OK for trunk. > > > Thanks > Bernd.
Due to the discussion on the PR77434 I thought that it might be helpful to diagnose signed integer shift left in boolean context too, because the result can never depend on the shift count. It is more likely that the precedence of << and ?: may be the reason like in (x << y ? 1 : 2) == 4 which is apparently warned by clang and a -Wparantheses warning for this construct may be good to have as well. But this code would still be suspicious even if (x << y) is put in parentheses, because the shift count does not change the result of the condition, as the integer overflow is undefined behavior, and if it does not have side effects or does not throw something, it can even be optimized away. Therefore I thought that it might be good to generalize the -Wcond-in-bool-context warning to cover all kinds of suspicious integer ops in boolean context, making it a -Wint-in-bool-context warning. I tried to implement that and the warning immediately found something of the form "bool flags = 1<<2;" in cp/parser.c at cp_parser_condition. This should obviously be fixed by making flags an int. I tried a bit with declaring variables with "if (type v = x)", but was unable to find any test case where this actually made a difference. Boot-strap and reg-testing on x86_64-pc-linux-gnu without regressions. Is it OK for trunk? Thanks Bernd.
gcc: 2016-09-04 Bernd Edlinger <bernd.edlin...@hotmail.de> PR c++/77434 * doc/invoke.texi: Document -Wint-in-bool-context. PR middle-end/77421 * dwarf2out.c (output_loc_operands): Fix assertion. c-family: 2016-09-04 Bernd Edlinger <bernd.edlin...@hotmail.de> PR c++/77434 * c.opt (Wint-in-bool-context): New warning. * c-common.c (c_common_truthvalue_conversion): Warn for suspicious integer expressions in boolean context. cp: 2016-09-04 Bernd Edlinger <bernd.edlin...@hotmail.de> PR c++/77434 * parser.c (cp_parser_condition): Make flags an int. testsuite: 2016-09-04 Bernd Edlinger <bernd.edlin...@hotmail.de> PR c++/77434 * c-c++-common/Wint-in-bool-context.c: New test.
Index: gcc/c-family/c-common.c =================================================================== --- gcc/c-family/c-common.c (revision 239971) +++ gcc/c-family/c-common.c (working copy) @@ -4617,7 +4617,22 @@ c_common_truthvalue_conversion (location_t locatio return c_common_truthvalue_conversion (location, TREE_OPERAND (expr, 0)); + case LSHIFT_EXPR: + 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 (TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST + && TREE_CODE (TREE_OPERAND (expr, 2)) == INTEGER_CST + && !integer_zerop (TREE_OPERAND (expr, 1)) + && !integer_zerop (TREE_OPERAND (expr, 2)) + && (!integer_onep (TREE_OPERAND (expr, 1)) + || !integer_onep (TREE_OPERAND (expr, 2)))) + warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, + "?: using integer constants in boolean context"); /* Distribute the conversion into the arms of a COND_EXPR. */ if (c_dialect_cxx ()) { Index: gcc/c-family/c.opt =================================================================== --- gcc/c-family/c.opt (revision 239971) +++ gcc/c-family/c.opt (working copy) @@ -525,6 +525,10 @@ Wint-conversion C ObjC Var(warn_int_conversion) Init(1) Warning Warn about incompatible integer to pointer and pointer to integer conversions. +Wint-in-bool-context +C ObjC C++ ObjC++ Var(warn_int_in_bool_context) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) +Warn for suspicious integer expressions in boolean context. + Wint-to-pointer-cast C ObjC C++ ObjC++ Var(warn_int_to_pointer_cast) Init(1) Warning Warn when there is a cast to a pointer from an integer of a different size. Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 239971) +++ gcc/cp/parser.c (working copy) @@ -11172,7 +11172,7 @@ cp_parser_condition (cp_parser* parser) { tree pushed_scope; bool non_constant_p; - bool flags = LOOKUP_ONLYCONVERTING; + int flags = LOOKUP_ONLYCONVERTING; /* Create the declaration. */ decl = start_decl (declarator, &type_specifiers, Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 239971) +++ gcc/doc/invoke.texi (working copy) @@ -273,7 +273,7 @@ Objective-C and Objective-C++ Dialects}. -Wframe-larger-than=@var{len} -Wno-free-nonheap-object -Wjump-misses-init @gol -Wignored-qualifiers -Wignored-attributes -Wincompatible-pointer-types @gol -Wimplicit -Wimplicit-function-declaration -Wimplicit-int @gol --Winit-self -Winline -Wno-int-conversion @gol +-Winit-self -Winline -Wno-int-conversion -Wint-in-bool-context @gol -Wno-int-to-pointer-cast -Winvalid-memory-model -Wno-invalid-offsetof @gol -Winvalid-pch -Wlarger-than=@var{len} @gol -Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol @@ -5816,6 +5816,15 @@ warning about it. The restrictions on @code{offsetof} may be relaxed in a future version of the C++ standard. +@item -Wint-in-bool-context +@opindex Wint-in-bool-context +@opindex Wno-int-in-bool-context +Warn for suspicious use of integer values where boolean values are expected, +such as conditional expressions (?:) using non-boolean integer constants in +boolean context, like @code{if (a <= b ? 2 : 3)}. Or left shifting of a +signed integer in boolean context, like @code{for (a = 0; 1 << a; a++);}. +This warning is enabled by @option{-Wall}. + @item -Wno-int-to-pointer-cast @opindex Wno-int-to-pointer-cast @opindex Wint-to-pointer-cast Index: gcc/dwarf2out.c =================================================================== --- gcc/dwarf2out.c (revision 239971) +++ gcc/dwarf2out.c (working copy) @@ -2051,9 +2051,9 @@ output_loc_operands (dw_loc_descr_ref loc, int for /* Make sure the offset has been computed and that we can encode it as an operand. */ gcc_assert (die_offset > 0 - && die_offset <= (loc->dw_loc_opc == DW_OP_call2) + && die_offset <= (loc->dw_loc_opc == DW_OP_call2 ? 0xffff - : 0xffffffff); + : 0xffffffff)); dw2_asm_output_data ((loc->dw_loc_opc == DW_OP_call2) ? 2 : 4, die_offset, NULL); } Index: gcc/testsuite/c-c++-common/Wint-in-bool-context.c =================================================================== --- gcc/testsuite/c-c++-common/Wint-in-bool-context.c (revision 0) +++ gcc/testsuite/c-c++-common/Wint-in-bool-context.c (working copy) @@ -0,0 +1,19 @@ +/* PR c++/77434 */ +/* { dg-options "-Wint-in-bool-context" } */ +/* { dg-do compile } */ + +int foo (int a, int b) +{ + if (a > 0 && a <= (b == 1) ? 1 : 2) /* { dg-warning "boolean context" } */ + return 1; + + if (a > 0 && a <= (b == 2) ? 1 : 1) /* { dg-bogus "boolean context" } */ + return 2; + + if (a > 0 && a <= (b == 3) ? 0 : 2) /* { dg-bogus "boolean context" } */ + return 3; + + for (a = 0; 1 << a; a++); /* { dg-warning "boolean context" } */ + + return 0; +}