[gcc r14-5561] c++: Implement C++ DR 2406 - [[fallthrough]] attribute and iteration statements

[gcc r14-5561] c++: Implement C++ DR 2406 - [[fallthrough]] attribute and iteration statements

[Jakub Jelinek]

https://gcc.gnu.org/g:52eedfa00960f2d255ec542626e3531a65aa8bb8

commit r14-5561-g52eedfa00960f2d255ec542626e3531a65aa8bb8
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Nov 17 15:43:31 2023 +0100

    c++: Implement C++ DR 2406 - [[fallthrough]] attribute and iteration statements
    
    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.
    
    2023-11-17  Jakub Jelinek  <jakub@redhat.com>
    
            PR c++/107571
    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.
            * internal-fn.def (FALLTHROUGH): Mention in comment the special
            meaning of the TREE_NOTHROW/GF_CALL_NOTHROW flag on the calls.
    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.

Diff:
---
 gcc/c-family/c-gimplify.cc        | 21 ++++++++++
 gcc/gimple-walk.cc                | 12 +++++-
 gcc/gimplify.cc                   | 33 ++++++++++++----
 gcc/internal-fn.def               |  4 +-
 gcc/testsuite/g++.dg/DRs/dr2406.C | 82 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 143 insertions(+), 9 deletions(-)

diff --git a/gcc/c-family/c-gimplify.cc b/gcc/c-family/c-gimplify.cc
index 2f44e125f92..694ccf921a6 100644
--- a/gcc/c-family/c-gimplify.cc
+++ b/gcc/c-family/c-gimplify.cc
@@ -307,6 +307,27 @@ genericize_c_loop (tree *stmt_p, location_t start_locus, tree cond, tree body,
     }
 
   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)
     {
diff --git a/gcc/gimple-walk.cc b/gcc/gimple-walk.cc
index 9516d61ffa9..20df7e2d7e4 100644
--- a/gcc/gimple-walk.cc
+++ b/gcc/gimple-walk.cc
@@ -56,11 +56,21 @@ walk_gimple_seq_mod (gimple_seq *pseq, walk_stmt_fn callback_stmt,
 	  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)
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 77f07af6ec5..d52d71b9b6b 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -2791,17 +2791,33 @@ expand_FALLTHROUGH_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p,
       *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);
@@ -2851,6 +2867,7 @@ expand_FALLTHROUGH_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p,
 	}
       break;
     default:
+      static_cast<location_t *>(wi->info)[0] = UNKNOWN_LOCATION;
       break;
     }
   return NULL_TREE;
@@ -2862,14 +2879,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");
 }
 
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index 319e0ab26cd..3d5aca0cddf 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -533,7 +533,9 @@ DEF_INTERNAL_FN (ATOMIC_AND_FETCH_CMP_0, ECF_LEAF, NULL)
 DEF_INTERNAL_FN (ATOMIC_OR_FETCH_CMP_0, ECF_LEAF, NULL)
 DEF_INTERNAL_FN (ATOMIC_XOR_FETCH_CMP_0, ECF_LEAF, NULL)
 
-/* To implement [[fallthrough]].  */
+/* To implement [[fallthrough]].  If the TREE_NOTHROW or GF_CALL_NOTHROW flag
+   is set on the call (normally redundant with ECF_NOTHROW), it marks
+   [[fallthrough]] at the end of C++ loop body.  */
 DEF_INTERNAL_FN (FALLTHROUGH, ECF_LEAF | ECF_NOTHROW, NULL)
 
 /* To implement __builtin_launder.  */
diff --git a/gcc/testsuite/g++.dg/DRs/dr2406.C b/gcc/testsuite/g++.dg/DRs/dr2406.C
new file mode 100644
index 00000000000..856a1ea2f91
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2406.C
@@ -0,0 +1,82 @@
+// DR 2406 - [[fallthrough]] attribute and iteration statements
+// PR c++/107571
+// { 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;
+    }
+}