[PATCH] Warn about bogus conditional operators

[Andi Kleen <andi at firstfloor dot org>]

Some time ago I fell into the trap of typing x > y ? : y which 
is equivalent to x > y ? 1 : y 
was obviously not what I meant, but instead wanted to pass out x.
I don't think it's ever very useful to omit the second operand
if the condition is a boolean, so let's add a warning for it.

-Andi

2008-12-11  Andi Kleen  <ak@linux.intel.com>

	* doc/invoke.texi: Document -Wno-omitted-conditional-op
	* gcc.dg/warn-omitted-condop.c: Add
	* g++.dg/warn-omitted-condop.C: Add
	* cp/parser.c: (cp_parser_question_colon_clause): 
	Call warn_for_omitted_condop.
	* c-parser.c: (c_parser_conditional_expression) 
	Call warn_for_omitted_condop.
	* c-common.c (warn_for_omitted_condop): Add.
	* c-common.h (warn_for_omitted_condop): Add prototype.
	* common.opt: Add Womitted-conditional-op.

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 143064)
+++ gcc/doc/invoke.texi	(working copy)
@@ -263,7 +263,7 @@
 -Wunused  -Wunused-function  -Wunused-label  -Wunused-parameter @gol
 -Wunused-value  -Wunused-variable @gol
 -Wvariadic-macros -Wvla @gol
--Wvolatile-register-var  -Wwrite-strings}
+-Wvolatile-register-var  -Wwrite-strings -Womitted-conditional-op}
 
 @item C and Objective-C-only Warning Options
 @gccoptlist{-Wbad-function-cast  -Wmissing-declarations @gol
@@ -4146,6 +4146,15 @@
 If any of @var{sym} is called, GCC will issue a warning. This can be useful
 in enforcing coding conventions that ban calls to certain functions, for
 example, @code{alloca}, @code{malloc}, etc.
+
+@item -Wno-omitted-conditional-op
+@opindex Wno-omitted-conditional-op
+warn for dangerous uses of the 
+?: with omitted middle operand GNU extension. When the condition
+in the ?: operator is a computed boolean the omitted value will
+be always 1. Often the user expects it to be a value computed
+inside the conditional expression instead. Gcc by default warns 
+for this, but this option disables it.
 @end table
 
 @node Debugging Options
Index: gcc/testsuite/gcc.dg/warn-omitted-condop.c
===================================================================
--- gcc/testsuite/gcc.dg/warn-omitted-condop.c	(revision 0)
+++ gcc/testsuite/gcc.dg/warn-omitted-condop.c	(revision 0)
@@ -0,0 +1,29 @@
+/* { dg-options "-Womitted-conditional-op" } */
+
+extern void f2 (int);
+
+void bar (int x, int y, int z)
+{
+#define T(op) f2 (x op y ? : 1) 
+#define T2(op) f2 (x op y ? 2 : 1) 
+
+  T(<); /* { dg-warning "Omitting middle operand" } */
+  T(>); /* { dg-warning "Omitting middle operand" } */
+  T(<=); /* { dg-warning "Omitting middle operand" } */
+  T(>=); /* { dg-warning "Omitting middle operand" } */
+  T(==); /* { dg-warning "Omitting middle operand" } */
+  T(!=); /* { dg-warning "Omitting middle operand" } */
+  T(||); /* { dg-warning "Omitting middle operand" } */
+  T(&&); /* { dg-warning "Omitting middle operand" } */
+  f2 (!x ? : 1);  /* { dg-warning "Omitting middle operand" } */
+  T2(<); /* { dg-bogus "Omitting middle operand" } */
+  T2(>); /* { dg-bogus "Omitting middle operand" } */
+  T2(==); /* { dg-bogus "Omitting middle operand" } */
+  T2(||); /* { dg-bogus "Omitting middle operand" } */
+  T2(&&); /* { dg-bogus "Omitting middle operand" } */
+  T(+); /* { dg-bogus "Omitting middle operand" } */
+  T(-); /* { dg-bogus "Omitting middle operand" } */
+  T(*); /* { dg-bogus "Omitting middle operand" } */
+  T(/); /* { dg-bogus "Omitting middle operand" } */
+  T(^); /* { dg-bogus "Omitting middle operand" } */
+}
Index: gcc/testsuite/g++.dg/warn/warn-omitted-condop.C
===================================================================
--- gcc/testsuite/g++.dg/warn/warn-omitted-condop.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/warn-omitted-condop.C	(revision 0)
@@ -0,0 +1,29 @@
+/* { dg-options "-Womitted-conditional-op" } */
+
+extern void f2 (int);
+
+void bar (int x, int y, int z)
+{
+#define T(op) f2 (x op y ? : 1) 
+#define T2(op) f2 (x op y ? 2 : 1) 
+
+  T(<); /* { dg-warning "Omitting middle operand" } */
+  T(>); /* { dg-warning "Omitting middle operand" } */
+  T(<=); /* { dg-warning "Omitting middle operand" } */
+  T(>=); /* { dg-warning "Omitting middle operand" } */
+  T(==); /* { dg-warning "Omitting middle operand" } */
+  T(!=); /* { dg-warning "Omitting middle operand" } */
+  T(||); /* { dg-warning "Omitting middle operand" } */
+  T(&&); /* { dg-warning "Omitting middle operand" } */
+  f2 (!x ? : 1);  /* { dg-warning "Omitting middle operand" } */
+  T2(<); /* { dg-bogus "Omitting middle operand" } */
+  T2(>); /* { dg-bogus "Omitting middle operand" } */
+  T2(==); /* { dg-bogus "Omitting middle operand" } */
+  T2(||); /* { dg-bogus "Omitting middle operand" } */
+  T2(&&); /* { dg-bogus "Omitting middle operand" } */
+  T(+); /* { dg-bogus "Omitting middle operand" } */
+  T(-); /* { dg-bogus "Omitting middle operand" } */
+  T(*); /* { dg-bogus "Omitting middle operand" } */
+  T(/); /* { dg-bogus "Omitting middle operand" } */
+  T(^); /* { dg-bogus "Omitting middle operand" } */
+}
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 143064)
+++ gcc/cp/parser.c	(working copy)
@@ -6336,11 +6336,16 @@
   cp_lexer_consume_token (parser->lexer);
   if (cp_parser_allow_gnu_extensions_p (parser)
       && cp_lexer_next_token_is (parser->lexer, CPP_COLON))
