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

Marek Polacek polacek@redhat.com
Thu Sep 1 14:51:00 GMT 2016


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  <polacek@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



More information about the Gcc-patches mailing list