Hi!
The following patch implements
CWG 2406 - [[fallthrough]] attribute and iteration statements
The genericization of some loops leaves nothing at all or just a label
after a body of a loop, so if the loop is later followed by
case or default label in a switch, the fallthrough statement isn't
diagnosed.
The following patch implements it by marking the IFN_FALLTHROUGH call
in such a case, such that during gimplification it can be pedantically
diagnosed even if it is followed by case or default label or some normal
labels followed by case/default labels.
While looking into this, I've discovered other problems.
expand_FALLTHROUGH_r is removing the IFN_FALLTHROUGH calls from the IL,
but wasn't telling that to walk_gimple_stmt/walk_gimple_seq_mod, so
the callers would then skip the next statement after it, and it would
return non-NULL if the removed stmt was last in the sequence. This could
lead to wi->callback_result being set even if it didn't appear at the very
end of switch sequence.
The patch makes use of wi->removed_stmt such that the callers properly
know what happened, and use different way to handle the end of switch
sequence case.
That change discovered a bug in the gimple-walk handling of
wi->removed_stmt. If that flag is set, the callback is telling the callers
that the current statement has been removed and so the innermost
walk_gimple_seq_mod shouldn't gsi_next. The problem is that
wi->removed_stmt is only reset at the start of a walk_gimple_stmt, but that
can be too late for some cases. If we have two nested gimple sequences,
say GIMPLE_BIND as the last stmt of some gimple seq, we remove the last
statement inside of that GIMPLE_BIND, set wi->removed_stmt there, don't
do gsi_next correctly because already gsi_remove moved us to the next stmt,
there is no next stmt, so we return back to the caller, but wi->removed_stmt
is still set and so we don't do gsi_next even in the outer sequence, despite
the GIMPLE_BIND (etc.) not being removed. That means we walk the
GIMPLE_BIND with its whole sequence again.
The patch fixes that by resetting wi->removed_stmt after we've used that
flag in walk_gimple_seq_mod. Nothing really uses that flag after the
outermost walk_gimple_seq_mod, it is just a private notification that
the stmt callback has removed a stmt.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-08-25 Jakub Jelinek <ja...@redhat.com>
gcc/
* gimplify.cc (expand_FALLTHROUGH_r): Use wi->removed_stmt after
gsi_remove, change the way of passing fallthrough stmt at the end
of sequence to expand_FALLTHROUGH. Diagnose IFN_FALLTHROUGH
with GF_CALL_NOTHROW flag.
(expand_FALLTHROUGH): Change loc into array of 2 location_t elts,
don't test wi.callback_result, instead check whether first
elt is not UNKNOWN_LOCATION and in that case pedwarn with the
second location.
* gimple-walk.cc (walk_gimple_seq_mod): Clear wi->removed_stmt
after the flag has been used.
gcc/c-family/
* c-gimplify.cc (genericize_c_loop): For C++ mark IFN_FALLTHROUGH
call at the end of loop body as TREE_NOTHROW.
gcc/testsuite/
* g++.dg/DRs/dr2406.C: New test.
--- gcc/gimplify.cc.jj 2023-08-23 11:22:28.115592483 +0200
+++ gcc/gimplify.cc 2023-08-25 13:43:58.711847414 +0200
@@ -2588,17 +2588,33 @@ expand_FALLTHROUGH_r (gimple_stmt_iterat
*handled_ops_p = false;
break;
case GIMPLE_CALL:
+ static_cast<location_t *>(wi->info)[0] = UNKNOWN_LOCATION;
if (gimple_call_internal_p (stmt, IFN_FALLTHROUGH))
{
+ location_t loc = gimple_location (stmt);
gsi_remove (gsi_p, true);
+ wi->removed_stmt = true;
+
+ /* nothrow flag is added by genericize_c_loop to mark fallthrough
+ statement at the end of some loop's body. Those should be
+ always diagnosed, either because they indeed don't precede
+ a case label or default label, or because the next statement
+ is not within the same iteration statement. */
+ if ((stmt->subcode & GF_CALL_NOTHROW) != 0)
+ {
+ pedwarn (loc, 0, "attribute %<fallthrough%> not preceding "
+ "a case label or default label");
+ break;
+ }
+
if (gsi_end_p (*gsi_p))
{
- *static_cast<location_t *>(wi->info) = gimple_location (stmt);
- return integer_zero_node;
+ static_cast<location_t *>(wi->info)[0] = BUILTINS_LOCATION;
+ static_cast<location_t *>(wi->info)[1] = loc;
+ break;
}
bool found = false;
- location_t loc = gimple_location (stmt);
gimple_stmt_iterator gsi2 = *gsi_p;
stmt = gsi_stmt (gsi2);
@@ -2648,6 +2664,7 @@ expand_FALLTHROUGH_r (gimple_stmt_iterat
}
break;
default:
+ static_cast<location_t *>(wi->info)[0] = UNKNOWN_LOCATION;
break;
}
return NULL_TREE;
@@ -2659,14 +2676,16 @@ static void
expand_FALLTHROUGH (gimple_seq *seq_p)
{
struct walk_stmt_info wi;
- location_t loc;
+ location_t loc[2];
memset (&wi, 0, sizeof (wi));
- wi.info = (void *) &loc;
+ loc[0] = UNKNOWN_LOCATION;
+ loc[1] = UNKNOWN_LOCATION;
+ wi.info = (void *) &loc[0];
walk_gimple_seq_mod (seq_p, expand_FALLTHROUGH_r, NULL, &wi);
- if (wi.callback_result == integer_zero_node)
+ if (loc[0] != UNKNOWN_LOCATION)
/* We've found [[fallthrough]]; at the end of a switch, which the C++
standard says is ill-formed; see [dcl.attr.fallthrough]. */
- pedwarn (loc, 0, "attribute %<fallthrough%> not preceding "
+ pedwarn (loc[1], 0, "attribute %<fallthrough%> not preceding "
"a case label or default label");
}
--- gcc/gimple-walk.cc.jj 2023-01-02 09:32:28.298199849 +0100
+++ gcc/gimple-walk.cc 2023-08-25 14:21:10.264376130 +0200
@@ -56,11 +56,21 @@ walk_gimple_seq_mod (gimple_seq *pseq, w
gcc_assert (wi);
wi->callback_result = ret;
- return wi->removed_stmt ? NULL : gsi_stmt (gsi);
+ gimple *g;
+ if (!wi->removed_stmt)
+ g = gsi_stmt (gsi);
+ else
+ {
+ g = NULL;
+ wi->removed_stmt = false;
+ }
+ return g;
}
if (!wi->removed_stmt)
gsi_next (&gsi);
+ else
+ wi->removed_stmt = false;
}
if (wi)
--- gcc/c-family/c-gimplify.cc.jj 2023-07-11 13:40:37.594467535 +0200
+++ gcc/c-family/c-gimplify.cc 2023-08-25 12:38:02.406574469 +0200
@@ -307,6 +307,27 @@ genericize_c_loop (tree *stmt_p, locatio
}
append_to_statement_list (body, &stmt_list);
+ if (c_dialect_cxx ()
+ && stmt_list
+ && TREE_CODE (stmt_list) == STATEMENT_LIST)
+ {
+ tree_stmt_iterator tsi = tsi_last (stmt_list);
+ if (!tsi_end_p (tsi))
+ {
+ tree t = *tsi;
+ while (TREE_CODE (t) == CLEANUP_POINT_EXPR
+ || TREE_CODE (t) == EXPR_STMT
+ || CONVERT_EXPR_CODE_P (TREE_CODE (t)))
+ t = TREE_OPERAND (t, 0);
+ /* For C++, if iteration statement body ends with fallthrough
+ statement, mark it such that we diagnose it even if next
+ statement would be labeled statement with case/default label. */
+ if (TREE_CODE (t) == CALL_EXPR
+ && !CALL_EXPR_FN (t)
+ && CALL_EXPR_IFN (t) == IFN_FALLTHROUGH)
+ TREE_NOTHROW (t) = 1;
+ }
+ }
finish_bc_block (&stmt_list, bc_continue, clab);
if (incr)
{
--- gcc/testsuite/g++.dg/DRs/dr2406.C.jj 2023-08-25 14:16:53.095670934
+0200
+++ gcc/testsuite/g++.dg/DRs/dr2406.C 2023-08-25 14:16:04.732290555 +0200
@@ -0,0 +1,81 @@
+// DR 2406 - [[fallthrough]] attribute and iteration statements
+// { dg-do compile { target c++11 } }
+// { dg-options "-pedantic-errors -Wimplicit-fallthrough" }
+
+void bar ();
+void baz ();
+void qux ();
+
+void
+foo (int n)
+{
+ switch (n)
+ {
+ case 1:
+ case 2:
+ bar ();
+ [[fallthrough]];
+ case 3:
+ do
+ {
+ [[fallthrough]]; // { dg-error "attribute 'fallthrough' not
preceding a case label or default label" }
+ }
+ while (false);
+ case 6:
+ do
+ {
+ [[fallthrough]]; // { dg-error "attribute 'fallthrough' not
preceding a case label or default label" }
+ }
+ while (n--);
+ case 7:
+ while (false)
+ {
+ [[fallthrough]]; // { dg-error "attribute 'fallthrough' not
preceding a case label or default label" }
+ }
+ case 5:
+ baz (); // { dg-warning "this statement may fall
through" }
+ case 4: // { dg-message "here" }
+ qux ();
+ [[fallthrough]]; // { dg-error "attribute 'fallthrough' not
preceding a case label or default label" }
+ }
+}
+
+void
+corge (int n)
+{
+ switch (n)
+ {
+ case 1:
+ {
+ int i = 0;
+ do
+ {
+ [[fallthrough]]; // { dg-error "attribute 'fallthrough' not
preceding a case label or default label" }
+ }
+ while (false);
+ }
+ case 2:
+ bar ();
+ break;
+ default:
+ break;
+ }
+}
+
+void
+fred (int n)
+{
+ switch (n)
+ {
+ case 1:
+ {
+ int i = 0;
+ [[fallthrough]];
+ }
+ case 2:
+ bar ();
+ break;
+ default:
+ break;
+ }
+}
Jakub
