diff mbox

RFC: A few more fallthrus

Message ID 20160811144930.GV7007@redhat.com
State New
Headers show

Commit Message

Marek Polacek Aug. 11, 2016, 2:49 p.m. UTC 
A few more cases where I'm unsure whether the fall through is intended.
Jason, can you please look at the cp/ part?
Richi, would you mind looking at the tree-complex.c bit?
What 'bout the pch.c?

Thanks,

2016-08-11  Marek Polacek  <polacek@redhat.com>

	PR c/7652
gcc/
	* tree-complex.c (expand_complex_division): Likewise.
gcc/cp/
	* call.c (add_builtin_candidate): Add gcc_fallthrough.
	* cxx-pretty-print.c (pp_cxx_unqualified_id): Likewise.
	* parser.c (cp_parser_skip_to_end_of_statement): Likewise.
	(cp_parser_cache_defarg): Likewise.
libcpp/
	* pch.c (write_macdef): Add CPP_FALLTHRU.


	Marek

Comments

Richard Biener Aug. 12, 2016, 7:20 a.m. UTC | #1 
On Thu, 11 Aug 2016, Marek Polacek wrote:

> A few more cases where I'm unsure whether the fall through is intended.
> Jason, can you please look at the cp/ part?
> Richi, would you mind looking at the tree-complex.c bit?
> What 'bout the pch.c?
> 
> Thanks,
> 
> 2016-08-11  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c/7652
> gcc/
> 	* tree-complex.c (expand_complex_division): Likewise.
> gcc/cp/
> 	* call.c (add_builtin_candidate): Add gcc_fallthrough.
> 	* cxx-pretty-print.c (pp_cxx_unqualified_id): Likewise.
> 	* parser.c (cp_parser_skip_to_end_of_statement): Likewise.
> 	(cp_parser_cache_defarg): Likewise.
> libcpp/
> 	* pch.c (write_macdef): Add CPP_FALLTHRU.
> 
> --- gcc/gcc/cp/call.c
> +++ gcc/gcc/cp/call.c
> @@ -2542,6 +2544,8 @@ add_builtin_candidate (struct z_candidate **candidates, enum tree_code code,
>  	  type2 = ptrdiff_type_node;
>  	  break;
>  	}
> +      /* XXX Really fallthru?  */
> +      /* FALLTHRU */
>      case MULT_EXPR:
>      case TRUNC_DIV_EXPR:
>        if (ARITHMETIC_TYPE_P (type1) && ARITHMETIC_TYPE_P (type2))
> --- gcc/gcc/cp/cxx-pretty-print.c
> +++ gcc/gcc/cp/cxx-pretty-print.c
> @@ -142,6 +142,8 @@ pp_cxx_unqualified_id (cxx_pretty_printer *pp, tree t)
>  
>      case OVERLOAD:
>        t = OVL_CURRENT (t);
> +      /* XXX Really fallthru?  */
> +      /* FALLTHRU */
>      case VAR_DECL:
>      case PARM_DECL:
>      case CONST_DECL:
> --- gcc/gcc/cp/parser.c
> +++ gcc/gcc/cp/parser.c
> @@ -3488,6 +3488,8 @@ cp_parser_skip_to_end_of_statement (cp_parser* parser)
>  	      cp_lexer_consume_token (parser->lexer);
>  	      return;
>  	    }
> +	  /* XXX Really fallthru?  */
> +	  /* FALLTHRU */
>  
>  	case CPP_OPEN_BRACE:
>  	  ++nesting_depth;
> @@ -27640,6 +27646,8 @@ cp_parser_cache_defarg (cp_parser *parser, bool nsdmi)
>  	      parser->in_template_argument_list_p = saved_italp;
>  	      break;
>  	    }
> +	  /* XXX Really fallthru?  */
> +	  /* FALLTHRU */
>  	case CPP_CLOSE_PAREN:
>  	case CPP_ELLIPSIS:
>  	  /* If we run into a non-nested `;', `}', or `]',
> --- gcc/gcc/tree-complex.c
> +++ gcc/gcc/tree-complex.c
> @@ -1336,6 +1336,8 @@ expand_complex_division (gimple_stmt_iterator *gsi, tree inner_type,
>        rr = gimplify_build2 (gsi, code, inner_type, ai, bi);
>        ri = gimplify_build2 (gsi, code, inner_type, ar, bi);
>        ri = gimplify_build1 (gsi, NEGATE_EXPR, inner_type, ri);
> +      /* XXX Really fallthru?  */
> +      /* FALLTHRU */
>  


Nope, looks like a bug to me.

Richard.

