| Message ID | 20160809152126.GQ7007@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Aug 09, 2016 at 05:21:26PM +0200, Marek Polacek wrote: > Bootstrapped/regtested on x86_64-linux and ppc64le-redhat-linux. > > 2016-08-09 Marek Polacek <polacek@redhat.com> > > PR c/7652 > gcc/c-family/ > * c-ada-spec.c (dump_generic_ada_node): Add return. > gcc/ > * cselib.c (cselib_expand_value_rtx_1): Add return. > * gengtype.c (dbgprint_count_type_at): Likewise. > * hsa-gen.c (gen_hsa_insn_for_internal_fn_call): Likewise. > * reg-stack.c (get_true_reg): Restructure to avoid fallthrough warning. > > --- gcc/gcc/cselib.c > +++ gcc/gcc/cselib.c > @@ -1618,6 +1618,7 @@ cselib_expand_value_rtx_1 (rtx orig, struct expand_value_data *evd, > else > return orig; > } > + return orig; > } > > CASE_CONST_ANY: > --- gcc/gcc/gengtype.c > +++ gcc/gcc/gengtype.c > @@ -175,6 +175,7 @@ dbgprint_count_type_at (const char *fil, int lin, const char *msg, type_p t) > { > case TYPE_UNDEFINED: > nb_undefined++; > + break; > case TYPE_SCALAR: > nb_scalar++; > break; These two are ok. > --- gcc/gcc/reg-stack.c > +++ gcc/gcc/reg-stack.c > @@ -423,23 +423,25 @@ get_true_reg (rtx *pat) > GET_MODE (subreg)); > return pat; > } > + pat = &XEXP (*pat, 0); > + break; > } While this one is similar to the cselib.c case, I wonder if it wouldn't be nicer to drop the {}s, reindent the case SUBREG: and just /* FALLTHRU */ at the end. Uninitialized decls can be declared anywhere in the switch in C++ or in C99+. I don't feel strongly about it, so if you don't want to do that, this hunk is ok too. > case FLOAT: > case FIX: > case FLOAT_EXTEND: > - pat = & XEXP (*pat, 0); > + pat = &XEXP (*pat, 0); > break; > > case UNSPEC: > if (XINT (*pat, 1) == UNSPEC_TRUNC_NOOP > || XINT (*pat, 1) == UNSPEC_FILD_ATOMIC) > - pat = & XVECEXP (*pat, 0, 0); > + pat = &XVECEXP (*pat, 0, 0); > return pat; > > case FLOAT_TRUNCATE: > if (!flag_unsafe_math_optimizations) > return pat; > - pat = & XEXP (*pat, 0); > + pat = &XEXP (*pat, 0); > break; > > default: The above 3 are obviously ok. Jakub
Hi, On Tue, Aug 09, 2016 at 05:21:26PM +0200, Marek Polacek wrote: > Testing with -Wimplicit-fallthrough turned up a few spots where > fall through is either a bug, or just too confusing. This patch > should be desirable even though the warning is still not in. > > Eric, can you look at the c-ada-spec.c part? > Martin, does the hsa-gen.c part look correct? > > --- gcc/gcc/hsa-gen.c > +++ gcc/gcc/hsa-gen.c > @@ -5039,6 +5039,7 @@ gen_hsa_insn_for_internal_fn_call (gcall *stmt, hsa_bb *hbb) > case IFN_FMIN: > case IFN_FMAX: > gen_hsa_insns_for_call_of_internal_fn (stmt, hbb); > + break; > > default: > HSA_SORRY_ATV (gimple_location (stmt), Yes it is correct. Thanks for catching this. -Wimplicit-fallthrough is really useful work! Martin
> 2016-08-09 Marek Polacek <polacek@redhat.com> > > PR c/7652 > gcc/c-family/ > * c-ada-spec.c (dump_generic_ada_node): Add return. OK, thanks.
diff --git gcc/gcc/c-family/c-ada-spec.c gcc/gcc/c-family/c-ada-spec.c index e33fdff..17b8610 100644 --- gcc/gcc/c-family/c-ada-spec.c +++ gcc/gcc/c-family/c-ada-spec.c @@ -1862,6 +1862,7 @@ dump_generic_ada_node (pretty_printer *buffer, tree node, tree type, int spc, case TREE_BINFO: dump_generic_ada_node (buffer, BINFO_TYPE (node), type, spc, limited_access, name_only); + return 0; case TREE_VEC: pp_string (buffer, "--- unexpected node: TREE_VEC"); diff --git gcc/gcc/cselib.c gcc/gcc/cselib.c index 0c5183c..1277842 100644 --- gcc/gcc/cselib.c +++ gcc/gcc/cselib.c @@ -1618,6 +1618,7 @@ cselib_expand_value_rtx_1 (rtx orig, struct expand_value_data *evd, else return orig; } + return orig; } CASE_CONST_ANY: diff --git gcc/gcc/gengtype.c gcc/gcc/gengtype.c index 5479b8f..0518355 100644 --- gcc/gcc/gengtype.c +++ gcc/gcc/gengtype.c @@ -175,6 +175,7 @@ dbgprint_count_type_at (const char *fil, int lin, const char *msg, type_p t) { case TYPE_UNDEFINED: nb_undefined++; + break; case TYPE_SCALAR: nb_scalar++; break; diff --git gcc/gcc/hsa-gen.c gcc/gcc/hsa-gen.c index 6cf1538..fa4ef12 100644 --- gcc/gcc/hsa-gen.c +++ gcc/gcc/hsa-gen.c @@ -5039,6 +5039,7 @@ gen_hsa_insn_for_internal_fn_call (gcall *stmt, hsa_bb *hbb) case IFN_FMIN: case IFN_FMAX: gen_hsa_insns_for_call_of_internal_fn (stmt, hbb); + break; default: HSA_SORRY_ATV (gimple_location (stmt), diff --git gcc/gcc/reg-stack.c gcc/gcc/reg-stack.c index c931349..907f28d 100644 --- gcc/gcc/reg-stack.c +++ gcc/gcc/reg-stack.c @@ -423,23 +423,25 @@ get_true_reg (rtx *pat) GET_MODE (subreg)); return pat; } + pat = &XEXP (*pat, 0); + break; } case FLOAT: case FIX: case FLOAT_EXTEND: - pat = & XEXP (*pat, 0); + pat = &XEXP (*pat, 0); break; case UNSPEC: if (XINT (*pat, 1) == UNSPEC_TRUNC_NOOP || XINT (*pat, 1) == UNSPEC_FILD_ATOMIC) - pat = & XVECEXP (*pat, 0, 0); + pat = &XVECEXP (*pat, 0, 0); return pat; case FLOAT_TRUNCATE: if (!flag_unsafe_math_optimizations) return pat; - pat = & XEXP (*pat, 0); + pat = &XEXP (*pat, 0); break; default: