Re: [PATCHv2] Fix a few simple cases where -Wparentheses does not warn for omitted middle value

[Jeff Law <law@redhat.com>]

On 09/06/2016 04:07 PM, Bernd Edlinger wrote:
> On 09/05/16 23:50, Bernd Edlinger wrote:
>> > Hi,
>> >
>> > I've noticed that there is already a -Wparentheses warning for code like
>> >
>> >   int y = x == 2 ?: 1
>> >
>> > => warning: the omitted middle operand in ?: will always be 'true',
>> > suggest explicit middle operand [-Wparentheses]
>> >
>> > But it is not emitted for code that uses bool, like:
>> >
>> > void foo(bool x)
>> > {
>> >    int y = x ?: 1;
>> >
>> > and under C it is not emitted for compound expressions
>> > that end with a comparison, like:
>> >
>> >    int y = (i++,i==1) ?: 1;
>> >
>> > C++ is OK, but does only miss to warn on the bool data type.
>> >
>> > The attached patch should fix these warnings.
>> >
> Well, reg-testing showed few test cases were broken, that made me
> aware of an issue with templates when the LHS of ?: is dependent.
>
> In that case the type is not available at the template declaration,
> and the warning cannot be generated at the declaration but only when
> the template is instantiated.  The new patch fixes this, and a
> pre-existing issue, entered as PR 77496, when the type can not be
> implicitly converted to boolean.
>
>
> Boot-strapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Thanks
> Bernd.
>
>
> patch-omitted-cond-op.diff
>
>
> gcc/c-family:
> 2016-09-06  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>
> 	PR c++/77496
> 	* c-common.c (warn_for_omitted_condop): Also warn for boolean data.
>
> gcc/c:
> 2016-09-06  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>
> 	PR c++/77496
> 	* c-parser.c (c_parser_conditional_expression): Pass the rightmost
> 	COMPOUND_EXPR to warn_for_omitted_condop.
>
> gcc/cp:
> 2016-09-06  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>
> 	PR c++/77496
> 	* call.c (build_conditional_expr_1): Call warn_for_omitted_condop.
> 	* class.c (instantiate_type): Look through the SAVE_EXPR.
>
> gcc/testsuite:
> 2016-09-06  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>
> 	PR c++/77496
> 	* c-c++-common/warn-ommitted-condop.c: Add more test cases.
> 	* g++.dg/ext/pr77496.C: New test.
> 	* g++.dg/warn/pr77496.C: New test.
>
>
> Index: gcc/c/c-parser.c
> ===================================================================
> --- gcc/c/c-parser.c	(revision 240001)
> +++ gcc/c/c-parser.c	(working copy)
> @@ -6423,16 +6423,20 @@ c_parser_conditional_expression (c_parser *parser,
>    if (c_parser_next_token_is (parser, CPP_COLON))
>      {
>        tree eptype = NULL_TREE;
> +      tree e;
Move the declaration to where "e" is initialized.

> Index: gcc/c-family/c-common.c
> ===================================================================
> --- gcc/c-family/c-common.c	(revision 240001)
> +++ gcc/c-family/c-common.c	(working copy)
> @@ -10613,17 +10613,21 @@ fold_offsetof (tree expr)
>    return convert (size_type_node, fold_offsetof_1 (expr));
>  }
>
> -/* Warn for A ?: C expressions (with B omitted) where A is a boolean
> +/* Warn for A ?: C expressions (with B omitted) where A is a boolean
>     expression, because B will always be true. */
>
>  void
> -warn_for_omitted_condop (location_t location, tree cond)
> -{
> -  if (truth_value_p (TREE_CODE (cond)))
> -      warning_at (location, OPT_Wparentheses,
> +warn_for_omitted_condop (location_t location, tree cond)
> +{
> +  /* In C++ template declarations it can happen that the type is dependent
> +     and not yet known, thus TREE_TYPE (cond) == NULL_TREE.  */
> +  if (truth_value_p (TREE_CODE (cond))
> +      || (TREE_TYPE (cond) != NULL_TREE &&
> +	  TREE_CODE (TREE_TYPE (cond)) == BOOLEAN_TYPE))
THe "&&" at the end of the second condition goes on the next line.


With those two nits fixed, this is fine for the trunk.

jeff