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.edlin...@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.edlin...@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.edlin...@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.edlin...@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

Reply via email to