Hi,
this message did not get listed on the gcc-patches archive.
I've got no bounce, and it just vanished, several times.
Any idea what is wrong?
Bernd.
-------- Forwarded Message --------
Subject: [PATCHv3, resent] Add a warning for suspicious use of
conditional expressions in boolean context
Date: Thu, 15 Sep 2016 11:19:32 +0200
From: Bernd Edlinger <bernd.edlin...@hotmail.de>
To: GCC Patches <gcc-patc...@gcc.gnu.org>
CC: Jason Merrill <ja...@redhat.com>, Jeff Law <l...@redhat.com>, Joseph
Myers <jos...@codesourcery.com>
Hi,
I send the latest version of the warning patch again, because I don't
see the previous patch submission on the gcc-patches list. It dropped
out silently, twice :(
Don't know what went wrong, so please excuse me if this e-mail arrives
duplicate.
On 09/14/16 20:11, Jason Merrill wrote:
>>
>> Yes. The reasoning I initially had was that it is completely
>> pointless to have something of the form "if (x ? 1 : 2)" or
>> "if (x ? 0 : 0)" because the result does not even depend on x
>> in this case. But something like "if (x ? 4999 : 0)" looks
>> bogus but does at least not ignore x.
>>
>> If the false-positives are becoming too much of a problem here,
>> then I should of course revert to the previous heuristic again.
>
> I think we could have both, where the weaker form is part of -Wall and
> people can explicitly select the stronger form.
>
Yes, agreed. So here is what I would think will be the first version.
It can later be extended to cover the more pedantic cases which
will not be enabled by -Wall.
I would like to send a follow-up patch for the warning on
signed-integer shift left in boolean context, which I think
should also be good for Wall.
(I already had that feature in patch version 2 but that's meanwhile
outdated).
Bootstrap and reg-testing on x86_64-pc-linux-gnu.
Is it OK for trunk?
Thanks
Bernd.
gcc:
2016-09-14 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR c++/77434
* doc/invoke.texi: Document -Wcond-in-bool-context.
PR middle-end/77421
* dwarf2out.c (output_loc_operands): Fix an assertion.
c-family:
2016-09-14 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR c++/77434
* c.opt (Wcond-in-bool-context): New warning.
* c-common.c (c_common_truthvalue_conversion): Warn on integer
constants in boolean context.
cp:
2016-09-14 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR c++/77434
* cvt.c (cp_convert_and_check): Suppress Wint-in-bool-context here.
testsuite:
2016-09-14 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR c++/77434
* c-c++-common/Wcond-in-bool-context.c: New test.
Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c (revision 240135)
+++ gcc/builtins.c (working copy)
@@ -7887,15 +7887,18 @@ fold_builtin_classify (location_t loc, tree fndecl
tree isinf_call = build_call_expr_loc (loc, isinf_fn, 1, arg);
signbit_call = fold_build2_loc (loc, NE_EXPR, integer_type_node,
- signbit_call, integer_zero_node);
+ signbit_call, integer_zero_node);
isinf_call = fold_build2_loc (loc, NE_EXPR, integer_type_node,
- isinf_call, integer_zero_node);
+ isinf_call, integer_zero_node);
- tmp = fold_build3_loc (loc, COND_EXPR, integer_type_node, signbit_call,
- integer_minus_one_node, integer_one_node);
tmp = fold_build3_loc (loc, COND_EXPR, integer_type_node,
- isinf_call, tmp,
- integer_zero_node);
+ signbit_call, integer_minus_one_node,
+ integer_one_node);
+ /* Avoid a possible -Wint-in-bool-context warning in C. */
+ tmp = fold_build2_loc (loc, PLUS_EXPR, integer_type_node, tmp,
+ integer_zero_node);
+ tmp = fold_build3_loc (loc, COND_EXPR, integer_type_node,
+ isinf_call, tmp, integer_zero_node);
}
return tmp;
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c (revision 240135)
+++ gcc/c-family/c-common.c (working copy)
@@ -4652,6 +4652,19 @@ c_common_truthvalue_conversion (location_t locatio
TREE_OPERAND (expr, 0));
case COND_EXPR:
+ if (warn_int_in_bool_context)
+ {
+ tree val1 = fold_for_warn (TREE_OPERAND (expr, 1));
+ tree val2 = fold_for_warn (TREE_OPERAND (expr, 2));
+ if (TREE_CODE (val1) == INTEGER_CST
+ && TREE_CODE (val2) == INTEGER_CST
+ && !integer_zerop (val1)
+ && !integer_zerop (val2)
+ && (!integer_onep (val1)
+ || !integer_onep (val2)))
+ 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 240135)
+++ gcc/c-family/c.opt (working copy)
@@ -545,6 +545,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/cvt.c
===================================================================
--- gcc/cp/cvt.c (revision 240135)
+++ gcc/cp/cvt.c (working copy)
@@ -645,6 +645,7 @@ cp_convert_and_check (tree type, tree expr, tsubst
{
/* Avoid bogus -Wparentheses warnings. */
warning_sentinel w (warn_parentheses);
+ warning_sentinel c (warn_int_in_bool_context);
folded_result = cp_convert (type, folded, tf_none);
}
folded_result = fold_simple (folded_result);
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi (revision 240135)
+++ 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
@@ -5837,6 +5837,14 @@ 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)}.
+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 240135)
+++ 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,32 @@
+/* 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;
+
+ if (a == b ? 0 : 0) /* { dg-bogus "boolean context" } */
+ return 4;
+
+ if (a == ((b ? 2|4 : 1) & 3 ? 0 : 2)) /* { dg-bogus "boolean context" } */
+ return 5;
+
+ if (a ? 1 : 1+1) /* { dg-warning "boolean context" } */
+ return 6;
+
+ if (b ? 1+1 : 1) /* { dg-warning "boolean context" } */
+ return 7;
+
+ if (__builtin_isinf_sign ((float)a/b)) /* { dg-bogus "boolean context" } */
+ return 8;
+
+ return 0;
+}
