On Fri, Jun 10, 2016 at 02:52:08PM +0200, Jakub Jelinek wrote:
> Won't this just give up on say:
> void
> foo (int a, int b)
> {
> switch (a)
> {
> { int c; }
> { int d; }
> { int e; }
> b++;
> case 1:
> break;
> }
> }
>
> ? Such blocks can also be can be arbitrarily nested.
Oh yeah.
> Wouldn't it be better to just walk_gimple_seq with NULL callback_op
> and non-NULL callback_stmt that would stop on the first real stmt in there?
Ok, such as in the following?
> Also, the above loop looks confusing, I'd be expecting gimple_seq_first_stmt
> (seq) before testing it for GIMPLE_BIND or casting to gbind.
Well, if I give a GIMPLE_BIND to gimple_seq_first_stmt, it does nothing and
just returns it, I think.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-06-10 Marek Polacek <pola...@redhat.com>
PR middle-end/71476
* gimplify.c (maybe_warn_switch_unreachable): Factored out of
gimplify_switch_expr.
(warn_switch_unreachable_r): New function.
* c-c++-common/Wswitch-unreachable-4.c: New test.
* gcc.dg/Wswitch-unreachable-2.c: New test.
* g++.dg/tm/jump1.C: Move dg-warning.
diff --git gcc/gimplify.c gcc/gimplify.c
index 7c19cf3..ae8b4fc 100644
--- gcc/gimplify.c
+++ gcc/gimplify.c
@@ -1559,6 +1559,73 @@ gimplify_statement_list (tree *expr_p, gimple_seq *pre_p)
return GS_ALL_DONE;
}
+/* Callback for walk_gimple_seq. */
+
+static tree
+warn_switch_unreachable_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p,
+ struct walk_stmt_info *wi)
+{
+ gimple *stmt = gsi_stmt (*gsi_p);
+
+ *handled_ops_p = true;
+ switch (gimple_code (stmt))
+ {
+ case GIMPLE_TRY:
+ /* A compiler-generated cleanup or a user-written try block.
+ If it's empty, don't dive into it--that would result in
+ worse location info. */
+ if (gimple_try_eval (stmt) == NULL)
+ {
+ wi->info = stmt;
+ return integer_zero_node;
+ }
+ /* Fall through. */
+ case GIMPLE_BIND:
+ case GIMPLE_CATCH:
+ case GIMPLE_EH_FILTER:
+ case GIMPLE_TRANSACTION:
+ /* Walk the sub-statements. */
+ *handled_ops_p = false;
+ break;
+ default:
+ /* Save the first "real" statement (not a decl/lexical scope/...). */
+ wi->info = stmt;
+ return integer_zero_node;
+ }
+ return NULL_TREE;
+}
+
+/* Possibly warn about unreachable statements between switch's controlling
+ expression and the first case. SEQ is the body of a switch expression. */
+
+static void
+maybe_warn_switch_unreachable (gimple_seq seq)
+{
+ if (!warn_switch_unreachable
+ /* This warning doesn't play well with Fortran when optimizations
+ are on. */
+ || lang_GNU_Fortran ()
+ || seq == NULL)
+ return;
+
+ struct walk_stmt_info wi;
+ memset (&wi, 0, sizeof (wi));
+ walk_gimple_seq (seq, warn_switch_unreachable_r, NULL, &wi);
+ gimple *stmt = (gimple *) wi.info;
+
+ if (stmt && gimple_code (stmt) != GIMPLE_LABEL)
+ {
+ if (gimple_code (stmt) == GIMPLE_GOTO
+ && TREE_CODE (gimple_goto_dest (stmt)) == LABEL_DECL
+ && DECL_ARTIFICIAL (gimple_goto_dest (stmt)))
+ /* Don't warn for compiler-generated gotos. These occur
+ in Duff's devices, for example. */;
+ else
+ warning_at (gimple_location (stmt), OPT_Wswitch_unreachable,
+ "statement will never be executed");
+ }
+}
+
/* Gimplify a SWITCH_EXPR, and collect the vector of labels it can
branch to. */
@@ -1596,39 +1663,8 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
gimplify_stmt (&SWITCH_BODY (switch_expr), &switch_body_seq);
- /* Possibly warn about unreachable statements between switch's
- controlling expression and the first case. */
- if (warn_switch_unreachable
- /* This warning doesn't play well with Fortran when optimizations
- are on. */
- && !lang_GNU_Fortran ()
- && switch_body_seq != NULL)
- {
- gimple_seq seq = switch_body_seq;
- /* Look into the innermost lexical scope. */
- while (gimple_code (seq) == GIMPLE_BIND)
- seq = gimple_bind_body (as_a <gbind *> (seq));
- gimple *stmt = gimple_seq_first_stmt (seq);
- if (gimple_code (stmt) == GIMPLE_TRY)
- {
- /* A compiler-generated cleanup or a user-written try block.
- Try to get the first statement in its try-block, for better
- location. */
- if ((seq = gimple_try_eval (stmt)))
- stmt = gimple_seq_first_stmt (seq);
- }
- if (gimple_code (stmt) != GIMPLE_LABEL)
- {
- if (gimple_code (stmt) == GIMPLE_GOTO
- && TREE_CODE (gimple_goto_dest (stmt)) == LABEL_DECL
- && DECL_ARTIFICIAL (gimple_goto_dest (stmt)))
- /* Don't warn for compiler-generated gotos. These occur
- in Duff's devices, for example. */;
- else
- warning_at (gimple_location (stmt), OPT_Wswitch_unreachable,
- "statement will never be executed");
- }
- }
+ maybe_warn_switch_unreachable (switch_body_seq);
+
labels = gimplify_ctxp->case_labels;
gimplify_ctxp->case_labels = saved_labels;
diff --git gcc/testsuite/c-c++-common/Wswitch-unreachable-4.c
gcc/testsuite/c-c++-common/Wswitch-unreachable-4.c
index e69de29..e7378a7 100644
--- gcc/testsuite/c-c++-common/Wswitch-unreachable-4.c
+++ gcc/testsuite/c-c++-common/Wswitch-unreachable-4.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+
+void
+foo (int a, int b)
+{
+ switch (a)
+ {
+ { int c; }
+ { int d; }
+ { int e; }
+ b++; /* { dg-warning "statement will never be executed" } */
+ case 1:
+ break;
+ }
+
+ switch (a)
+ {
+ { int c; }
+ { int d = 1; } /* { dg-warning "statement will never be executed" } */
+ { int e; }
+ b++;
+ case 1:
+ break;
+ }
+}
diff --git gcc/testsuite/g++.dg/tm/jump1.C gcc/testsuite/g++.dg/tm/jump1.C
index e28282d..a27c201 100644
--- gcc/testsuite/g++.dg/tm/jump1.C
+++ gcc/testsuite/g++.dg/tm/jump1.C
@@ -14,8 +14,8 @@ void f()
switch (i)
{
- synchronized { // { dg-warning "statement will never be
executed" }
- ++i;
+ synchronized {
+ ++i; // { dg-warning "statement will never be
executed" }
case 42: // { dg-error "" }
++i;
}
diff --git gcc/testsuite/gcc.dg/Wswitch-unreachable-2.c
gcc/testsuite/gcc.dg/Wswitch-unreachable-2.c
index e69de29..343baea 100644
--- gcc/testsuite/gcc.dg/Wswitch-unreachable-2.c
+++ gcc/testsuite/gcc.dg/Wswitch-unreachable-2.c
@@ -0,0 +1,12 @@
+/* PR middle-end/71476 */
+/* { dg-do compile } */
+/* { dg-options "-Wswitch-unreachable" } */
+
+void
+foo (int a)
+{
+ switch (a)
+ {
+ void f (void) { }
+ }
+}
Marek
