Re: [patch,openacc] C, C++ OpenACC wait diagnostic change

[Julian Brown <julian at codesourcery dot com>]

On Fri, 28 Sep 2018 14:17:42 +0100
Julian Brown <julian@codesourcery.com> wrote:

> On Wed, 26 Sep 2018 14:08:37 -0700
> Cesar Philippidis <cesar@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