On 09/15/2016 03:19 AM, Bernd Edlinger wrote:
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.
changelog-pr77434v3.txt
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.
patch-pr77434v3.diff
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;
This hunk is not mentioned in the ChangeLog and there's a good chance
this has or is going to change significantly. I don't like the tmp+0
workaround either. If there isn't an immediate need, can we let this
hunk slide and come back to it after the other changes from Tamar &
Wilco are wrapped up?
I think this is OK with the builtins.c hunk dropped as long as exclusion
of that hunk doesn't trigger spurious warnings.
jeff