+  {
+    warn_for_omitted_condop (logical_or_expr);
     /* Implicit true clause.  */
     expr = NULL_TREE;
+  }
   else
+  {
     /* Parse the expression.  */
     expr = cp_parser_expression (parser, /*cast_p=*/false);
+  }
 
   /* The next token should be a `:'.  */
   cp_parser_require (parser, CPP_COLON, "%<:%>");
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 143064)
+++ gcc/common.opt	(working copy)
@@ -237,6 +237,10 @@
 Common RejectNegative Var(warn_coverage_mismatch) Warning
 Warn instead of error in case profiles in -fprofile-use do not match
 
+Womitted-conditional-op
+Common Var(warn_omitted_condop) Init(1)
+Warn for omitted middle operands in ?: with unexpected meaning
+
 aux-info
 Common Separate
 -aux-info <file>	Emit declaration information into <file>
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c	(revision 143064)
+++ gcc/c-common.c	(working copy)
@@ -8362,4 +8362,26 @@
     }
 }
 
+/* Warn for A ?: C expressions (with B omitted) where A is a computed 
+   boolean expression. */
+
+void
+warn_for_omitted_condop (tree cond) 
+{ 
+  enum tree_code code = TREE_CODE (cond);
+
+  if (!warn_omitted_condop)
+    return;
+  if (TREE_CODE_CLASS (code) == tcc_comparison ||
+      code == TRUTH_ANDIF_EXPR || 
+      code == TRUTH_ORIF_EXPR || 
+      code == TRUTH_ANDIF_EXPR || 
+      code == TRUTH_NOT_EXPR)
+    {
+      warning (OPT_Womitted_conditional_op, 
+	       "Omitting middle operand in ?: with computed "
+	       "boolean conditional has unexpected meaning");
+    }    
+} 
+
 #include "gt-c-common.h"
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h	(revision 143064)
+++ gcc/c-common.h	(working copy)
@@ -780,6 +780,7 @@
 /* This is misnamed, it actually performs end-of-compilation processing.  */
 extern void finish_file	(void);
 
+extern void warn_for_omitted_condop (tree cond);
 
 /* These macros provide convenient access to the various _STMT nodes.  */
 
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c	(revision 143064)
+++ gcc/c-parser.c	(working copy)
@@ -4448,6 +4448,7 @@
     {
       pedwarn (c_parser_peek_token (parser)->location, OPT_pedantic, 
 	       "ISO C forbids omitting the middle term of a ?: expression");
+      warn_for_omitted_condop (cond.value);
       /* Make sure first operand is calculated only once.  */
       exp1.value = save_expr (default_conversion (cond.value));
       cond.value = c_objc_common_truthvalue_conversion (cond_loc, exp1.value);