Re: [PATCH] Avoid some -Wunreachable-code-ctrl

[Jason Merrill <jason@redhat.com>]

On 11/29/21 10:03, Richard Biener via Gcc-patches wrote:
> This cleans up unreachable code diagnosed by -Wunreachable-code-ctrl.
> It largely follows the previous series but discovers a few extra
> cases, namely dead code after break or continue or loops without
> exits.
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> Richard.
> 
> 2021-11-29  Richard Biener  <rguenther@suse.de>
> 
> gcc/c/
> 	* gimple-parser.c (c_parser_gimple_postfix_expression):
> 	avoid unreachable code after break.
> 
> gcc/
> 	* cfgrtl.c (skip_insns_after_block): Refactor code to
> 	be more easily readable.
> 	* expr.c (op_by_pieces_d::run): Remove unreachable
> 	assert.
> 	* sched-deps.c (sched_analyze): Remove unreachable
> 	gcc_unreachable.
> 	* sel-sched-ir.c (in_same_ebb_p): Likewise.
> 	* tree-ssa-alias.c (nonoverlapping_refs_since_match_p):
> 	Remove unreachable code.
> 	* tree-vect-slp.c (vectorize_slp_instance_root_stmt):
> 	Refactor to avoid unreachable loop iteration.
> 	* tree.c (walk_tree_1): Remove unreachable break.
> 	* vec-perm-indices.c (vec_perm_indices::series_p): Remove
> 	unreachable return.
> 
> gcc/cp/
> 	* parser.c (cp_parser_postfix_expression): Remove
> 	unreachable code.
> 	* pt.c (tsubst_expr): Remove unreachable breaks.
> 
> gcc/fortran/
> 	* frontend-passes.c (gfc_expr_walker): Remove unreachable
> 	break.
> 	* scanner.c (skip_fixed_comments): Remove unreachable
> 	gcc_unreachable.
> 	* trans-expr.c (gfc_expr_is_variable): Refactor to make
> 	control flow more obvious.
> ---
>   gcc/c/gimple-parser.c         |  8 +-------
>   gcc/cfgrtl.c                  | 10 ++--------
>   gcc/cp/parser.c               |  4 ----
>   gcc/cp/pt.c                   |  2 --
>   gcc/expr.c                    |  3 ---
>   gcc/fortran/frontend-passes.c |  1 -
>   gcc/fortran/scanner.c         |  1 -
>   gcc/fortran/trans-expr.c      | 11 +++--------
>   gcc/sched-deps.c              |  2 --
>   gcc/sel-sched-ir.c            |  3 ---
>   gcc/tree-ssa-alias.c          |  3 ---
>   gcc/tree-vect-slp.c           | 22 ++++++++--------------
>   gcc/tree.c                    |  2 --
>   gcc/vec-perm-indices.c        |  1 -
>   14 files changed, 14 insertions(+), 59 deletions(-)
> 
> diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c
> index 32f22dbb8a7..f594a8ccb31 100644
> --- a/gcc/c/gimple-parser.c
> +++ b/gcc/c/gimple-parser.c
> @@ -1698,13 +1698,7 @@ c_parser_gimple_postfix_expression (gimple_parser &parser)
>   	    }
>   	  break;
>   	}
> -      else
> -	{
> -	  c_parser_error (parser, "expected expression");
> -	  expr.set_error ();
> -	  break;
> -	}
> -      break;
> +      /* Fallthru.  */
>       default:
>         c_parser_error (parser, "expected expression");
>         expr.set_error ();
> diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
> index 3744adcc2ba..287a3db643a 100644
> --- a/gcc/cfgrtl.c
> +++ b/gcc/cfgrtl.c
> @@ -3539,14 +3539,8 @@ skip_insns_after_block (basic_block bb)
>   	  continue;
>   
>   	case NOTE:
> -	  switch (NOTE_KIND (insn))
> -	    {
> -	    case NOTE_INSN_BLOCK_END:
> -	      gcc_unreachable ();
> -	    default:
> -	      continue;
> -	    }
> -	  break;
> +	  gcc_assert (NOTE_KIND (insn) != NOTE_INSN_BLOCK_END);
> +	  continue;
>   
>   	case CODE_LABEL:
>   	  if (NEXT_INSN (insn)
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 0bd58525726..cc88a36dd39 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -7892,10 +7892,6 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p,
>               return postfix_expression;
>   	}
>       }
> -
> -  /* We should never get here.  */
> -  gcc_unreachable ();

