Message ID | 0EFAB2BDD0F67E4FB6CCC8B9F87D7569429DC138@IRSMSX103.ger.corp.intel.com |
---|---|
State | New |
Headers | show |
On Thu, Apr 10, 2014 at 02:23:16PM +0000, Zamyatin, Igor wrote: > 2014-04-10 Igor Zamyatin <igor.zamyatin@intel.com> > > PR c++/60189 > * parser.c (cp_parser_postfix_expression): Make sure only > semicolon can go after Cilk_sync. > > gcc/testsuite/ChangeLog: > > 2014-04-10 Igor Zamyatin <igor.zamyatin@intel.com> > > PR c++/60189 > * c-c++-common/cilk-plus/CK/invalid_sync.cс: New test. CCing Jason as this is a C++ FE change. > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -5835,20 +5835,33 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p, > } > break; > } > - > + > case RID_CILK_SYNC: > - if (flag_cilkplus) > - { > - tree sync_expr = build_cilk_sync (); > - SET_EXPR_LOCATION (sync_expr, > - cp_lexer_peek_token (parser->lexer)->location); > - finish_expr_stmt (sync_expr); > - } > - else > - error_at (token->location, "-fcilkplus must be enabled to use" > - " %<_Cilk_sync%>"); > - cp_lexer_consume_token (parser->lexer); > - break; > + { I don't see the point of adding the extra {} around the whole case, there is no varaible declared at that point. Other than that it looks good to me, but I'll defer the review to Jason. > + cp_lexer_consume_token (parser->lexer); > + if (flag_cilkplus) > + { > + token = cp_lexer_peek_token (parser->lexer); > + if (token->type != CPP_SEMICOLON) > + { > + error_at (token->location, "%<_Cilk_sync%> must be followed" > + " by semicolon"); > + postfix_expression = error_mark_node; > + break; > + } > + tree sync_expr = build_cilk_sync (); > + SET_EXPR_LOCATION (sync_expr, > + cp_lexer_peek_token (parser->lexer)->location); > + finish_expr_stmt (sync_expr); > + } > + else > + { > + error_at (token->location, "-fcilkplus must be enabled to use" > + " %<_Cilk_sync%>"); > + postfix_expression = error_mark_node; > + } > + break; > + } Jakub
On 04/10/2014 10:27 AM, Jakub Jelinek wrote: > I don't see the point of adding the extra {} around the whole case, > there is no variable declared at that point. Agreed. >> + token = cp_lexer_peek_token (parser->lexer); >> + if (token->type != CPP_SEMICOLON) >> + { >> + error_at (token->location, "%<_Cilk_sync%> must be followed" >> + " by semicolon"); >> + postfix_expression = error_mark_node; >> + break; >> + } Any reason not to use cp_parser_require here? Jason
> > >> + token = cp_lexer_peek_token (parser->lexer); > >> + if (token->type != CPP_SEMICOLON) > >> + { > >> + error_at (token->location, "%<_Cilk_sync%> must be followed" > >> + " by semicolon"); > >> + postfix_expression = error_mark_node; > >> + break; > >> + } > > Any reason not to use cp_parser_require here? Right! Will try it and repost the patch. Thanks! Igor > > Jason
> > >> + token = cp_lexer_peek_token (parser->lexer); > >> + if (token->type != CPP_SEMICOLON) > >> + { > >> + error_at (token->location, "%<_Cilk_sync%> must be followed" > >> + " by semicolon"); > >> + postfix_expression = error_mark_node; > >> + break; > >> + } > > Any reason not to use cp_parser_require here? I remembered - I haven't used cp_parser_require since it calls cp_lexer_consume_token which is not needed at this point. It is already called a bit earlier. Igor > > Jason
On 04/11/2014 03:08 PM, Zamyatin, Igor wrote:
> I remembered - I haven't used cp_parser_require since it calls cp_lexer_consume_token which is not needed at this point. It is already called a bit earlier.
So the call to cp_parser_require can replace that call as well.
Jason
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 7bea3d2..95f9c93 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -5835,20 +5835,33 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p, } break; } - + case RID_CILK_SYNC: - if (flag_cilkplus) - { - tree sync_expr = build_cilk_sync (); - SET_EXPR_LOCATION (sync_expr, - cp_lexer_peek_token (parser->lexer)->location); - finish_expr_stmt (sync_expr); - } - else - error_at (token->location, "-fcilkplus must be enabled to use" - " %<_Cilk_sync%>"); - cp_lexer_consume_token (parser->lexer); - break; + { + cp_lexer_consume_token (parser->lexer); + if (flag_cilkplus) + { + token = cp_lexer_peek_token (parser->lexer); + if (token->type != CPP_SEMICOLON) + { + error_at (token->location, "%<_Cilk_sync%> must be followed" + " by semicolon"); + postfix_expression = error_mark_node; + break; + } + tree sync_expr = build_cilk_sync (); + SET_EXPR_LOCATION (sync_expr, + cp_lexer_peek_token (parser->lexer)->location); + finish_expr_stmt (sync_expr); + } + else + { + error_at (token->location, "-fcilkplus must be enabled to use" + " %<_Cilk_sync%>"); + postfix_expression = error_mark_node; + } + break; + } case RID_BUILTIN_SHUFFLE: { diff --git a/gcc/testsuite/c-c++-common/cilk-plus/CK/invalid_sync.сc b/gcc/testsuite/c-c++-common/cilk-plus/CK/invalid_sync.cс new file mode 100644 index 0000000..e7bec68 --- /dev/null +++ b/gcc/testsuite/c-c++-common/cilk-plus/CK/invalid_sync.cс @@ -0,0 +1,9 @@ +/* PR c/60189 */ +/* { dg-do compile } */ +/* { dg-options "-fcilkplus" } */ + +int main (void) +{ + _Cilk_sync return; /* { dg-error " '_Cilk_sync' must be followed by semicolon" } */ + return 0; +}