On 12/1/21 02:57, Richard Biener wrote:
On Tue, 30 Nov 2021, Jason Merrill wrote:
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 <rguent...@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.
It seems to be quite inconsistently used in the code base though. Do
you suggest the coding conventions to be amended with something like
"If a function returns non-void and the last statement in the outermost
lexical scope is not a return statement you must add a gcc_unreachable ()
call at this place."
? Most definitely most functions do _not_ follow this.
I think there's room for patterns that are neither required nor
prohibited. :)
The case above involves
while (true)
{
loop without exit (break or goto)
}
}
if somebody would add code doing a break and not add a return he'd
get a diagnostic now. With the gcc_unreachable () in place he'd
_not_ get a diagnostic but maybe some ICEs at compile-time for some
testcase we'd yet have to discover.
Not sure what I think is better ;)
An error at compile time is generally better than a run-time abort, but
I'm not confident that we would give an error for all functions where
it's possible to flow off the end under some circumstances, and the
abort would catch them.
I'm sure we do not want to diagnose
"warning: gcc_unreachable () might be reached"
correct? So placing a gcc_unreachable, while catching errors at
runtime, will suppress diagnostics at compile time.
I see gcc_unreachable as essentially the same as any other assert, in
that it documents an assumption made by the (lack of) code that follows.
It seems entirely reasonable to me for the compiler to warn that a
function will *always* call fancy_abort. I wouldn't nomally warn about
it being possibly reachable when we don't know the constraints on the input.
I'm not strongly against these changes, but in general I'm skeptical of
removing defensive code just because it isn't currently reachable.
Jason