>      case PAIR (ONLY_REAL, VARYING):
>      case PAIR (ONLY_IMAG, VARYING):
> --- gcc/libcpp/pch.c
> +++ gcc/libcpp/pch.c
> @@ -55,6 +55,8 @@ write_macdef (cpp_reader *pfile, cpp_hashnode *hn, void *file_p)
>      case NT_VOID:
>        if (! (hn->flags & NODE_POISONED))
>  	return 1;
> +      /* XXX Really fallthru?  */
> +      /* FALLTHRU */
>  
>      case NT_MACRO:
>        if ((hn->flags & NODE_BUILTIN)
> 
> 	Marek
> 
>
Marek Polacek Aug. 16, 2016, 4:59 p.m. UTC | #2 
On Thu, Aug 11, 2016 at 04:49:30PM +0200, Marek Polacek wrote:
> A few more cases where I'm unsure whether the fall through is intended.
> Jason, can you please look at the cp/ part?

Given Jason is on PTO this week, can I just commit the patch as-is, modulo
the already-committed tree-complex.c part?  We can always revisit these few
cases later.  It'd make my next patch submission more manageable.

> Richi, would you mind looking at the tree-complex.c bit?
> What 'bout the pch.c?
> 
> Thanks,
> 
> 2016-08-11  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c/7652
> gcc/
> 	* tree-complex.c (expand_complex_division): Likewise.
> gcc/cp/
> 	* call.c (add_builtin_candidate): Add gcc_fallthrough.
> 	* cxx-pretty-print.c (pp_cxx_unqualified_id): Likewise.
> 	* parser.c (cp_parser_skip_to_end_of_statement): Likewise.
> 	(cp_parser_cache_defarg): Likewise.
> libcpp/
> 	* pch.c (write_macdef): Add CPP_FALLTHRU.
> 
> --- gcc/gcc/cp/call.c
> +++ gcc/gcc/cp/call.c
> @@ -2542,6 +2544,8 @@ add_builtin_candidate (struct z_candidate **candidates, enum tree_code code,
>  	  type2 = ptrdiff_type_node;
>  	  break;
>  	}
> +      /* XXX Really fallthru?  */
> +      /* FALLTHRU */
>      case MULT_EXPR:
>      case TRUNC_DIV_EXPR:
>        if (ARITHMETIC_TYPE_P (type1) && ARITHMETIC_TYPE_P (type2))
> --- gcc/gcc/cp/cxx-pretty-print.c
> +++ gcc/gcc/cp/cxx-pretty-print.c
> @@ -142,6 +142,8 @@ pp_cxx_unqualified_id (cxx_pretty_printer *pp, tree t)
>  
>      case OVERLOAD:
>        t = OVL_CURRENT (t);
> +      /* XXX Really fallthru?  */
> +      /* FALLTHRU */
>      case VAR_DECL:
>      case PARM_DECL:
>      case CONST_DECL:
> --- gcc/gcc/cp/parser.c
> +++ gcc/gcc/cp/parser.c
> @@ -3488,6 +3488,8 @@ cp_parser_skip_to_end_of_statement (cp_parser* parser)
>  	      cp_lexer_consume_token (parser->lexer);
>  	      return;
>  	    }
> +	  /* XXX Really fallthru?  */
> +	  /* FALLTHRU */
>  
>  	case CPP_OPEN_BRACE:
>  	  ++nesting_depth;
> @@ -27640,6 +27646,8 @@ cp_parser_cache_defarg (cp_parser *parser, bool nsdmi)
>  	      parser->in_template_argument_list_p = saved_italp;
>  	      break;
>  	    }
> +	  /* XXX Really fallthru?  */
> +	  /* FALLTHRU */
>  	case CPP_CLOSE_PAREN:
>  	case CPP_ELLIPSIS:
>  	  /* If we run into a non-nested `;', `}', or `]',
> --- gcc/gcc/tree-complex.c
> +++ gcc/gcc/tree-complex.c
> @@ -1336,6 +1336,8 @@ expand_complex_division (gimple_stmt_iterator *gsi, tree inner_type,
>        rr = gimplify_build2 (gsi, code, inner_type, ai, bi);
>        ri = gimplify_build2 (gsi, code, inner_type, ar, bi);
>        ri = gimplify_build1 (gsi, NEGATE_EXPR, inner_type, ri);
> +      /* XXX Really fallthru?  */
> +      /* FALLTHRU */
>  
>      case PAIR (ONLY_REAL, VARYING):
>      case PAIR (ONLY_IMAG, VARYING):
> --- gcc/libcpp/pch.c
> +++ gcc/libcpp/pch.c
> @@ -55,6 +55,8 @@ write_macdef (cpp_reader *pfile, cpp_hashnode *hn, void *file_p)
>      case NT_VOID:
>        if (! (hn->flags & NODE_POISONED))
>  	return 1;
> +      /* XXX Really fallthru?  */
> +      /* FALLTHRU */
>  
>      case NT_MACRO:
>        if ((hn->flags & NODE_BUILTIN)

	Marek
Jeff Law Aug. 18, 2016, 3:12 a.m. UTC | #3 
On 08/16/2016 10:59 AM, Marek Polacek wrote:
> On Thu, Aug 11, 2016 at 04:49:30PM +0200, Marek Polacek wrote:
>> A few more cases where I'm unsure whether the fall through is intended.
>> Jason, can you please look at the cp/ part?
>
> Given Jason is on PTO this week, can I just commit the patch as-is, modulo
> the already-committed tree-complex.c part?  We can always revisit these few
> cases later.  It'd make my next patch submission more manageable.
>
>> Richi, would you mind looking at the tree-complex.c bit?
>> What 'bout the pch.c?
>>
>> Thanks,
>>
>> 2016-08-11  Marek Polacek  <polacek@redhat.com>
>>
>> 	PR c/7652
>> gcc/
>> 	* tree-complex.c (expand_complex_division): Likewise.
>> gcc/cp/
>> 	* call.c (add_builtin_candidate): Add gcc_fallthrough.
>> 	* cxx-pretty-print.c (pp_cxx_unqualified_id): Likewise.
>> 	* parser.c (cp_parser_skip_to_end_of_statement): Likewise.
>> 	(cp_parser_cache_defarg): Likewise.
>> libcpp/
>> 	* pch.c (write_macdef): Add CPP_FALLTHRU.
They obviously don't change behavior, so they're safe in that sense. 
And you've got a marker so you can find them later or anyone else 
looking at the code knows we weren't sure on these.

OK.
jeff
diff mbox

Patch

--- gcc/gcc/cp/call.c
+++ gcc/gcc/cp/call.c
@@ -2542,6 +2544,8 @@  add_builtin_candidate (struct z_candidate **candidates, enum tree_code code,
 	  type2 = ptrdiff_type_node;
 	  break;
 	}
+      /* XXX Really fallthru?  */
+      /* FALLTHRU */
     case MULT_EXPR:
     case TRUNC_DIV_EXPR:
       if (ARITHMETIC_TYPE_P (type1) && ARITHMETIC_TYPE_P (type2))
--- gcc/gcc/cp/cxx-pretty-print.c
+++ gcc/gcc/cp/cxx-pretty-print.c
@@ -142,6 +142,8 @@  pp_cxx_unqualified_id (cxx_pretty_printer *pp, tree t)
 
     case OVERLOAD:
       t = OVL_CURRENT (t);
+      /* XXX Really fallthru?  */
+      /* FALLTHRU */
     case VAR_DECL:
     case PARM_DECL:
     case CONST_DECL:
--- gcc/gcc/cp/parser.c
+++ gcc/gcc/cp/parser.c
@@ -3488,6 +3488,8 @@  cp_parser_skip_to_end_of_statement (cp_parser* parser)
 	      cp_lexer_consume_token (parser->lexer);
 	      return;
 	    }
