Re: [C++ PATCH] Avoid -Wreturn-type warnings if a switch has default label, no breaks inside of it, but is followed by a break (PR sanitizer/81275)

[Jakub Jelinek <jakub@redhat.com>]

On Sat, Nov 25, 2017 at 10:01:22AM +0100, Jakub Jelinek wrote:
> Actually, thinking about it some more, maybe it would be more efficient
> to gather this information during construction of the SWITCH_STMT in some
> new flag on the tree, so cxx_block_may_fallthru would just:

Here it is implemented, bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?

2017-11-25  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/81275
	* cp-tree.h (SWITCH_STMT_CANNOT_FALLTHRU_P): Define.
	(note_break_stmt, note_iteration_stmt_body_start,
	note_iteration_stmt_body_end): Declare.
	* decl.c (struct cp_switch): Add has_default_p, break_stmt_seen_p
	and in_loop_body_p fields. 
	(push_switch): Clear them.
	(pop_switch): Set SWITCH_STMT_CANNOT_FALLTHRU_P if has_default_p
	and !break_stmt_seen_p.  Assert in_loop_body_p is false.
	(note_break_stmt, note_iteration_stmt_body_start,
	note_iteration_stmt_body_end): New functions.
	(finish_case_label): Set has_default_p when both low and high
	are NULL_TREE.
	* parser.c (cp_parser_iteration_statement): Use
	note_iteration_stmt_body_start and note_iteration_stmt_body_end
	around parsing iteration body.
	* pt.c (tsubst_expr): Likewise.
	* cp-objcp-common.c (cxx_block_may_fallthru): Return false for
	SWITCH_STMT which contains no BREAK_STMTs, contains a default:
	CASE_LABEL_EXPR and where SWITCH_STMT_BODY isn't empty and
	can't fallthru.
	* semantics.c (finish_break_stmt): Call note_break_stmt.

	* g++.dg/warn/pr81275-1.C: New test.
	* g++.dg/warn/pr81275-2.C: New test.
	* g++.dg/warn/pr81275-3.C: New test.

