Hi! On Fri, 8 May 2015 14:24:15 +0300, Ilmir Usmanov <[email protected]> wrote: > On 06.05.2015 14:38, Thomas Schwinge wrote: > > On Tue, 5 May 2015 15:38:03 -0400, David Malcolm <[email protected]> > > wrote: > >> On Wed, 2015-04-29 at 14:10 +0200, Mikael Morin wrote: > >>> Le 29/04/2015 02:02, David Malcolm a écrit : > >>>> diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c > >>>> index 2c7c554..30e4eab 100644 > >>>> --- a/gcc/fortran/parse.c > >>>> +++ b/gcc/fortran/parse.c > >>>> @@ -4283,7 +4283,7 @@ parse_oacc_structured_block (gfc_statement acc_st) > >>>> unexpected_eof (); > >>>> else if (st != acc_end_st) > >>>> gfc_error ("Expecting %s at %C", gfc_ascii_statement > >>>> (acc_end_st)); > >>>> - reject_statement (); > >>>> + reject_statement (); > >>>> } > >>>> while (st != acc_end_st); > >>>> > >>> I think this one is a bug; there should be braces around 'gfc_error' and > >>> 'reject_statement'. > If 'st' is 'acc_end_st', as it shall be, the statement is rejected. So, > this is a bug. > > > > >>> At least that's the pattern in 'parse_oacc_loop', and how the > >>> 'unexpected_statement' function is used. > >> FWIW, Jeff had approved that patch, so I've committed the patch to trunk > >> (as r222823), making the indentation reflect the block structure. > >> > >> Thomas: should the > >> reject_statement (); > >> call in the above be guarded by the > >> else if (st != acc_end_st) > >> clause? > > Indeed, this seems to be a bug that has been introduced very early in the > > OpenACC Fortran front end development -- see how the > > parse_oacc_structured_block function evolved in the patches posted in > > <http://news.gmane.org/find-root.php?message_id=%3C52E1595D.9000007%40samsung.com%3E> > > and following (Ilmir, CCed "just in case"). I also see that the > > corresponding OpenMP code, parse_omp_structured_block, just calls > > unexpected_statement, which Ilmir's initial patch also did, but at some > > point, he then changed this to the current code: gfc_error followed by > > reject_statement, as cited above -- I would guess for the reason to get a > > better error message? (Tobias, should this thus also be done for OpenMP, > > and/or extend unexpected_statement accordingly?) > That's true. > I've checked abandoned openacc-1_0-branch and I used > unexpected_statement there (there still odd *_acc_* naming presents > instead of new-and-shiny *_oacc_* one), but, as you mentioned, I've > changed this for better error reporting... and introduced the bug. > > > > > And then, I'm a bit confused: is it "OK" that despite this presumed logic > > error, which affects all (?) valid executions of this parsing code, we're > > not running into any issues with the OpenACC Fortran front end test > > cases? > I think, this is OK, since this is an !$ACC END _smth_ statement and it > shall not present in the AST. So, it is abandoned later anyway ;) (if I > remember correctly, during gfc_clear_new_st call). Although the bug does > not affect the logic, it is still a bug. > > > OK for trunk? > From my point of view, OK.
Thanks for your review! Nobody else had any comments, so I have now
committed my patch to trunk in r226246:
commit 1ed4ddb2615c807c239e1b3eb214655a4cc87f1a
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Mon Jul 27 14:26:41 2015 +0000
Fix logic error in Fortran OpenACC parsing
gcc/fortran/
* parse.c (parse_oacc_structured_block): Fix logic error.
Reported by Mikael Morin <[email protected]>.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@226246
138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/fortran/ChangeLog | 5 +++++
gcc/fortran/parse.c | 6 ++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git gcc/fortran/ChangeLog gcc/fortran/ChangeLog
index 0ed6b9b..e5b7681 100644
--- gcc/fortran/ChangeLog
+++ gcc/fortran/ChangeLog
@@ -1,3 +1,8 @@
+2015-07-27 Thomas Schwinge <[email protected]>
+
+ * parse.c (parse_oacc_structured_block): Fix logic error.
+ Reported by Mikael Morin <[email protected]>.
+
2015-07-24 Mikael Morin <[email protected]>
PR fortran/64986
diff --git gcc/fortran/parse.c gcc/fortran/parse.c
index 45ad63f..04b4c80 100644
--- gcc/fortran/parse.c
+++ gcc/fortran/parse.c
@@ -4383,8 +4383,10 @@ parse_oacc_structured_block (gfc_statement acc_st)
if (st == ST_NONE)
unexpected_eof ();
else if (st != acc_end_st)
- gfc_error ("Expecting %s at %C", gfc_ascii_statement (acc_end_st));
- reject_statement ();
+ {
+ gfc_error ("Expecting %s at %C", gfc_ascii_statement (acc_end_st));
+ reject_statement ();
+ }
}
while (st != acc_end_st);
Grüße,
Thomas
pgp8lg16qnyQJ.pgp
Description: PGP signature