Hmm, I generally disagree with removing gcc_unreachable() asserts 
because they are unreachable; it seems like it increases the fragility 
of the code in case later changes wrongly make them reachable.

> -  return error_mark_node;
>   }
>   
>   /* Helper function for cp_parser_parenthesized_expression_list and
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 31ed773e145..f4b9d9673fb 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -18242,13 +18242,11 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
>         stmt = finish_co_yield_expr (input_location,
>   				   RECUR (TREE_OPERAND (t, 0)));
>         RETURN (stmt);
> -      break;
>   
>       case CO_AWAIT_EXPR:
>         stmt = finish_co_await_expr (input_location,
>   				   RECUR (TREE_OPERAND (t, 0)));
>         RETURN (stmt);
> -      break;
>   
>       case EXPR_STMT:
>         tmp = RECUR (EXPR_STMT_EXPR (t));
> diff --git a/gcc/expr.c b/gcc/expr.c
> index 5673902b1fc..b2815257509 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -1342,9 +1342,6 @@ op_by_pieces_d::run ()
>   	}
>       }
>     while (1);
> -
> -  /* The code above should have handled everything.  */
> -  gcc_assert (!length);
>   }
>   
>   /* Derived class from op_by_pieces_d, providing support for block move
> diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c
> index f5ba7cecd54..16ee2afc9c0 100644
> --- a/gcc/fortran/frontend-passes.c
> +++ b/gcc/fortran/frontend-passes.c
> @@ -5229,7 +5229,6 @@ gfc_expr_walker (gfc_expr **e, walk_expr_fn_t exprfn, void *data)
>   	  case EXPR_OP:
>   	    WALK_SUBEXPR ((*e)->value.op.op1);
>   	    WALK_SUBEXPR_TAIL ((*e)->value.op.op2);
> -	    break;
>   	  case EXPR_FUNCTION:
>   	    for (a = (*e)->value.function.actual; a; a = a->next)
>   	      WALK_SUBEXPR (a->expr);
> diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
> index 69b81ab97f8..4d72ff78543 100644
> --- a/gcc/fortran/scanner.c
> +++ b/gcc/fortran/scanner.c
> @@ -1159,7 +1159,6 @@ skip_fixed_comments (void)
>   	  skip_comment_line ();
>   	  continue;
>   
> -	  gcc_unreachable ();
>   check_for_digits:
>   	  {
>   	    /* Required for OpenMP's conditional compilation sentinel. */
> diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
> index bc502c0f43c..e413b2d7a1f 100644
> --- a/gcc/fortran/trans-expr.c
> +++ b/gcc/fortran/trans-expr.c
> @@ -11079,14 +11079,9 @@ gfc_expr_is_variable (gfc_expr *expr)
>   	  func_ifc = expr->value.function.esym;
>   	  goto found_ifc;
>   	}
> -      else
> -	{
> -	  gcc_assert (expr->symtree);
> -	  func_ifc = expr->symtree->n.sym;
> -	  goto found_ifc;
> -	}
> -
> -      gcc_unreachable ();
> +      gcc_assert (expr->symtree);
> +      func_ifc = expr->symtree->n.sym;
> +      goto found_ifc;
>       }
>   
>     comp = gfc_get_proc_ptr_comp (expr);
> diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
> index 5814204c681..62aa47a73bd 100644
> --- a/gcc/sched-deps.c
> +++ b/gcc/sched-deps.c
> @@ -3816,7 +3816,6 @@ sched_analyze (class deps_desc *deps, rtx_insn *head, rtx_insn *tail)
>   
>     for (insn = head;; insn = NEXT_INSN (insn))
>       {
> -
>         if (INSN_P (insn))
>   	{
>   	  /* And initialize deps_lists.  */
> @@ -3836,7 +3835,6 @@ sched_analyze (class deps_desc *deps, rtx_insn *head, rtx_insn *tail)
>   	  return;
>   	}
>       }
> -  gcc_unreachable ();
>   }
>   
>   /* Helper for sched_free_deps ().
> diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
> index 48965bfb0ad..b76a48eb16e 100644
> --- a/gcc/sel-sched-ir.c
> +++ b/gcc/sel-sched-ir.c
> @@ -4918,9 +4918,6 @@ in_same_ebb_p (insn_t insn, insn_t succ)
>   
>         ptr = bb_next_bb (ptr);
>       }
> -
> -  gcc_unreachable ();
> -  return false;
>   }
>   
>   /* Recomputes the reverse topological order for the function and
> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> index 3c253e2843f..88fd7821c5e 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -1800,9 +1800,6 @@ nonoverlapping_refs_since_match_p (tree match1, tree ref1,
>   	  return 1;
>   	}
>       }
> -
> -  ++alias_stats.nonoverlapping_refs_since_match_p_must_overlap;
> -  return 0;
>   }
>   
>   /* Return TYPE_UID which can be used to match record types we consider
> diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
> index 94c75497495..cfea0c80be3 100644
> --- a/gcc/tree-vect-slp.c
> +++ b/gcc/tree-vect-slp.c
> @@ -7311,20 +7311,14 @@ vectorize_slp_instance_root_stmt (slp_tree node, slp_instance instance)
>       {
>         if (SLP_TREE_NUMBER_OF_VEC_STMTS (node) == 1)
>   	{
> -	  gimple *child_stmt;
> -	  int j;
> -
> -	  FOR_EACH_VEC_ELT (SLP_TREE_VEC_STMTS (node), j, child_stmt)
> -	    {
> -	      tree vect_lhs = gimple_get_lhs (child_stmt);
> -	      tree root_lhs = gimple_get_lhs (instance->root_stmts[0]->stmt);
> -	      if (!useless_type_conversion_p (TREE_TYPE (root_lhs),
> -					      TREE_TYPE (vect_lhs)))
> -		vect_lhs = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (root_lhs),
> -				   vect_lhs);
> -	      rstmt = gimple_build_assign (root_lhs, vect_lhs);
> -	      break;
> -	    }
> +	  gimple *child_stmt = SLP_TREE_VEC_STMTS (node)[0];
> +	  tree vect_lhs = gimple_get_lhs (child_stmt);
> +	  tree root_lhs = gimple_get_lhs (instance->root_stmts[0]->stmt);
> +	  if (!useless_type_conversion_p (TREE_TYPE (root_lhs),
> +					  TREE_TYPE (vect_lhs)))
> +	    vect_lhs = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (root_lhs),
> +			       vect_lhs);
> +	  rstmt = gimple_build_assign (root_lhs, vect_lhs);
>   	}
>         else if (SLP_TREE_NUMBER_OF_VEC_STMTS (node) > 1)
>   	{
> diff --git a/gcc/tree.c b/gcc/tree.c
> index 910fb06d6f5..4d91fdea758 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -11127,7 +11127,6 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>       case TREE_LIST:
>         WALK_SUBTREE (TREE_VALUE (*tp));
>         WALK_SUBTREE_TAIL (TREE_CHAIN (*tp));
> -      break;
>   
>       case TREE_VEC:
>         {
> @@ -11206,7 +11205,6 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>   	  WALK_SUBTREE (OMP_CLAUSE_OPERAND (*tp, i));
>   	WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (*tp));
>         }
> -      break;
>   
>       case TARGET_EXPR:
>         {
> diff --git a/gcc/vec-perm-indices.c b/gcc/vec-perm-indices.c
> index 31b32ea0589..9e6b5af327c 100644
> --- a/gcc/vec-perm-indices.c
> +++ b/gcc/vec-perm-indices.c
> @@ -228,7 +228,6 @@ vec_perm_indices::series_p (unsigned int out_base, unsigned int out_step,
>   
>         out_base += out_step;
>       }
> -  return true;
>   }
>   
>   /* Return true if all elements of the permutation vector are in the range
>