+	  /* XXX Really fallthru?  */
+	  /* FALLTHRU */
 
 	case CPP_OPEN_BRACE:
 	  ++nesting_depth;
@@ -27640,6 +27646,8 @@  cp_parser_cache_defarg (cp_parser *parser, bool nsdmi)
 	      parser->in_template_argument_list_p = saved_italp;
 	      break;
 	    }
+	  /* XXX Really fallthru?  */
+	  /* FALLTHRU */
 	case CPP_CLOSE_PAREN:
 	case CPP_ELLIPSIS:
 	  /* If we run into a non-nested `;', `}', or `]',
--- gcc/gcc/tree-complex.c
+++ gcc/gcc/tree-complex.c
@@ -1336,6 +1336,8 @@  expand_complex_division (gimple_stmt_iterator *gsi, tree inner_type,
       rr = gimplify_build2 (gsi, code, inner_type, ai, bi);
       ri = gimplify_build2 (gsi, code, inner_type, ar, bi);
       ri = gimplify_build1 (gsi, NEGATE_EXPR, inner_type, ri);
+      /* XXX Really fallthru?  */
+      /* FALLTHRU */
 
     case PAIR (ONLY_REAL, VARYING):
     case PAIR (ONLY_IMAG, VARYING):
--- gcc/libcpp/pch.c
+++ gcc/libcpp/pch.c
@@ -55,6 +55,8 @@  write_macdef (cpp_reader *pfile, cpp_hashnode *hn, void *file_p)
     case NT_VOID:
       if (! (hn->flags & NODE_POISONED))
 	return 1;
+      /* XXX Really fallthru?  */
+      /* FALLTHRU */
 
     case NT_MACRO:
       if ((hn->flags & NODE_BUILTIN)