Re: Implement -Wimplicit-fallthrough (version 8): add gcc_fallthrough()

[Jakub Jelinek <jakub at redhat dot com>]

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 */

> 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 */

> 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.

> --- 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?

	Jakub