--- gcc/cp/cp-tree.h.jj	2017-11-17 08:40:32.000000000 +0100
+++ gcc/cp/cp-tree.h	2017-11-25 21:25:48.277897180 +0100
@@ -364,6 +364,7 @@ extern GTY(()) tree cp_global_trees[CPTI
       IF_STMT_CONSTEXPR_P (IF_STMT)
       TEMPLATE_TYPE_PARM_FOR_CLASS (TEMPLATE_TYPE_PARM)
       DECL_NAMESPACE_INLINE_P (in NAMESPACE_DECL)
+      SWITCH_STMT_CANNOT_FALLTHRU_P (in SWITCH_STMT)
    1: IDENTIFIER_KIND_BIT_1 (in IDENTIFIER_NODE)
       TI_PENDING_TEMPLATE_FLAG.
       TEMPLATE_PARMS_FOR_INLINE.
@@ -4840,6 +4841,12 @@ more_aggr_init_expr_args_p (const aggr_i
 #define SWITCH_STMT_BODY(NODE)	TREE_OPERAND (SWITCH_STMT_CHECK (NODE), 1)
 #define SWITCH_STMT_TYPE(NODE)	TREE_OPERAND (SWITCH_STMT_CHECK (NODE), 2)
 #define SWITCH_STMT_SCOPE(NODE)	TREE_OPERAND (SWITCH_STMT_CHECK (NODE), 3)
+/* Set if the body of a switch stmt contains a default: case label
+   and does not contain any break; stmts, thus if SWITCH_STMT_BODY
+   is not empty and doesn't fallthru, then the whole SWITCH_STMT
+   can't fallthru either.  */
+#define SWITCH_STMT_CANNOT_FALLTHRU_P(NODE) \
+  TREE_LANG_FLAG_0 (SWITCH_STMT_CHECK (NODE))
 
 /* STMT_EXPR accessor.  */
 #define STMT_EXPR_STMT(NODE)	TREE_OPERAND (STMT_EXPR_CHECK (NODE), 0)
@@ -6102,6 +6109,9 @@ enum cp_tree_node_structure_enum cp_tree
 extern void finish_scope			(void);
 extern void push_switch				(tree);
 extern void pop_switch				(void);
+extern void note_break_stmt			(void);
+extern bool note_iteration_stmt_body_start	(void);
+extern void note_iteration_stmt_body_end	(bool);
 extern tree make_lambda_name			(void);
 extern int decls_match				(tree, tree);
 extern bool maybe_version_functions		(tree, tree);
--- gcc/cp/decl.c.jj	2017-11-22 21:37:46.000000000 +0100
+++ gcc/cp/decl.c	2017-11-25 21:25:26.306162437 +0100
@@ -3427,6 +3427,13 @@ struct cp_switch
   /* Remember whether there was a case value that is outside the
      range of the original type of the controlling expression.  */
   bool outside_range_p;
+  /* Remember whether a default: case label has been seen.  */
+  bool has_default_p;
+  /* Remember whether a BREAK_STMT has been seen in this SWITCH_STMT.  */
+  bool break_stmt_seen_p;
+  /* Set if inside of {FOR,DO,WHILE}_BODY nested inside of a switch,
+     where BREAK_STMT doesn't belong to the SWITCH_STMT.  */
+  bool in_loop_body_p;
 };
 
 /* A stack of the currently active switch statements.  The innermost
@@ -3449,6 +3456,9 @@ push_switch (tree switch_stmt)
   p->switch_stmt = switch_stmt;
   p->cases = splay_tree_new (case_compare, NULL, NULL);
   p->outside_range_p = false;
+  p->has_default_p = false;
+  p->break_stmt_seen_p = false;
+  p->in_loop_body_p = false;
   switch_stack = p;
 }
 
@@ -3469,11 +3479,50 @@ pop_switch (void)
 			  SWITCH_STMT_COND (cs->switch_stmt),
 			  bool_cond_p, cs->outside_range_p);
 
+  /* For the benefit of block_may_fallthru remember if the switch body
+     contains a default: case label and no break; stmts.  */
+  if (cs->has_default_p && !cs->break_stmt_seen_p)
+    SWITCH_STMT_CANNOT_FALLTHRU_P (cs->switch_stmt) = 1;
+  gcc_assert (!cs->in_loop_body_p);
   splay_tree_delete (cs->cases);
   switch_stack = switch_stack->next;
   free (cs);
 }
 
+/* Note that a BREAK_STMT is about to be added.  If it is inside of
+   a SWITCH_STMT and not inside of a loop body inside of it, note
+   in switch_stack we've seen a BREAK_STMT.  */
+
+void
+note_break_stmt (void)
+{
+  if (switch_stack && !switch_stack->in_loop_body_p)
+    switch_stack->break_stmt_seen_p = true;
+}
+
+/* Note the start of processing of an iteration statement's body.
+   The note_break_stmt function will do nothing while processing it.
+   Return a flag that should be passed to note_iteration_stmt_body_end.  */
+
+bool
+note_iteration_stmt_body_start (void)
+{
+  if (!switch_stack)
+    return false;
+  bool ret = switch_stack->in_loop_body_p;
+  switch_stack->in_loop_body_p = true;
+  return ret;
+}
+
+/* Note the end of processing of an iteration statement's body.  */
+
+void
+note_iteration_stmt_body_end (bool prev)
+{
+  if (switch_stack)
+    switch_stack->in_loop_body_p = prev;
+}
+
 /* Convert a case constant VALUE in a switch to the type TYPE of the switch
    condition.  Note that if TYPE and VALUE are already integral we don't
    really do the conversion because the language-independent
@@ -3508,6 +3557,9 @@ finish_case_label (location_t loc, tree
   cp_binding_level *p;
   tree type;
 
+  if (low_value == NULL_TREE && high_value == NULL_TREE)
+    switch_stack->has_default_p = true;
+
   if (processing_template_decl)
     {
       tree label;
--- gcc/cp/parser.c.jj	2017-11-23 21:17:52.000000000 +0100
+++ gcc/cp/parser.c	2017-11-25 21:29:52.644946109 +0100
@@ -12256,7 +12256,9 @@ cp_parser_iteration_statement (cp_parser
 	parens.require_close (parser);
 	/* Parse the dependent statement.  */
 	parser->in_statement = IN_ITERATION_STMT;
