On Thu, Sep 01, 2016 at 04:27:01PM +0200, Jakub Jelinek wrote:
> On Thu, Sep 01, 2016 at 03:42:12PM +0200, Marek Polacek wrote:
> > --- gcc/gcc/c-family/c-common.c
> > +++ gcc/gcc/c-family/c-common.c
> > @@ -11590,6 +11590,7 @@ resolve_overloaded_builtin (location_t loc, tree
> > function,
> > gcc_unreachable ();
> > }
> > /* Fallthrough to the normal processing. */
> > + gcc_fallthrough ();
> > }
> > case BUILT_IN_ATOMIC_EXCHANGE_N:
> > case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_N:
> > @@ -11598,6 +11599,7 @@ resolve_overloaded_builtin (location_t loc, tree
> > function,
> > {
> > fetch_op = false;
> > /* Fallthrough to further processing. */
> > + gcc_fallthrough ();
> > }
> > case BUILT_IN_ATOMIC_ADD_FETCH_N:
> > case BUILT_IN_ATOMIC_SUB_FETCH_N:
> > @@ -11614,6 +11616,7 @@ resolve_overloaded_builtin (location_t loc, tree
> > function,
> > {
> > orig_format = false;
> > /* Fallthru for parameter processing. */
> > + gcc_fallthrough ();
> > }
>
> These 3 look just like misformatted stuff, {}s around for no reason
> whatsoever, the first case even badly indented and the latter two with a
> single stmt inside of {}.
> I think it would be better to just remove the useless outer {} pair
> in all the cases, reindent and replace the comments with /* FALLTHRU */
Ok, fixed.
> > diff --git gcc/gcc/c/c-typeck.c gcc/gcc/c/c-typeck.c
> > index 5194027..e89bdc8 100644
> > --- gcc/gcc/c/c-typeck.c
> > +++ gcc/gcc/c/c-typeck.c
> > @@ -605,7 +605,7 @@ composite_type (tree t1, tree t2)
> >
> > t1 = build_function_type (valtype, newargs);
> > t1 = qualify_type (t1, t2);
> > - /* ... falls through ... */
> > + gcc_fallthrough ();
> > }
>
> One option for here (other than removing the {}s, which would require
> separate var declarations from their assignments) would be to just do:
> - /* ... falls through ... */
> }
> + /* FALLTHRU */
Done.
> > diff --git gcc/gcc/config/rs6000/rs6000.c gcc/gcc/config/rs6000/rs6000.c
> > index 2f15a05..c25314e 100644
> > --- gcc/gcc/config/rs6000/rs6000.c
> > +++ gcc/gcc/config/rs6000/rs6000.c
> > @@ -5489,7 +5489,7 @@ rs6000_builtin_vectorized_libmass (combined_fn fn,
> > tree type_out,
> > CASE_CFN_HYPOT:
> > CASE_CFN_POW:
> > n_args = 2;
> > - /* fall through */
> > + gcc_fallthrough ();
> >
> > CASE_CFN_ACOS:
> > CASE_CFN_ACOSH:
>
> This is needed becase CSE_CFN_ACOS is a macro, right? I guess it is fine.
Yes, exactly.
> > --- gcc/gcc/cp/error.c
> > +++ gcc/gcc/cp/error.c
> > @@ -576,6 +576,7 @@ dump_type (cxx_pretty_printer *pp, tree t, int flags)
> > default:
> > pp_unsupported_tree (pp, t);
> > /* Fall through to error. */
> > + gcc_fallthrough ();
> >
> > case ERROR_MARK:
> > pp_string (pp, M_("<type error>"));
> > @@ -1277,6 +1278,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags)
> > default:
> > pp_unsupported_tree (pp, t);
> > /* Fall through to error. */
> > + gcc_fallthrough ();
> >
> > case ERROR_MARK:
> > pp_string (pp, M_("<declaration error>"));
> > @@ -2778,6 +2780,7 @@ dump_expr (cxx_pretty_printer *pp, tree t, int flags)
> > default:
> > pp_unsupported_tree (pp, t);
> > /* fall through to ERROR_MARK... */
> > + gcc_fallthrough ();
>
> Why the above ones? Replacing the comments with /* FALLTHRU */ would be
> sufficient IMHO. Or is there any value in explaining why it falls through?
I thought I'd keep it since it conveys something more than a pure "falls
through" comments, but I changed it anyway. Here's the pure comment
changes patch which I hope can be committed right away.
2016-09-01 Marek Polacek <pola...@redhat.com>
PR c/7652
gcc/c-family/
* c-common.c (resolve_overloaded_builtin): Fix formatting. Add
FALLTHRU comments.
gcc/c/
* c-typeck.c (composite_type): Add FALLTHRU comment.
gcc/gcc/cp/
* error.c (dump_type): Fix falls through comment.
(dump_decl): Likewise.
(dump_expr): Likewise.
diff --git gcc/gcc/c-family/c-common.c gcc/gcc/c-family/c-common.c
index b29334a..399ba97 100644
--- gcc/gcc/c-family/c-common.c
+++ gcc/gcc/c-family/c-common.c
@@ -11547,7 +11547,7 @@ resolve_overloaded_builtin (location_t loc, tree
function,
/* Handle these 4 together so that they can fall through to the next
case if the call is transformed to an _N variant. */
switch (orig_code)
- {
+ {
case BUILT_IN_ATOMIC_EXCHANGE:
{
if (resolve_overloaded_atomic_exchange (loc, function, params,
@@ -11588,17 +11588,15 @@ resolve_overloaded_builtin (location_t loc, tree
function,
}
default:
gcc_unreachable ();
- }
- /* Fallthrough to the normal processing. */
+ }
}
+ /* FALLTHRU */
case BUILT_IN_ATOMIC_EXCHANGE_N:
case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_N:
case BUILT_IN_ATOMIC_LOAD_N:
case BUILT_IN_ATOMIC_STORE_N:
- {
- fetch_op = false;
- /* Fallthrough to further processing. */
- }
+ fetch_op = false;
+ /* FALLTHRU */
case BUILT_IN_ATOMIC_ADD_FETCH_N:
case BUILT_IN_ATOMIC_SUB_FETCH_N:
case BUILT_IN_ATOMIC_AND_FETCH_N:
@@ -11611,10 +11609,8 @@ resolve_overloaded_builtin (location_t loc, tree
function,
case BUILT_IN_ATOMIC_FETCH_NAND_N:
case BUILT_IN_ATOMIC_FETCH_XOR_N:
case BUILT_IN_ATOMIC_FETCH_OR_N:
- {
- orig_format = false;
- /* Fallthru for parameter processing. */
- }
+ orig_format = false;
+ /* FALLTHRU */
case BUILT_IN_SYNC_FETCH_AND_ADD_N:
case BUILT_IN_SYNC_FETCH_AND_SUB_N:
case BUILT_IN_SYNC_FETCH_AND_OR_N:
diff --git gcc/gcc/c/c-typeck.c gcc/gcc/c/c-typeck.c
index 5194027..fc7a71e 100644
--- gcc/gcc/c/c-typeck.c
+++ gcc/gcc/c/c-typeck.c
@@ -605,8 +605,8 @@ composite_type (tree t1, tree t2)
t1 = build_function_type (valtype, newargs);
t1 = qualify_type (t1, t2);
- /* ... falls through ... */
}
+ /* FALLTHRU */
default:
return build_type_attribute_variant (t1, attributes);
diff --git gcc/gcc/cp/error.c gcc/gcc/cp/error.c
index 58cd48c..88049ee 100644
--- gcc/gcc/cp/error.c
+++ gcc/gcc/cp/error.c
@@ -575,7 +575,7 @@ dump_type (cxx_pretty_printer *pp, tree t, int flags)
default:
pp_unsupported_tree (pp, t);
- /* Fall through to error. */
+ /* Fall through. */
case ERROR_MARK:
pp_string (pp, M_("<type error>"));
@@ -1276,7 +1276,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags)
default:
pp_unsupported_tree (pp, t);
- /* Fall through to error. */
+ /* Fall through. */
case ERROR_MARK:
pp_string (pp, M_("<declaration error>"));
@@ -2777,7 +2777,7 @@ dump_expr (cxx_pretty_printer *pp, tree t, int flags)
`report_error_function'. That could cause an infinite loop. */
default:
pp_unsupported_tree (pp, t);
- /* fall through to ERROR_MARK... */
+ /* Fall through. */
case ERROR_MARK:
pp_string (pp, M_("<expression error>"));
break;
Marek
