| Message ID | 55540130.5080206@charter.net |
|---|---|
| State | New |
| Headers | show |
* ping * On 05/13/2015 06:58 PM, Jerry DeLisle wrote: > The attached patch reverts a change I made for pr65456 which caused this > regression and adds a check for quotes farther down in the function. This > avoids treating a '!' in a string as a comment and wiping the rest of the line. > > I found the added code is needed because an interposing quote was falling > through and being changed into an empty space at around line 1393. (I do not > recall the history of why that space is being done) > > The patch also updates test case continuation_13.f90. I found another compiler I > use for comparison interprets the continuation differently and is inserting a > space that is not there. So the format at line 800 is handled differently now > with gfortran and the line 900 format example is added to test both cases with > and without a space just before the continuation. > > I think now gfortran has this right, but I do not rule out that I am missing > some obscure rule. This begs a question about the space character substituted > near line 1393 in scanner.c, but at the moment I do not think it is related. > > I have also added a test for the the original problem reported in this PR to > avoid future repeating of the problem. I will provide a Changelog entry for the > testsuite changes. > > Regression tested on x86-64-linux. OK to trunk? followed to 5.1? > > Regards, > > Jerry > > > 2015-05-14 Jerry DeLisle <jvdelisle@gcc.gnu.org> > > PR fortran/65903 > * io.c (format_lex): Change to NONSTRING when checking for > possible doubled quote. > * scanner.c (gfc_next_char_literal): Revert change from 64506, > add a check for quotes, and return. >
On Sat, May 16, 2015 at 07:52:38AM -0700, Jerry DeLisle wrote: > * ping * > > > 2015-05-14 Jerry DeLisle <jvdelisle@gcc.gnu.org> > > > > PR fortran/65903 > > * io.c (format_lex): Change to NONSTRING when checking for > > possible doubled quote. > > * scanner.c (gfc_next_char_literal): Revert change from 64506, > > add a check for quotes, and return. > > OK.
Le 14/05/2015 03:58, Jerry DeLisle a écrit : > The attached patch reverts a change I made for pr65456 which caused this > regression and adds a check for quotes farther down in the function. This > avoids treating a '!' in a string as a comment and wiping the rest of the line. > > I found the added code is needed because an interposing quote was falling > through and being changed into an empty space at around line 1393. (I do not > recall the history of why that space is being done) > > The patch also updates test case continuation_13.f90. I found another compiler I > use for comparison interprets the continuation differently and is inserting a > space that is not there. So the format at line 800 is handled differently now > with gfortran and the line 900 format example is added to test both cases with > and without a space just before the continuation. > > I think now gfortran has this right, but I do not rule out that I am missing > some obscure rule. This begs a question about the space character substituted > near line 1393 in scanner.c, but at the moment I do not think it is related. > > I have also added a test for the the original problem reported in this PR to > avoid future repeating of the problem. I will provide a Changelog entry for the > testsuite changes. > > Regression tested on x86-64-linux. OK to trunk? followed to 5.1? > > Regards, > > Jerry > > > 2015-05-14 Jerry DeLisle <jvdelisle@gcc.gnu.org> > > PR fortran/65903 > * io.c (format_lex): Change to NONSTRING when checking for > possible doubled quote. > * scanner.c (gfc_next_char_literal): Revert change from 64506, > add a check for quotes, and return. > > Index: gcc/testsuite/gfortran.dg/continuation_13.f90 > =================================================================== > --- gcc/testsuite/gfortran.dg/continuation_13.f90 (revision 223105) > +++ gcc/testsuite/gfortran.dg/continuation_13.f90 (working copy) > @@ -19,6 +19,8 @@ character(25) :: astring > ) > 800 format('This is actually ok.'& !comment > ' end' ) > +900 format('This is actually ok.' & !comment > + ' end' ) > write(astring,100) > if (astring.ne."This format is OK.") call abort > write(astring,200) Hello, is the new format labelled 900 correct? It seems to me it is equivalent to 900 format('This is actually ok.' ' end') which is missing a comma, no? This is a real question, I'm no format guru. By the way, the existing 800 format is also not right, it shouldn't have a comment, and it should have a '&' at the beginning of the next line. Section 3.3.2.4 "Free form statement continuation", last paragraph has: > If a character context is to be continued, an “&” shall be the last > nonblank character on the line and shall not be followed by commentary. > There shall be a later line that is not a comment; an “&” shall be the > first nonblank character on the next such line and the statement > continues with the next character following that “&”. To be honest, I see little value in supporting continuation between the two consecutive quotes coding one quote inside a string constant, it's confusing. But if it's supported, the above applies, doesn't it? Regarding the patch itself, I haven't looked at it in great details yet, but as it's a revert basically, it shouldn't do a lot of harm. I see that the fixed form part isn't reverted, is it on purpose? Mikael
On 05/16/2015 08:17 AM, Mikael Morin wrote: ---- snip ---- >> >> Index: gcc/testsuite/gfortran.dg/continuation_13.f90 >> =================================================================== >> --- gcc/testsuite/gfortran.dg/continuation_13.f90 (revision 223105) >> +++ gcc/testsuite/gfortran.dg/continuation_13.f90 (working copy) >> @@ -19,6 +19,8 @@ character(25) :: astring >> ) >> 800 format('This is actually ok.'& !comment >> ' end' ) >> +900 format('This is actually ok.' & !comment >> + ' end' ) >> write(astring,100) >> if (astring.ne."This format is OK.") call abort >> write(astring,200) > > Hello, is the new format labelled 900 correct? > It seems to me it is equivalent to > 900 format('This is actually ok.' ' end') > which is missing a comma, no? > This is a real question, I'm no format guru. Yes, I agree one ought to use a comma. See below. > > By the way, the existing 800 format is also not right, it shouldn't have > a comment, and it should have a '&' at the beginning of the next line. > Section 3.3.2.4 "Free form statement continuation", last paragraph has: > >> If a character context is to be continued, an “&” shall be the last >> nonblank character on the line and shall not be followed by commentary. >> There shall be a later line that is not a comment; an “&” shall be the >> first nonblank character on the next such line and the statement >> continues with the next character following that “&”. The "right" way is to put the "&" at the beginning of the next line. It is common practice to allow this as an extension and it is assumed to begin at the first non-space character encountered. This does require special handling of continued literal strings. This is why we try to give errors with -std=f95, etc. $ gfc -std=f95 continuation_13.f90 continuation_13.f90:24:10: ' end' ) 1 Error: GNU Extension: Missing comma at (1) continuation_13.f90:41:17: write(astring,900) 1 Error: FORMAT label 900 at (1) not defined I think these are cases where even the error detection is not perfect. User beware. > > To be honest, I see little value in supporting continuation between the > two consecutive quotes coding one quote inside a string constant, it's > confusing. But if it's supported, the above applies, doesn't it? > > > Regarding the patch itself, I haven't looked at it in great details yet, > but as it's a revert basically, it shouldn't do a lot of harm. > I see that the fixed form part isn't reverted, is it on purpose? Fixed form continuations are handled differently and I am not trying to address issues there, if any. As I have time, I will dabble further into this. Getting ready to move here from Southern California back to Washington State. Regards, Jerry
On 05/16/2015 08:11 AM, Steve Kargl wrote: > On Sat, May 16, 2015 at 07:52:38AM -0700, Jerry DeLisle wrote: >> * ping * >> >>> 2015-05-14 Jerry DeLisle <jvdelisle@gcc.gnu.org> >>> >>> PR fortran/65903 >>> * io.c (format_lex): Change to NONSTRING when checking for >>> possible doubled quote. >>> * scanner.c (gfc_next_char_literal): Revert change from 64506, >>> add a check for quotes, and return. >>> > > OK. > Thanks Steve, Committed revision 223248.
Index: gcc/fortran/io.c =================================================================== --- gcc/fortran/io.c (revision 223105) +++ gcc/fortran/io.c (working copy) @@ -385,7 +385,7 @@ format_lex (void) if (c == delim) { - c = next_char (INSTRING_NOWARN); + c = next_char (NONSTRING); if (c == '\0') { Index: gcc/fortran/scanner.c =================================================================== --- gcc/fortran/scanner.c (revision 223105) +++ gcc/fortran/scanner.c (working copy) @@ -1272,21 +1272,11 @@ restart: are still in a string and we are looking for a possible doubled quote and we end up here. See PR64506. */ - if (in_string) + if (in_string && c != '\n') { gfc_current_locus = old_loc; - - if (c == '!') - { - skip_comment_line (); - goto restart; - } - - if (c != '\n') - { - c = '&'; - goto done; - } + c = '&'; + goto done; } if (c != '!' && c != '\n') @@ -1392,6 +1382,8 @@ restart: "Missing %<&%> in continued character " "constant at %C"); } + else if (!in_string && (c == '\'' || c == '"')) + goto done; /* Both !$omp and !$ -fopenmp continuation lines have & on the continuation line only optionally. */ else if (openmp_flag || openacc_flag || openmp_cond_flag) Index: gcc/testsuite/gfortran.dg/continuation_13.f90 =================================================================== --- gcc/testsuite/gfortran.dg/continuation_13.f90 (revision 223105) +++ gcc/testsuite/gfortran.dg/continuation_13.f90 (working copy) @@ -19,6 +19,8 @@ character(25) :: astring ) 800 format('This is actually ok.'& !comment ' end' ) +900 format('This is actually ok.' & !comment + ' end' ) write(astring,100) if (astring.ne."This format is OK.") call abort write(astring,200) @@ -34,6 +36,8 @@ if (astring.ne."This format now works.'") call abo write(astring,700) if (astring.ne."This format now works.'") call abort write(astring,800) +if (astring.ne."This is actually ok.' end") call abort +write(astring,900) if (astring.ne."This is actually ok. end") call abort end