+	bool prev = note_iteration_stmt_body_start ();
 	cp_parser_already_scoped_statement (parser, if_p, guard_tinfo);
+	note_iteration_stmt_body_end (prev);
 	parser->in_statement = in_statement;
 	/* We're done with the while-statement.  */
 	finish_while_stmt (statement);
@@ -12271,7 +12273,9 @@ cp_parser_iteration_statement (cp_parser
 	statement = begin_do_stmt ();
 	/* Parse the body of the do-statement.  */
 	parser->in_statement = IN_ITERATION_STMT;
+	bool prev = note_iteration_stmt_body_start ();
 	cp_parser_implicitly_scoped_statement (parser, NULL, guard_tinfo);
+	note_iteration_stmt_body_end (prev);
 	parser->in_statement = in_statement;
 	finish_do_body (statement);
 	/* Look for the `while' keyword.  */
@@ -12303,7 +12307,9 @@ cp_parser_iteration_statement (cp_parser
 
 	/* Parse the body of the for-statement.  */
 	parser->in_statement = IN_ITERATION_STMT;
+	bool prev = note_iteration_stmt_body_start ();
 	cp_parser_already_scoped_statement (parser, if_p, guard_tinfo);
+	note_iteration_stmt_body_end (prev);
 	parser->in_statement = in_statement;
 
 	/* We're done with the for-statement.  */
--- gcc/cp/cp-objcp-common.c.jj	2017-11-24 22:38:51.214379987 +0100
+++ gcc/cp/cp-objcp-common.c	2017-11-25 20:43:09.221755658 +0100
@@ -349,6 +349,11 @@ cxx_block_may_fallthru (const_tree stmt)
     case THROW_EXPR:
       return false;
 
+    case SWITCH_STMT:
+      return (!SWITCH_STMT_CANNOT_FALLTHRU_P (stmt)
+	      || SWITCH_STMT_BODY (stmt) == NULL_TREE
+	      || block_may_fallthru (SWITCH_STMT_BODY (stmt)));
+
     default:
       return true;
     }
--- gcc/cp/semantics.c.jj	2017-11-23 21:13:28.000000000 +0100
+++ gcc/cp/semantics.c	2017-11-25 21:07:22.443247156 +0100
@@ -1118,6 +1118,7 @@ finish_break_stmt (void)
      understand.  */
   if (!block_may_fallthru (cur_stmt_list))
     return void_node;
+  note_break_stmt ();
   return add_stmt (build_stmt (input_location, BREAK_STMT));
 }
 
