The new testcase included in this patch used to ICE in gcc after diagnosing the first error, and in g++ it only diagnosed the error in the first metadirective, ignoring the second one. The solution is to make error recovery in the C front end more like that in the C++ front end, and remove the code in both front ends that previously tried to skip all the way over the following statement (instead of just to the end of the metadirective pragma) after an error.
gcc/c/ChangeLog * c-parser.cc (c_parser_skip_to_closing_brace): New, copied from the equivalent function in the C++ front end. (c_parser_skip_to_end_of_block_or_statement): Pass false to the error flag. (c_parser_omp_context_selector): Immediately return error_mark_node after giving an error that the integer trait property is invalid, similarly to C++ front end. (c_parser_omp_context_selector_specification): Likewise handle error return from c_parser_omp_context_selector similarly to C++. (c_parser_omp_metadirective): Do not call c_parser_skip_to_end_of_block_or_statement after an error. gcc/cp/ChangeLog * parser.cc (cp_parser_omp_metadirective): Do not call cp_parser_skip_to_end_of_block_or_statement after an error. gcc/testsuite/ChangeLog * c-c++-common/gomp/declare-variant-2.c: Adjust patterns now that C and C++ now behave similarly. * c-c++-common/gomp/metadirective-error-recovery.c: New. --- gcc/c/c-parser.cc | 84 ++++++++++++++----- gcc/cp/parser.cc | 9 +- .../c-c++-common/gomp/declare-variant-2.c | 13 ++- .../gomp/metadirective-error-recovery.c | 20 +++++ 4 files changed, 90 insertions(+), 36 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/gomp/metadirective-error-recovery.c diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index 4a25bf283a3..faef65879c6 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -1420,6 +1420,51 @@ c_parser_skip_to_end_of_parameter (c_parser *parser) parser->error = false; } +/* Skip tokens until a non-nested closing curly brace is the next + token, or there are no more tokens. Return true in the first case, + false otherwise. */ + +static bool +c_parser_skip_to_closing_brace (c_parser *parser) +{ + unsigned nesting_depth = 0; + + while (true) + { + c_token *token = c_parser_peek_token (parser); + + switch (token->type) + { + case CPP_PRAGMA_EOL: + if (!parser->in_pragma) + break; + /* FALLTHRU */ + case CPP_EOF: + /* If we've run out of tokens, stop. */ + return false; + + case CPP_CLOSE_BRACE: + /* If the next token is a non-nested `}', then we have reached + the end of the current block. */ + if (nesting_depth-- == 0) + return true; + break; + + case CPP_OPEN_BRACE: + /* If it the next token is a `{', then we are entering a new + block. Consume the entire block. */ + ++nesting_depth; + break; + + default: + break; + } + + /* Consume the token. */ + c_parser_consume_token (parser); + } +} + /* Expect to be at the end of the pragma directive and consume an end of line marker. */ @@ -1535,7 +1580,7 @@ c_parser_skip_to_end_of_block_or_statement (c_parser *parser, here for secondary error recovery, after parser->error has been cleared. */ c_parser_consume_pragma (parser); - c_parser_skip_to_pragma_eol (parser); + c_parser_skip_to_pragma_eol (parser, false); parser->error = save_error; continue; @@ -26774,19 +26819,17 @@ c_parser_omp_context_selector (c_parser *parser, enum omp_tss_code set, case OMP_TRAIT_PROPERTY_DEV_NUM_EXPR: case OMP_TRAIT_PROPERTY_BOOL_EXPR: t = c_parser_expr_no_commas (parser, NULL).value; - if (t != error_mark_node) - { - mark_exp_read (t); - t = c_fully_fold (t, false, NULL); - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - error_at (token->location, - "property must be integer expression"); - else - properties = make_trait_property (NULL_TREE, t, - properties); - } - else + if (t == error_mark_node) return error_mark_node; + mark_exp_read (t); + t = c_fully_fold (t, false, NULL); + if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) + { + error_at (token->location, + "property must be integer expression"); + return error_mark_node; + } + properties = make_trait_property (NULL_TREE, t, properties); break; case OMP_TRAIT_PROPERTY_CLAUSE_LIST: if (sel == OMP_TRAIT_CONSTRUCT_SIMD) @@ -26888,11 +26931,14 @@ c_parser_omp_context_selector_specification (c_parser *parser, tree parms) tree selectors = c_parser_omp_context_selector (parser, set, parms); if (selectors == error_mark_node) - ret = error_mark_node; + { + c_parser_skip_to_closing_brace (parser); + ret = error_mark_node; + } else if (ret != error_mark_node) ret = make_trait_set_selector (set, selectors, ret); - braces.skip_until_found_close (parser); + braces.require_close (parser); if (c_parser_next_token_is (parser, CPP_COMMA)) c_parser_consume_token (parser); @@ -29097,7 +29143,6 @@ c_parser_omp_metadirective (c_parser *parser, bool *if_p) { error_at (match_loc, "too many %<otherwise%> or %<default%> " "clauses in %<metadirective%>"); - c_parser_skip_to_end_of_block_or_statement (parser, true); goto error; } default_seen = true; @@ -29106,14 +29151,12 @@ c_parser_omp_metadirective (c_parser *parser, bool *if_p) { error_at (match_loc, "%<otherwise%> or %<default%> clause " "must appear last in %<metadirective%>"); - c_parser_skip_to_end_of_block_or_statement (parser, true); goto error; } if (!default_p && strcmp (p, "when") != 0) { error_at (match_loc, "%qs is not valid for %qs", p, "metadirective"); - c_parser_skip_to_end_of_block_or_statement (parser, true); goto error; } @@ -29181,7 +29224,6 @@ c_parser_omp_metadirective (c_parser *parser, bool *if_p) if (i == 0) { error_at (loc, "expected directive name"); - c_parser_skip_to_end_of_block_or_statement (parser, true); goto error; } @@ -29390,9 +29432,9 @@ c_parser_omp_metadirective (c_parser *parser, bool *if_p) return; error: + /* Skip the metadirective pragma. Do not skip the metadirective body. */ if (parser->in_pragma) - c_parser_skip_to_pragma_eol (parser); - c_parser_skip_to_end_of_block_or_statement (parser, true); + c_parser_skip_to_pragma_eol (parser, false); } /* Main entry point to parsing most OpenMP pragmas. */ diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 3e0b58e3492..022e4db4650 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -51451,7 +51451,6 @@ cp_parser_omp_metadirective (cp_parser *parser, cp_token *pragma_tok, { error_at (match_loc, "too many %<otherwise%> or %<default%> " "clauses in %<metadirective%>"); - cp_parser_skip_to_end_of_block_or_statement (parser, true); goto fail; } else @@ -51461,14 +51460,12 @@ cp_parser_omp_metadirective (cp_parser *parser, cp_token *pragma_tok, { error_at (match_loc, "%<otherwise%> or %<default%> clause " "must appear last in %<metadirective%>"); - cp_parser_skip_to_end_of_block_or_statement (parser, true); goto fail; } if (!default_p && strcmp (p, "when") != 0) { error_at (match_loc, "%qs is not valid for %qs", p, "metadirective"); - cp_parser_skip_to_end_of_block_or_statement (parser, true); goto fail; } @@ -51537,7 +51534,6 @@ cp_parser_omp_metadirective (cp_parser *parser, cp_token *pragma_tok, if (i == 0) { error_at (loc, "expected directive name"); - cp_parser_skip_to_end_of_block_or_statement (parser, true); goto fail; } @@ -51756,11 +51752,8 @@ cp_parser_omp_metadirective (cp_parser *parser, cp_token *pragma_tok, return; fail: - /* Skip the metadirective pragma. */ + /* Skip the metadirective pragma. Do not skip the metadirective body. */ cp_parser_skip_to_pragma_eol (parser, pragma_tok); - - /* Skip the metadirective body. */ - cp_parser_skip_to_end_of_block_or_statement (parser, true); } diff --git a/gcc/testsuite/c-c++-common/gomp/declare-variant-2.c b/gcc/testsuite/c-c++-common/gomp/declare-variant-2.c index 7711dbc7bdd..f8f51431353 100644 --- a/gcc/testsuite/c-c++-common/gomp/declare-variant-2.c +++ b/gcc/testsuite/c-c++-common/gomp/declare-variant-2.c @@ -47,10 +47,9 @@ void f23 (void); #pragma omp declare variant (f1) match(construct={teams,parallel,master,for}) /* { dg-warning "unknown selector 'master' for context selector set 'construct'" } */ void f24 (void); #pragma omp declare variant (f1) match(construct={parallel(1 /* { dg-error "selector 'parallel' does not accept any properties" } */ -void f25 (void); /* { dg-error "expected '\\\}' before end of line" "" { target c++ } .-1 } */ - /* { dg-error "expected '\\\}' before '\\(' token" "" { target c } .-2 } */ +void f25 (void); /* { dg-error "expected '\\\}' before end of line" "" { target *-*-* } .-1 } */ #pragma omp declare variant (f1) match(construct={parallel(1)}) /* { dg-error "selector 'parallel' does not accept any properties" } */ -void f26 (void); /* { dg-error "expected '\\\}' before '\\(' token" "" { target c } .-1 } */ +void f26 (void); #pragma omp declare variant (f0) match(construct={simd(12)}) /* { dg-error "expected \[^\n\r]* clause before" } */ void f27 (void); /* { dg-error "'\\)' before numeric constant" "" { target c++ } .-1 } */ #pragma omp declare variant (f1) match(construct={parallel},construct={for}) /* { dg-error "selector set 'construct' specified more than once" } */ @@ -96,13 +95,13 @@ void f46 (void); #pragma omp declare variant (f1) match(implementation={vendor("foobar")}) /* { dg-warning "unknown property '.foobar.' of 'vendor' selector" } */ void f47 (void); #pragma omp declare variant (f1) match(implementation={unified_address(yes)}) /* { dg-error "selector 'unified_address' does not accept any properties" } */ -void f48 (void); /* { dg-error "expected '\\\}' before '\\(' token" "" { target c } .-1 } */ +void f48 (void); #pragma omp declare variant (f1) match(implementation={unified_shared_memory(no)}) /* { dg-error "selector 'unified_shared_memory' does not accept any properties" } */ -void f49 (void); /* { dg-error "expected '\\\}' before '\\(' token" "" { target c } .-1 } */ +void f49 (void); #pragma omp declare variant (f1) match(implementation={dynamic_allocators(42)}) /* { dg-error "selector 'dynamic_allocators' does not accept any properties" } */ -void f50 (void); /* { dg-error "expected '\\\}' before '\\(' token" "" { target c } .-1 } */ +void f50 (void); #pragma omp declare variant (f1) match(implementation={reverse_offload()}) /* { dg-error "selector 'reverse_offload' does not accept any properties" } */ -void f51 (void); /* { dg-error "expected '\\\}' before '\\(' token" "" { target c } .-1 } */ +void f51 (void); #pragma omp declare variant (f1) match(implementation={atomic_default_mem_order}) /* { dg-error "expected '\\(' before '\\\}' token" } */ void f52 (void); #pragma omp declare variant (f1) match(implementation={atomic_default_mem_order(acquire)}) diff --git a/gcc/testsuite/c-c++-common/gomp/metadirective-error-recovery.c b/gcc/testsuite/c-c++-common/gomp/metadirective-error-recovery.c new file mode 100644 index 00000000000..324228113c2 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/metadirective-error-recovery.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ + +/* This test used to ICE in C and only diagnose the first error in C++. */ + +struct s { + int a, b; +}; + +void f (int aa, int bb) +{ + struct s s1, s2; + s1.a = aa; + s1.b = bb; + s2.a = aa + 1; + s2.b = bb + 1; + + /* A struct is not a valid argument for the condition selector. */ + #pragma omp metadirective when(user={condition(s1)} : nothing) otherwise(nothing) /* { dg-error "property must be integer expression" } */ + #pragma omp metadirective when(user={condition(s2)} : nothing) otherwise(nothing) /* { dg-error "property must be integer expression" } */ +} -- 2.34.1