public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RFC: A few more fallthrus
@ 2016-08-11 14:49 Marek Polacek
  2016-08-12  7:20 ` Richard Biener
  2016-08-16 16:59 ` Marek Polacek
  0 siblings, 2 replies; 6+ messages in thread
From: Marek Polacek @ 2016-08-11 14:49 UTC (permalink / raw)
  To: GCC Patches, Jason Merrill, Richard Biener

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFC: A few more fallthrus
  2016-08-11 14:49 RFC: A few more fallthrus Marek Polacek
@ 2016-08-12  7:20 ` Richard Biener
  2016-08-12 10:14   ` Marek Polacek
  2016-08-16 16:59 ` Marek Polacek
  1 sibling, 1 reply; 6+ messages in thread
From: Richard Biener @ 2016-08-12  7:20 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches, Jason Merrill

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

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFC: A few more fallthrus
  2016-08-12  7:20 ` Richard Biener
@ 2016-08-12 10:14   ` Marek Polacek
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Polacek @ 2016-08-12 10:14 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches, Jason Merrill

On Fri, Aug 12, 2016 at 09:20:36AM +0200, Richard Biener wrote:
> > --- 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.

Thanks, thus:

Bootstrapped/regtested on x86_64-linux, applying to trunk.

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

	PR c/7652
	* tree-complex.c (expand_complex_division): Add missing break.

diff --git gcc/tree-complex.c gcc/tree-complex.c
index d7baf22..7668520 100644
--- gcc/tree-complex.c
+++ gcc/tree-complex.c
@@ -1336,6 +1336,7 @@ 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);
+      break;
 
     case PAIR (ONLY_REAL, VARYING):
     case PAIR (ONLY_IMAG, VARYING):

	Marek

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFC: A few more fallthrus
  2016-08-11 14:49 RFC: A few more fallthrus Marek Polacek
  2016-08-12  7:20 ` Richard Biener
@ 2016-08-16 16:59 ` Marek Polacek
  2016-08-18  3:13   ` Jeff Law
  1 sibling, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2016-08-16 16:59 UTC (permalink / raw)
  To: GCC Patches, Jason Merrill, Richard Biener

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFC: A few more fallthrus
  2016-08-16 16:59 ` Marek Polacek
@ 2016-08-18  3:13   ` Jeff Law
  2016-10-05 19:47     ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2016-08-18  3:13 UTC (permalink / raw)
  To: Marek Polacek, GCC Patches, Jason Merrill, Richard Biener

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFC: A few more fallthrus
  2016-08-18  3:13   ` Jeff Law
@ 2016-10-05 19:47     ` Jason Merrill
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2016-10-05 19:47 UTC (permalink / raw)
  To: Jeff Law; +Cc: Marek Polacek, GCC Patches, Richard Biener

[-- Attachment #1: Type: text/plain, Size: 69 bytes --]

One of these was a bug, though it doesn't affect regression testing.

[-- Attachment #2: break.diff --]
[-- Type: text/plain, Size: 1582 bytes --]

commit c49bb7df1d286456f2e5738c05ea56e6816a7475
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Oct 5 11:32:33 2016 -0400

    * parser.c (cp_parser_skip_to_end_of_statement): Add missing break.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 8c5cb20..0914ae2 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -2544,7 +2544,6 @@ 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:
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index f4d2a2e..a290c87 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -142,7 +142,6 @@ 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:
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index f3dc359..60bbf49 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -3495,8 +3495,7 @@ cp_parser_skip_to_end_of_statement (cp_parser* parser)
 	      cp_lexer_consume_token (parser->lexer);
 	      return;
 	    }
-	  /* XXX Really fallthru?  */
-	  /* FALLTHRU */
+	  break;
 
 	case CPP_OPEN_BRACE:
 	  ++nesting_depth;
@@ -27932,7 +27931,6 @@ 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:

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-10-05 19:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-11 14:49 RFC: A few more fallthrus Marek Polacek
2016-08-12  7:20 ` Richard Biener
2016-08-12 10:14   ` Marek Polacek
2016-08-16 16:59 ` Marek Polacek
2016-08-18  3:13   ` Jeff Law
2016-10-05 19:47     ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).