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

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

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.

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;
 
       middle_loc = c_parser_peek_token (parser)->location;
-      pedwarn (middle_loc, OPT_Wpedantic, 
+      pedwarn (middle_loc, OPT_Wpedantic,
 	       "ISO C forbids omitting the middle term of a ?: expression");
-      warn_for_omitted_condop (middle_loc, cond.value);
       if (TREE_CODE (cond.value) == EXCESS_PRECISION_EXPR)
 	{
 	  eptype = TREE_TYPE (cond.value);
 	  cond.value = TREE_OPERAND (cond.value, 0);
 	}
+      e = cond.value;
+      while (TREE_CODE (e) == COMPOUND_EXPR)
+	e = TREE_OPERAND (e, 1);
+      warn_for_omitted_condop (middle_loc, e);
       /* Make sure first operand is calculated only once.  */
       exp1.value = c_save_expr (default_conversion (cond.value));
       if (eptype)
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))
+      warning_at (location, OPT_Wparentheses,
 		"the omitted middle operand in ?: will always be %<true%>, "
 		"suggest explicit middle operand");
-} 
+}
 
 /* Give an error for storing into ARG, which is 'const'.  USE indicates
    how ARG was being used.  */
Index: gcc/cp/call.c
===================================================================
--- gcc/cp/call.c	(revision 240001)
+++ gcc/cp/call.c	(working copy)
@@ -4653,9 +4653,12 @@ build_conditional_expr_1 (location_t loc, tree arg
   if (!arg2)
     {
       if (complain & tf_error)
-	pedwarn (loc, OPT_Wpedantic, 
+	pedwarn (loc, OPT_Wpedantic,
 		 "ISO C++ forbids omitting the middle term of a ?: expression");
 
+      if ((complain & tf_warning) && !truth_value_p (TREE_CODE (arg1)))
+	warn_for_omitted_condop (loc, arg1);
+
       /* Make sure that lvalues remain lvalues.  See g++.oliva/ext1.C.  */
       if (lvalue_p (arg1))
 	arg2 = arg1 = cp_stabilize_reference (arg1);
Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 240001)
+++ gcc/cp/class.c	(working copy)
@@ -8262,7 +8262,12 @@ instantiate_type (tree lhstype, tree rhs, tsubst_f
       return error_mark_node;
     }
 
-  /* There only a few kinds of expressions that may have a type
+  /* If we instantiate a template, and it is a A ?: C expression
+     with omitted B, look through the SAVE_EXPR.  */
+  if (TREE_CODE (rhs) == SAVE_EXPR)
+    rhs = TREE_OPERAND (rhs, 0);
+
+  /* There are only a few kinds of expressions that may have a type
      dependent on overload resolution.  */
   gcc_assert (TREE_CODE (rhs) == ADDR_EXPR
 	      || TREE_CODE (rhs) == COMPONENT_REF
Index: gcc/testsuite/c-c++-common/warn-ommitted-condop.c
===================================================================
--- gcc/testsuite/c-c++-common/warn-ommitted-condop.c	(revision 240001)
+++ gcc/testsuite/c-c++-common/warn-ommitted-condop.c	(working copy)
@@ -1,11 +1,15 @@
 /* { dg-options "-Wparentheses -ftrack-macro-expansion=0" } */
 
+#ifndef __cplusplus
+#define bool _Bool
+#endif
+
 extern void f2 (int);
 
-void bar (int x, int y, int z)
+void bar (int x, int y, int z, bool b)
 {
-#define T(op) f2 (x op y ? : 1) 
-#define T2(op) f2 (x op y ? 2 : 1) 
+#define T(op) f2 (x op y ? : 1)
+#define T2(op) f2 (x op y ? 2 : 1)
 
   T(<); /* { dg-warning "omitted middle operand" } */
   T(>); /* { dg-warning "omitted middle operand" } */
@@ -16,6 +20,8 @@ extern void f2 (int);
   T(||); /* { dg-warning "omitted middle operand" } */
   T(&&); /* { dg-warning "omitted middle operand" } */
   f2 (!x ? : 1);  /* { dg-warning "omitted middle operand" } */
+  f2 ((x,!x) ? : 1);  /* { dg-warning "omitted middle operand" } */
+  f2 ((x,y,!x) ? : 1);  /* { dg-warning "omitted middle operand" } */
   T2(<); /* { dg-bogus "omitted middle operand" } */
   T2(>); /* { dg-bogus "omitted middle operand" } */
   T2(==); /* { dg-bogus "omitted middle operand" } */
@@ -26,4 +32,5 @@ extern void f2 (int);
   T(*); /* { dg-bogus "omitted middle operand" } */
   T(/); /* { dg-bogus "omitted middle operand" } */
   T(^); /* { dg-bogus "omitted middle operand" } */
+  f2 (b ? : 1);  /* { dg-warning "omitted middle operand" } */
 }
Index: gcc/testsuite/g++.dg/ext/pr77496.C
===================================================================
--- gcc/testsuite/g++.dg/ext/pr77496.C	(revision 0)
+++ gcc/testsuite/g++.dg/ext/pr77496.C	(working copy)
@@ -0,0 +1,21 @@
+// { dg-do compile }
+// { dg-options "" }
+
+template <class x>
+class z : x
+{
+public:
+  bool zz () { return false; }
+  int f () { return zz ? : 1; } // { dg-error "cannot convert" }
+};
+
+class t
+{
+};
+
+int
+main ()
+{
+  z<t> x;
+  return x.f ();
+}
Index: gcc/testsuite/g++.dg/warn/pr77496.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr77496.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr77496.C	(working copy)
@@ -0,0 +1,21 @@
+// { dg-do compile }
+// { dg-options "-Wparentheses" }
+
+template <class x>
+class z : x
+{
+public:
+  bool zz () { return false; }
+  int f () { return zz () ? : 1; } // { dg-warning "omitted middle operand" }
+};
+
+class t
+{
+};
+
+int
+main ()
+{
+  z<t> x;
+  return x.f ();
+}