On Fri, 28 Sep 2018 14:17:42 +0100 Julian Brown <jul...@codesourcery.com> wrote:
> On Wed, 26 Sep 2018 14:08:37 -0700 > Cesar Philippidis <ce...@codesourcery.com> wrote: > > > On 09/26/2018 12:50 PM, Joseph Myers wrote: > > > On Wed, 26 Sep 2018, Cesar Philippidis wrote: > > > > > >> Attached is an old patch which updated the C and C++ FEs to use > > >> %<)%> for the right ')' symbol. It's mostly a cosmetic change. > > >> All of the changes are self-contained to the OpenACC code > > >> path. > > > > > > Why is the "before ')'" included in the call to c_parser_error at > > > all? c_parser_error calls c_parse_error which adds its own " > > > before " and token description or expansion, so I'd expect the > > > current error to result in a message ending in something of the > > > form "before X before Y". > > > Julian, I need to start working on deep copy in OpenACC. Can you > > take over this patch? The error handling code in the C FE needs to > > be removed because it's dead. > > I agree that the error-handling path in question in the C FE is dead. > The difference is that in C, c_parser_oacc_wait_list parses the open > parenthesis, the list and then the close parenthesis separately, and > so a token sequence like: > > (1 > > will return an expression list of length 1. In the C++ FE rather, a > cp_parser_parenthesized_expression_list is parsed all in one go, and > if the input is not that well-formed sequence then NULL is returned > (or a zero-length vector for an empty list). > > But for C, it does not appear that c_parser_expr_list has a code path > that can return a zero-length list at all. So, we can elide the > diagnostic with no change to compiler behaviour. This patch does that, > and also changes the C++ diagnostic, leading to errors being reported > like: > > diag.c: In function 'int main(int, char*)': > diag.c:6:59: error: expected ')' before end of line > 6 | #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait (1 > | ~ ^ > | ) > diag.c:6:59: error: expected integer expression list before end of > line > > Actually I'm not too sure how useful the second error line is. Maybe > we should just remove it to improve consistency between C & C++? > > The attached has been tested with offloading to nvptx and > bootstrapped. OK? Ping? Thanks, Julian