--- gcc/cp/pt.c.jj	2017-11-23 21:13:28.000000000 +0100
+++ gcc/cp/pt.c	2017-11-25 21:32:26.956082032 +0100
@@ -16122,7 +16122,11 @@ tsubst_expr (tree t, tree args, tsubst_f
       finish_for_cond (tmp, stmt, false);
       tmp = RECUR (FOR_EXPR (t));
       finish_for_expr (tmp, stmt);
-      RECUR (FOR_BODY (t));
+      {
+	bool prev = note_iteration_stmt_body_start ();
+	RECUR (FOR_BODY (t));
+	note_iteration_stmt_body_end (prev);
+      }
       finish_for_stmt (stmt);
       break;
 
@@ -16146,7 +16150,9 @@ tsubst_expr (tree t, tree args, tsubst_f
 	else
 	  stmt = cp_convert_range_for (stmt, decl, expr, NULL_TREE, 0,
 				       RANGE_FOR_IVDEP (t));
+	bool prev = note_iteration_stmt_body_start ();
         RECUR (RANGE_FOR_BODY (t));
+	note_iteration_stmt_body_end (prev);
         finish_for_stmt (stmt);
       }
       break;
@@ -16155,13 +16161,21 @@ tsubst_expr (tree t, tree args, tsubst_f
       stmt = begin_while_stmt ();
       tmp = RECUR (WHILE_COND (t));
       finish_while_stmt_cond (tmp, stmt, false);
-      RECUR (WHILE_BODY (t));
+      {
+	bool prev = note_iteration_stmt_body_start ();
+	RECUR (WHILE_BODY (t));
+	note_iteration_stmt_body_end (prev);
+      }
       finish_while_stmt (stmt);
       break;
 
     case DO_STMT:
       stmt = begin_do_stmt ();
-      RECUR (DO_BODY (t));
+      {
+	bool prev = note_iteration_stmt_body_start ();
+	RECUR (DO_BODY (t));
+	note_iteration_stmt_body_end (prev);
+      }
       finish_do_body (stmt);
       tmp = RECUR (DO_COND (t));
       finish_do_stmt (tmp, stmt, false);
--- gcc/testsuite/g++.dg/warn/pr81275-1.C.jj	2017-11-25 20:40:07.679937379 +0100
+++ gcc/testsuite/g++.dg/warn/pr81275-1.C	2017-11-25 21:44:13.303549363 +0100
@@ -0,0 +1,165 @@
+// PR sanitizer/81875
+// { dg-do compile }
+// { dg-options "-Wreturn-type" }
+
+struct C { C (); ~C (); };
+
+int
+f1 (int a, int b)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  return 7;
+	case 24:
+	  return 19;
+	default:
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-bogus "control reaches end of non-void function" }
+
+int
+f2 (int a, int b, int c, int d)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  while (c >= 10)
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  return 7;
+	case 29:
+	  switch (d)
+	    {
+	    case 35:
+	      break;
+	    default:
+	      return 9;
+	    }
+	  return 8;
+	case 24:
+	  do
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  while (c >= 10);
+	  return 19;
+	default:
+	  for (int e = 0; e < c; ++e)
+	    if (e == d)
+	      break;
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-bogus "control reaches end of non-void function" }
+
+template <int N>
+int
+f3 (int a, int b)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  return 7;
+	case 24:
+	  return 19;
+	default:
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-bogus "control reaches end of non-void function" }
+
+template <int N>
+int
+f4 (int a, int b, int c, int d)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  while (c >= 10)
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  return 7;
+	case 29:
+	  switch (d)
+	    {
+	    case 35:
+	      break;
+	    default:
+	      return 9;
+	    }
+	  return 8;
+	case 24:
+	  do
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  while (c >= 10);
+	  return 19;
+	default:
+	  for (int e = 0; e < c; ++e)
+	    if (e == d)
+	      break;
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-bogus "control reaches end of non-void function" }
+
+int
+f5 (int a, int b)
+{
+  return f3 <0> (a, b);
+}
+
+int
+f6 (int a, int b, int c, int d)
+{
+  return f4 <2> (a, b, c, d);
+}
--- gcc/testsuite/g++.dg/warn/pr81275-2.C.jj	2017-11-25 21:37:40.291296947 +0100
+++ gcc/testsuite/g++.dg/warn/pr81275-2.C	2017-11-25 21:44:21.796446769 +0100
@@ -0,0 +1,165 @@
+// PR sanitizer/81875
+// { dg-do compile }
+// { dg-options "-Wreturn-type" }
+
+struct C { C (); ~C (); };
+
+int
+f1 (int a, int b)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  return 7;
+	case 24:
+	  return 19;
+	case 25:
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-warning "control reaches end of non-void function" }
+
+int
+f2 (int a, int b, int c, int d)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  while (c >= 10)
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  return 7;
+	case 29:
+	  switch (d)
+	    {
+	    case 35:
+	      break;
+	    default:
+	      return 9;
+	    }
+	  return 8;
+	case 24:
+	  do
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  while (c >= 10);
+	  return 19;
+	case 25:
+	  for (int e = 0; e < c; ++e)
+	    if (e == d)
+	      break;
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-warning "control reaches end of non-void function" }
+
+template <int N>
+int
+f3 (int a, int b)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  return 7;
+	case 24:
+	  return 19;
+	case 25:
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-warning "control reaches end of non-void function" }
+
+template <int N>
+int
+f4 (int a, int b, int c, int d)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  while (c >= 10)
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  return 7;
+	case 29:
+	  switch (d)
+	    {
+	    case 35:
+	      break;
+	    default:
+	      return 9;
+	    }
+	  return 8;
+	case 24:
+	  do
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  while (c >= 10);
+	  return 19;
+	case 25:
+	  for (int e = 0; e < c; ++e)
+	    if (e == d)
+	      break;
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-warning "control reaches end of non-void function" }
+
+int
+f5 (int a, int b)
+{
+  return f3 <0> (a, b);
+}
+
+int
+f6 (int a, int b, int c, int d)
+{
+  return f4 <2> (a, b, c, d);
+}
--- gcc/testsuite/g++.dg/warn/pr81275-3.C.jj	2017-11-25 21:45:03.282945613 +0100
+++ gcc/testsuite/g++.dg/warn/pr81275-3.C	2017-11-25 21:47:08.797429400 +0100
@@ -0,0 +1,173 @@
+// PR sanitizer/81875
+// { dg-do compile }
+// { dg-options "-Wreturn-type" }
+
+struct C { C (); ~C (); };
+
+int
+f1 (int a, int b, int c)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  return 7;
+	case 24:
+	  if (c == 5)
+	    break;
+	  return 19;
+	default:
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-warning "control reaches end of non-void function" }
+
+int
+f2 (int a, int b, int c, int d)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  while (c >= 10)
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  return 7;
+	case 29:
+	  switch (d)
+	    {
+	    case 35:
+	      break;
+	    default:
+	      return 9;
+	    }
+	  if (c == d + 20)
+	    break;
+	  return 8;
+	case 24:
+	  do
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  while (c >= 10);
+	  return 19;
+	default:
+	  for (int e = 0; e < c; ++e)
+	    if (e == d)
+	      break;
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-warning "control reaches end of non-void function" }
+
+template <int N>
+int
+f3 (int a, int b, int c)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  return 7;
+	case 24:
+	  if (c == 5)
+	    break;
+	  return 19;
+	default:
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-warning "control reaches end of non-void function" }
+
+template <int N>
+int
+f4 (int a, int b, int c, int d)
+{
+  C f;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  while (c >= 10)
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  return 7;
+	case 29:
+	  switch (d)
+	    {
+	    case 35:
+	      break;
+	    default:
+	      return 9;
+	    }
+	  if (c == d + 20)
+	    break;
+	  return 8;
+	case 24:
+	  do
+	    {
+	      if (c == d)
+		break;
+	      c--;
+	    }
+	  while (c >= 10);
+	  return 19;
+	default:
+	  for (int e = 0; e < c; ++e)
+	    if (e == d)
+	      break;
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-warning "control reaches end of non-void function" }
+
+int
+f5 (int a, int b, int c)
+{
+  return f3 <0> (a, b, c);
+}
+
+int
+f6 (int a, int b, int c, int d)
+{
+  return f4 <2> (a, b, c, d);
+}


	Jakub