[PATCH] Warn for dangerous use of omitted middle operand in ?: v4

Andi Kleen andi@firstfloor.org
Tue Jan 20 12:02:00 GMT 2009


On Tue, Jan 20, 2009 at 09:53:52AM +0100, Andreas Schwab wrote:
> Andi Kleen <andi@firstfloor.org> writes:
> 
> > +Also 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
> 
> s/computed boolean/boolean expression/.

Thanks. Fixed patch.


2009-01-18  Andi Kleen  <ak@linux.intel.com>

        * gcc.dg/warn-omitted-condop.c: Add
        * g++.dg/warn-omitted-condop.C: Add
        * cp/parser.c: (cp_parser_question_colon_clause): 
	Switch to use cp_lexer_peek_token.
        Call warn_for_omitted_condop. Call pedwarn for omitted
	middle operand.
        * 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.
	* doc/invoke.texi: Document omitted condop warning.


Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 143495)
+++ gcc/doc/invoke.texi	(working copy)
@@ -3040,6 +3040,12 @@
 @end group
 @end smallexample
 
+Also warn for dangerous uses of the 
+?: with omitted middle operand GNU extension. When the condition
+in the ?: operator is a boolean expression the omitted value will
+be always 1. Often the user expects it to be a value computed
+inside the conditional expression instead. 
+
 This warning is enabled by @option{-Wall}.
 
 @item -Wsequence-point
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 "-Wparentheses" } */
+
+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 "omitted middle operand" } */
+  T(>); /* { dg-warning "omitted middle operand" } */
+  T(<=); /* { dg-warning "omitted middle operand" } */
+  T(>=); /* { dg-warning "omitted middle operand" } */
+  T(==); /* { dg-warning "omitted middle operand" } */
+  T(!=); /* { dg-warning "omitted middle operand" } */
+  T(||); /* { dg-warning "omitted middle operand" } */
+  T(&&); /* { dg-warning "omitted middle operand" } */
+  f2 (!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" } */
+  T2(||); /* { dg-bogus "omitted middle operand" } */
+  T2(&&); /* { dg-bogus "omitted middle operand" } */
+  T(+); /* { dg-bogus "omitted middle operand" } */
+  T(-); /* { dg-bogus "omitted middle operand" } */
+  T(*); /* { dg-bogus "omitted middle operand" } */
+  T(/); /* { dg-bogus "omitted middle operand" } */
+  T(^); /* { dg-bogus "omitted 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 "-Wparentheses" } */
+
+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 "omitted middle operand" } */
+  T(>); /* { dg-warning "omitted middle operand" } */
+  T(<=); /* { dg-warning "omitted middle operand" } */
+  T(>=); /* { dg-warning "omitted middle operand" } */
+  T(==); /* { dg-warning "omitted middle operand" } */
+  T(!=); /* { dg-warning "omitted middle operand" } */
+  T(||); /* { dg-warning "omitted middle operand" } */
+  T(&&); /* { dg-warning "omitted middle operand" } */
+  f2 (!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" } */
+  T2(||); /* { dg-bogus "omitted middle operand" } */
+  T2(&&); /* { dg-bogus "omitted middle operand" } */
+  T(+); /* { dg-bogus "omitted middle operand" } */
+  T(-); /* { dg-bogus "omitted middle operand" } */
+  T(*); /* { dg-bogus "omitted middle operand" } */
+  T(/); /* { dg-bogus "omitted middle operand" } */
+  T(^); /* { dg-bogus "omitted middle operand" } */
+}
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 143495)
+++ gcc/cp/parser.c	(working copy)
@@ -6341,16 +6341,25 @@
 {
   tree expr;
   tree assignment_expr;
+  struct cp_token *token;
 
   /* Consume the `?' token.  */
   cp_lexer_consume_token (parser->lexer);
-  if (cp_parser_allow_gnu_extensions_p (parser)
-      && cp_lexer_next_token_is (parser->lexer, CPP_COLON))
+
+  token = cp_lexer_peek_token (parser->lexer);
+  if (cp_parser_allow_gnu_extensions_p (parser) && token->type == CPP_COLON)
+  {
+    pedwarn (token->location, OPT_pedantic, 
+             "ISO C++ does not allow ?: with omitted middle operand");
+    warn_for_omitted_condop (token->location, logical_or_expr);
     /* Implicit true clause.  */
     expr = NULL_TREE;
+  }
   else
+  {
     /* Parse the expression.  */
     expr = cp_parser_expression (parser, /*cast_p=*/false, NULL);
+  }
 
   /* The next token should be a `:'.  */
   cp_parser_require (parser, CPP_COLON, "%<:%>");
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c	(revision 143495)
+++ gcc/c-common.c	(working copy)
@@ -8363,4 +8363,26 @@
     }
 }
 
+/* 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) 
+{ 
+  enum tree_code code = TREE_CODE (cond);
+
+  if (!warn_parentheses)
+    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_at (location, OPT_Womitted_conditional_op, 
+		"the omitted middle operand in ?: will always be %<true%>, "
+		"suggest explicit middle operand");
+    }    
+} 
+
 #include "gt-c-common.h"
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h	(revision 143495)
+++ 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 (location_t, tree);
 
 /* These macros provide convenient access to the various _STMT nodes.  */
 
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c	(revision 143495)
+++ gcc/c-parser.c	(working copy)
@@ -4432,7 +4432,7 @@
 c_parser_conditional_expression (c_parser *parser, struct c_expr *after)
 {
   struct c_expr cond, exp1, exp2, ret;
-  location_t cond_loc;
+  location_t cond_loc, middle_loc;
 
   gcc_assert (!after || c_dialect_objc ());
 
@@ -4446,8 +4446,10 @@
   c_parser_consume_token (parser);
   if (c_parser_next_token_is (parser, CPP_COLON))
     {
-      pedwarn (c_parser_peek_token (parser)->location, OPT_pedantic, 
+      middle_loc = c_parser_peek_token (parser)->location;
+      pedwarn (middle_loc, OPT_pedantic, 
 	       "ISO C forbids omitting the middle term of a ?: expression");
+      warn_for_omitted_condop (middle_loc, 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);
 

-- 
ak@linux.intel.com -- Speaking for myself only.



More information about the Gcc-patches mailing list