This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Warn for dangerous use of omitted middle operand in ?:
- From: Andi Kleen <andi at firstfloor dot org>
- To: gcc-patches at gcc dot gnu dot org
- Date: Mon, 31 May 2010 10:19:17 +0200
- Subject: [PATCH] Warn for dangerous use of omitted middle operand in ?:
Here's an old patch which has been in principle reviewed approved
a long time ago (http://gcc.gnu.org/ml/gcc-patches/2009-01/msg01350.html),
but never made it in. I did a bootstrap and test on a recent trunk.
This adds a warning for dangerous uses of ?: with omitted
middle operand. The warning is part of -Wparentheses.
I also a minor bug in the existing code -- the C++
parser didn't have a pedwarn when using the omitted middle operand.
Can someone please commit it?
Thanks,
-Andi
2009-05-22 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.
diff --git a/gcc/c-common.c b/gcc/c-common.c
index 115e302..90eac36 100644
--- a/gcc/c-common.c
+++ b/gcc/c-common.c
@@ -8588,6 +8588,30 @@ fold_offsetof (tree expr, tree stop_ref)
return convert (size_type_node, fold_offsetof_1 (expr, stop_ref));
}
+/* 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_AND_EXPR
+ || code == TRUTH_XOR_EXPR
+ || code == TRUTH_OR_EXPR
+ || code == TRUTH_NOT_EXPR)
+ {
+ warning_at (location, OPT_Wparentheses,
+ "the omitted middle operand in ?: will always be %<true%>, "
+ "suggest explicit middle operand");
+ }
+}
+
/* Print an error message for an invalid lvalue. USE says
how the lvalue is being used and so selects the error message. */
diff --git a/gcc/c-common.h b/gcc/c-common.h
index f0541e9..928242b 100644
--- a/gcc/c-common.h
+++ b/gcc/c-common.h
@@ -918,6 +918,7 @@ extern void c_parse_file (void);
/* 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. */
diff --git a/gcc/c-parser.c b/gcc/c-parser.c
index b30b063..42f816b 100644
--- a/gcc/c-parser.c
+++ b/gcc/c-parser.c
@@ -4795,7 +4795,7 @@ static struct c_expr
c_parser_conditional_expression (c_parser *parser, struct c_expr *after)
{
struct c_expr cond, exp1, exp2, ret;
- location_t cond_loc, colon_loc;
+ location_t cond_loc, colon_loc, middle_loc;
gcc_assert (!after || c_dialect_objc ());
@@ -4809,8 +4809,11 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after)
if (c_parser_next_token_is (parser, CPP_COLON))
{
tree eptype = NULL_TREE;
- 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);
if (TREE_CODE (cond.value) == EXCESS_PRECISION_EXPR)
{
eptype = TREE_TYPE (cond.value);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 4a50b55..6f448b9 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6770,15 +6770,20 @@ cp_parser_question_colon_clause (cp_parser* parser, tree logical_or_expr)
{
tree expr;
tree assignment_expr;
+ struct cp_token *token;
/* Consume the `?' token. */
cp_lexer_consume_token (parser->lexer);
+ token = cp_lexer_peek_token (parser->lexer);
if (cp_parser_allow_gnu_extensions_p (parser)
- && cp_lexer_next_token_is (parser->lexer, CPP_COLON))
+ && token->type == CPP_COLON)
{
+ pedwarn (token->location, OPT_pedantic,
+ "ISO C++ does not allow ?: with omitted middle operand");
/* Implicit true clause. */
expr = NULL_TREE;
c_inhibit_evaluation_warnings += logical_or_expr == truthvalue_true_node;
+ warn_for_omitted_condop (token->location, logical_or_expr);
}
else
{
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 17b1b3f..1a078ae 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3258,6 +3258,12 @@ look like this:
@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
diff --git a/gcc/testsuite/g++.dg/warn/warn-omitted-condop.C b/gcc/testsuite/g++.dg/warn/warn-omitted-condop.C
new file mode 100644
index 0000000..9fda387
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/warn-omitted-condop.C
@@ -0,0 +1,58 @@
+/* { 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" } */
+}
+/* { 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" } */
+}
diff --git a/gcc/testsuite/gcc.dg/warn-omitted-condop.c b/gcc/testsuite/gcc.dg/warn-omitted-condop.c
new file mode 100644
index 0000000..9fda387
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/warn-omitted-condop.c
@@ -0,0 +1,58 @@
+/* { 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" } */
+}
+/* { 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" } */
+}
--
ak@linux.intel.com -- Speaking for myself only.