Re: [patch, libgfortran] PR114304 - [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"
Hi Jerry, hello world, Jerry D wrote: On 4/5/24 10:47 AM, Jerry D wrote: On 4/4/24 2:41 PM, Tobias Burnus wrote: I think for the current testcases, I like the patch – the question is only what's about: ',3' as input for 'comma' (or '.3' as input for 'point') [...] But for 'comma': [...] * GCC with your patch: Same result: ios != 0 and nothing read. Expected: [...] read-in value is 0.3. [...] See attached updated patch. Regressions tested on x86-64. OK for trunk and 13 after a bit. OK. Thanks for the patch! Tobias
Re: [patch, libgfortran] PR114304 - [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"
On 4/5/24 10:47 AM, Jerry D wrote:
On 4/4/24 2:41 PM, Tobias Burnus wrote:
Hi Jerry,
I think for the current testcases, I like the patch – the question is
only what's about:
',3' as input for 'comma' (or '.3' as input for 'point')
For 'point' – 0.3 is read and ios = 0 (as expected)
But for 'comma':
* GCC 12 reads nothing and has ios = 0.
* GCC 13/mainline has an error (ios != 0 – and reads nothing)
* GCC with your patch: Same result: ios != 0 and nothing read.
Expected: Same as with ','/'comma' – namely: read-in value is 0.3.
→ https://godbolt.org/z/4rc8fz4sT for the full example, which works
with ifort, ifx and flang
* * *
Can you check and fix this? It looks perfectly valid to me to have
remove the '0' in the floating point numbers '0.3' or '0,3' seems to
be permitted – and it works for '.' (with 'point') but not for ','
(with 'comma').
Yes, I found the spot to fix this.
F2023's "13.10.3.1 List-directed input forms" refers to "13.7.2.3.2 F
editing", which states:
"The standard form of the input field [...] The form of the mantissa
is an optional sign, followed by a string of one or more digits
optionally containing a decimal symbol."
The latter does not require that the digit has to be before the
decimal sign and as for output, it is optional, it is surely intended
that ",3" is a valid floating-point number for decimal='comma'.
Agree
* * *
I extended the testcase to check for this – see attached diff. All
'point' work, all 'comma' fail.
Thanks for working on this!
Tobias
Thanks much. After I fix it, I will use your extended test case in the
test suite.
Jerry -
See attached updated patch.
Regressions tested on x86-64. OK for trunk and 13 after a bit.
Jerry -
commit 690b9fa57d95796ba0e92a172e1490601f25e03a
Author: Jerry DeLisle
Date: Fri Apr 5 19:25:13 2024 -0700
libfortran: Fix handling of formatted separators.
PR libfortran/114304
PR libfortran/105473
libgfortran/ChangeLog:
* io/list_read.c (eat_separator): Add logic to handle spaces
preceding a comma or semicolon such that that a 'null' read
occurs without error at the end of comma or semicolon
terminated input lines. Add check and error message for ';'.
(list_formatted_read_scalar): Treat comma as a decimal point
when specified by the decimal mode on the first item.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr105473.f90: Modify to verify new error message.
* gfortran.dg/pr114304.f90: New test.
diff --git a/gcc/testsuite/gfortran.dg/pr105473.f90 b/gcc/testsuite/gfortran.dg/pr105473.f90
index 2679f6bb447..863a312c794 100644
--- a/gcc/testsuite/gfortran.dg/pr105473.f90
+++ b/gcc/testsuite/gfortran.dg/pr105473.f90
@@ -9,11 +9,11 @@
n = 999; m = 777; r=1.2345
z = cmplx(0.0,0.0)
-! Check that semi-colon is allowed as separator with decimal=point.
+! Check that semi-colon is not allowed as separator with decimal=point.
ios=0
testinput = '1;17;3.14159'
read(testinput,*,decimal='point',iostat=ios) n, m, r
- if (ios /= 0) stop 1
+ if (ios /= 5010) stop 1
! Check that semi-colon allowed as a separator with decimal=point.
ios=0
diff --git a/gcc/testsuite/gfortran.dg/pr114304.f90 b/gcc/testsuite/gfortran.dg/pr114304.f90
new file mode 100644
index 000..2f913f1ab34
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr114304.f90
@@ -0,0 +1,114 @@
+! { dg-do run }
+!
+! PR fortran/114304
+!
+! See also PR fortran/105473
+!
+! Testing: Does list-directed reading an integer/real allow some non-integer input?
+!
+! Note: GCC result comments before fix of this PR.
+
+ implicit none
+ call t(.true., 'comma', ';') ! No error shown
+ call t(.false., 'point', ';') ! /!\ gfortran: no error, others: error
+ call t(.false., 'comma', ',') ! Error shown
+ call t(.true., 'point', ',') ! No error shown
+ call t(.false., 'comma', '.') ! Error shown
+ call t(.false., 'point', '.') ! Error shown
+ call t(.false., 'comma', '5.') ! Error shown
+ call t(.false., 'point', '5.') ! gfortran/flang: Error shown, ifort: no error
+ call t(.false., 'comma', '5,') ! gfortran: error; others: no error
+ call t(.true., 'point', '5,') ! No error shown
+ call t(.true., 'comma', '5;') ! No error shown
+ call t(.false., 'point', '5;') ! /!\ gfortran: no error shown, others: error
+ call t(.true., 'comma', '7 .') ! No error shown
+ call t(.true., 'point', '7 .') ! No error shown
+ call t(.true., 'comma', '7 ,') ! /!\ gfortran: error; others: no error
+ call t(.true., 'point', '7 ,') ! No error shown
+ call t(.true., 'comma', '7 ;') ! No error shown
+ call t(.true., 'point', '7 ;') ! No error shown
+
+! print *, '---'
+
+ call t(.false., 'comma', '8.', .true.) ! Error shown
+ call t(.true., 'point', '8.', .true.) ! gfortran/flang: Error shown, ifort: no error
+ call t(.true., 'comma', '8,', .true.) ! gfortran: error; others
Re: [patch, libgfortran] PR114304 - [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"
On 4/4/24 2:41 PM, Tobias Burnus wrote: Hi Jerry, I think for the current testcases, I like the patch – the question is only what's about: ',3' as input for 'comma' (or '.3' as input for 'point') For 'point' – 0.3 is read and ios = 0 (as expected) But for 'comma': * GCC 12 reads nothing and has ios = 0. * GCC 13/mainline has an error (ios != 0 – and reads nothing) * GCC with your patch: Same result: ios != 0 and nothing read. Expected: Same as with ','/'comma' – namely: read-in value is 0.3. → https://godbolt.org/z/4rc8fz4sT for the full example, which works with ifort, ifx and flang * * * Can you check and fix this? It looks perfectly valid to me to have remove the '0' in the floating point numbers '0.3' or '0,3' seems to be permitted – and it works for '.' (with 'point') but not for ',' (with 'comma'). Yes, I found the spot to fix this. F2023's "13.10.3.1 List-directed input forms" refers to "13.7.2.3.2 F editing", which states: "The standard form of the input field [...] The form of the mantissa is an optional sign, followed by a string of one or more digits optionally containing a decimal symbol." The latter does not require that the digit has to be before the decimal sign and as for output, it is optional, it is surely intended that ",3" is a valid floating-point number for decimal='comma'. Agree * * * I extended the testcase to check for this – see attached diff. All 'point' work, all 'comma' fail. Thanks for working on this! Tobias Thanks much. After I fix it, I will use your extended test case in the test suite. Jerry -
Re: [patch, libgfortran] PR114304 - [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"
Hi Jerry,
I think for the current testcases, I like the patch – the question is
only what's about:
',3' as input for 'comma' (or '.3' as input for 'point')
For 'point' – 0.3 is read and ios = 0 (as expected)
But for 'comma':
* GCC 12 reads nothing and has ios = 0.
* GCC 13/mainline has an error (ios != 0 – and reads nothing)
* GCC with your patch: Same result: ios != 0 and nothing read.
Expected: Same as with ','/'comma' – namely: read-in value is 0.3.
→ https://godbolt.org/z/4rc8fz4sT for the full example, which works with
ifort, ifx and flang
* * *
Can you check and fix this? It looks perfectly valid to me to have
remove the '0' in the floating point numbers '0.3' or '0,3' seems to be
permitted – and it works for '.' (with 'point') but not for ',' (with
'comma').
F2023's "13.10.3.1 List-directed input forms" refers to "13.7.2.3.2 F
editing", which states:
"The standard form of the input field [...] The form of the mantissa is
an optional sign, followed by a string of one or more digits optionally
containing a decimal symbol."
The latter does not require that the digit has to be before the decimal
sign and as for output, it is optional, it is surely intended that ",3"
is a valid floating-point number for decimal='comma'.
* * *
I extended the testcase to check for this – see attached diff. All
'point' work, all 'comma' fail.
Thanks for working on this!
Tobiasdiff --git a/gcc/testsuite/gfortran.dg/pr114304.f90 b/gcc/testsuite/gfortran.dg/pr114304.f90
index 8344a9ea857..2bcf9bc7f57 100644
--- a/gcc/testsuite/gfortran.dg/pr114304.f90
+++ b/gcc/testsuite/gfortran.dg/pr114304.f90
@@ -70,7 +70,25 @@
call t(.true., 'point', '4,4 ,', .true.)
call t(.true., 'comma', '4;4 ;', .true.)
call t(.true., 'point', '4,4 ;', .true.)
+
+ call t2('comma', ',2')
+ call t2('point', '.2')
+ call t2('comma', ',2;')
+ call t2('point', '.2,')
+ call t2('comma', ',2 ,')
+ call t2('point', '.2 .')
contains
+subroutine t2(dec, testinput)
+ character(*) :: dec, testinput
+ integer ios
+ real :: r
+ r = 42
+ read(testinput,*,decimal=dec,iostat=ios) r
+ if (ios /= 0 .or. abs(r - 0.2) > epsilon(r)) then
+print '(*(g0))', dec, ', testinput = "',testinput,'"',', r=',r,' ios=',ios
+stop 3
+ end if
+end
subroutine t(valid, dec, testinput, isreal)
logical, value :: valid
character(len=*) :: dec, testinput
Re: [patch, libgfortran] PR114304 - [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"
On 4/4/24 2:31 AM, Tobias Burnus wrote:
Hi Jerry,
--- snip ---
The patch looks mostly like I would expect, except for decimal='point'
and a ';' which is *not* preceded by a space.
Thanks for working on it.
Regarding the 'except' case:
--- snip ---
i.e. for the following string, I had *expected an error*:
point, isreal = F , testinput = ";"n= 42 ios= 0
point, isreal = F , testinput = "5;"n= 5 ios= 0
point, isreal = T , testinput = "8;"r= 8. ios= 0
point, isreal = T , testinput = "3.3;"r= 3.2995 ios= 0
point, isreal = T , testinput = "3,3;"r= 3. ios= 0
--- snip ---
I have attached the modified testcase linked above; consider adding it
as well. - Changes to the one of the attachment:
- I added a few additional (albeit boring) tests
- I added an expected output + error diagnostic.
The testcase assumes an error for ';' as separator (with 'point'),
unless there is a space before it.
--- snip ---
Here attached is the revised patch. I replaced the new test case I had
with the one you provided modified and it now passes.
I modified the test case pr105473.f90 to expect the error on semicolon.
Regression tested on X86_64. OK for trunk and a bit later back port to 13?
Cheers,
Jerry
commit 9c8318cd8703d49ad7563e89765f8849ebc14385
Author: Jerry DeLisle
Date: Thu Apr 4 13:48:20 2024 -0700
libfortran: Fix handling of formatted separators.
PR libfortran/114304
PR libfortran/105473
libgfortran/ChangeLog:
* io/list_read.c (eat_separator): Add logic to handle spaces
preceding a comma or semicolon such that that a 'null' read
occurs without error at the end of comma or semicolon
terminated input lines. Add check and error message for ';'.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr105473.f90: Modify to verify new error message.
* gfortran.dg/pr114304.f90: New test.
diff --git a/gcc/testsuite/gfortran.dg/pr105473.f90 b/gcc/testsuite/gfortran.dg/pr105473.f90
index 2679f6bb447..863a312c794 100644
--- a/gcc/testsuite/gfortran.dg/pr105473.f90
+++ b/gcc/testsuite/gfortran.dg/pr105473.f90
@@ -9,11 +9,11 @@
n = 999; m = 777; r=1.2345
z = cmplx(0.0,0.0)
-! Check that semi-colon is allowed as separator with decimal=point.
+! Check that semi-colon is not allowed as separator with decimal=point.
ios=0
testinput = '1;17;3.14159'
read(testinput,*,decimal='point',iostat=ios) n, m, r
- if (ios /= 0) stop 1
+ if (ios /= 5010) stop 1
! Check that semi-colon allowed as a separator with decimal=point.
ios=0
diff --git a/gcc/testsuite/gfortran.dg/pr114304.f90 b/gcc/testsuite/gfortran.dg/pr114304.f90
new file mode 100644
index 000..8344a9ea857
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr114304.f90
@@ -0,0 +1,101 @@
+! { dg-do run }
+!
+! PR fortran/114304
+!
+! See also PR fortran/105473
+!
+! Testing: Does list-directed reading an integer/real allow some non-integer input?
+!
+! Note: GCC result comments before fix of this PR.
+
+ implicit none
+ call t(.true., 'comma', ';') ! No error shown
+ call t(.false., 'point', ';') ! /!\ gfortran: no error, others: error
+ call t(.false., 'comma', ',') ! Error shown
+ call t(.true., 'point', ',') ! No error shown
+ call t(.false., 'comma', '.') ! Error shown
+ call t(.false., 'point', '.') ! Error shown
+ call t(.false., 'comma', '5.') ! Error shown
+ call t(.false., 'point', '5.') ! gfortran/flang: Error shown, ifort: no error
+ call t(.false., 'comma', '5,') ! gfortran: error; others: no error
+ call t(.true., 'point', '5,') ! No error shown
+ call t(.true., 'comma', '5;') ! No error shown
+ call t(.false., 'point', '5;') ! /!\ gfortran: no error shown, others: error
+ call t(.true., 'comma', '7 .') ! No error shown
+ call t(.true., 'point', '7 .') ! No error shown
+ call t(.true., 'comma', '7 ,') ! /!\ gfortran: error; others: no error
+ call t(.true., 'point', '7 ,') ! No error shown
+ call t(.true., 'comma', '7 ;') ! No error shown
+ call t(.true., 'point', '7 ;') ! No error shown
+
+! print *, '---'
+
+ call t(.false., 'comma', '8.', .true.) ! Error shown
+ call t(.true., 'point', '8.', .true.) ! gfortran/flang: Error shown, ifort: no error
+ call t(.true., 'comma', '8,', .true.) ! gfortran: error; others: no error
+ call t(.true., 'point', '8,', .true.) ! No error shown
+ call t(.true., 'comma', '8;', .true.) ! No error shown
+ call t(.false., 'point', '8;', .true.) ! /!\ gfortran: no error shown, others: error
+ call t(.true., 'comma', '9 .', .true.) ! No error shown
+ call t(.true., 'point', '9 .', .true.) ! No error shown
+ call t(.true., 'comma', '9 ,', .true.) ! /!\ gfortran: error; others: no error
+ call t(.true., 'point', '9 ,', .true.) ! No error shown
+ call t(.true., 'comma', '9 ;', .true.) ! No error shown
+ call t(.true., 'point'
Re: [patch, libgfortran] PR114304 - [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"
On 4/4/24 2:31 AM, Tobias Burnus wrote: Hi Jerry, Jerry D wrote: The attached log entry and patch (git show) fixes this issue by adding logic to handle spaces in eat_separators. One or more spaces by themselves are a valid separator. So in this case we look at the character following the spaces to see if it is a comma or semicolon. If so, I change it to the valid separator for the given decimal mode, point or comma. This allows the comma or semicolon to be interpreted as a null read on the next effective item in the formatted read. I chose a permissive approach here that allows reads to proceed when the input line is mal-formed with an incorrect separator as long as there is at least one space in front of it. First: Consider also adding 'PR fortran/105473' to the commit log as the PRs are closely related, albeit this PR is different- The patch looks mostly like I would expect, except for decimal='point' and a ';' which is *not* preceded by a space. Thanks for working on it. Regarding the 'except' case: * * * If I try your patch with the testcase of at comment 19, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114304#c19 → https://gcc.gnu.org/bugzilla/attachment.cgi?id=57695 , I do note that with 'decimal=point', a tailing semicolon is silently accepted – even if not proceeded by a space. I did that on purpose out of leniency and fear of breaking something. I will add that in and do some testing. I think such code is invalid – and you could consider to reject it. Otherwise, the handling all seems to be in line with the Fortran spec. i.e. for the following string, I had *expected an error*: I will see what it does. I agree in principle. point, isreal = F , testinput = ";"n= 42 ios= 0 point, isreal = F , testinput = "5;"n= 5 ios= 0 point, isreal = T , testinput = "8;"r= 8. ios= 0 point, isreal = T , testinput = "3.3;"r= 3.2995 ios= 0 point, isreal = T , testinput = "3,3;"r= 3. ios= 0 while I think the following is OK (i.e. no error is what I expect) due to the the space before the ';'. Agree and this is what I was attempting. point, isreal = F , testinput = "7 ;"n= 7 ios= 0 point, isreal = T , testinput = "9 ;"r= 9. ios= 0 point, isreal = T , testinput = "4.4 ;"r= 4.4010 ios=0 point, isreal = T , testinput = "9 ;"r= 9. ios= 0 point, isreal = T , testinput = "4,4 ;"r= 4. ios= 0 * * * Looking at the other compilers, ifort, ifx and Flang do issue an error here. Likewise, g95 seems to yield an error in this case (see below). I do note that the Lapack testcase that triggered this PR did have such a code - but it was then changed because g95 did not like it: https://github.com/Reference-LAPACK/lapack/commit/64e8a7500d817869e5fcde35afd39af8bc7a8086 I am glad they fixed that, it triggered a whole lot of cycles here. In terms of gfortran: until recently did accept it (all versions, including 13+14); it then rejected it due to the change in PR105473 (GCC 14/mainline, backported to 13)– but I now think it rightly did so. With the current patch, it is accepted again. * * * I have attached the modified testcase linked above; consider adding it as well. - Changes to the one of the attachment: - I added a few additional (albeit boring) tests - I added an expected output + error diagnostic. The testcase assumes an error for ';' as separator (with 'point'), unless there is a space before it. [If we want to not diagnose this as vendor extension, we really need to add a comment to that testcase besides changing valid = .false. to .true.] I have gone back and forth on this issue many times trying to find the middle ground. The standard is fairly clear. To me it is on the user to not use malformed input so allowing with the intervening space we are simply ignoring the trailing ',' or ';' and calling the spaces sufficient as a valid separator. Regardless I now have your modified test case passing. Much appreciated. Thanks for the reviews. I will resubmit when I can, hopefully today. Cheers, Jerry
Re: [patch, libgfortran] PR114304 - [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"
Hi Jerry,
Jerry D wrote:
The attached log entry and patch (git show) fixes this issue by adding
logic to handle spaces in eat_separators. One or more spaces by
themselves are a valid separator. So in this case we look at the
character following the spaces to see if it is a comma or semicolon.
If so, I change it to the valid separator for the given decimal mode,
point or comma. This allows the comma or semicolon to be interpreted as
a null read on the next effective item in the formatted read.
I chose a permissive approach here that allows reads to proceed when the
input line is mal-formed with an incorrect separator as long as there is
at least one space in front of it.
First: Consider also adding 'PR fortran/105473' to the commit log
as the PRs are closely related, albeit this PR is different-
The patch looks mostly like I would expect, except for decimal='point'
and a ';' which is *not* preceded by a space.
Thanks for working on it.
Regarding the 'except' case:
* * *
If I try your patch with the testcase of at comment 19,
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114304#c19
→ https://gcc.gnu.org/bugzilla/attachment.cgi?id=57695 ,
I do note that with 'decimal=point', a tailing semicolon is silently
accepted – even if not proceeded by a space.
I think such code is invalid – and you could consider to reject it.
Otherwise, the handling all seems to be in line with the Fortran spec.
i.e. for the following string, I had *expected an error*:
point, isreal = F , testinput = ";"n= 42 ios= 0
point, isreal = F , testinput = "5;"n= 5 ios= 0
point, isreal = T , testinput = "8;"r= 8. ios= 0
point, isreal = T , testinput = "3.3;"r= 3.2995 ios= 0
point, isreal = T , testinput = "3,3;"r= 3. ios= 0
while I think the following is OK (i.e. no error is what I expect) due
to the the space before the ';'.
point, isreal = F , testinput = "7 ;"n= 7 ios= 0
point, isreal = T , testinput = "9 ;"r= 9. ios= 0
point, isreal = T , testinput = "4.4 ;"r= 4.4010 ios=0
point, isreal = T , testinput = "9 ;"r= 9. ios= 0
point, isreal = T , testinput = "4,4 ;"r= 4. ios= 0
* * *
Looking at the other compilers, ifort, ifx and Flang do issue an error
here. Likewise, g95 seems to yield an error in this case (see below).
I do note that the Lapack testcase that triggered this PR did have such
a code - but it was then changed because g95 did not like it:
https://github.com/Reference-LAPACK/lapack/commit/64e8a7500d817869e5fcde35afd39af8bc7a8086
In terms of gfortran: until recently did accept it (all versions,
including 13+14); it then rejected it due to the change in PR105473 (GCC
14/mainline, backported to 13)– but I now think it rightly did so. With
the current patch, it is accepted again.
* * *
I have attached the modified testcase linked above; consider adding it
as well. - Changes to the one of the attachment:
- I added a few additional (albeit boring) tests
- I added an expected output + error diagnostic.
The testcase assumes an error for ';' as separator (with 'point'),
unless there is a space before it.
[If we want to not diagnose this as vendor extension, we really need to
add a comment to that testcase besides changing valid = .false. to .true.]
Tobias! { dg-do run }
!
! PR fortran/114304
!
! See also PR fortran/105473
!
! Testing: Does list-directed reading an integer/real allows some non-integer input?
!
! Note: GCC result comments before fix of this PR.
implicit none
call t(.true., 'comma', ';') ! No error shown
call t(.false., 'point', ';') ! /!\ gfortran: no error, others: error
call t(.false., 'comma', ',') ! Error shown
call t(.true., 'point', ',') ! No error shown
call t(.false., 'comma', '.') ! Error shown
call t(.false., 'point', '.') ! Error shown
call t(.false., 'comma', '5.') ! Error shown
call t(.false., 'point', '5.') ! gfortran/flang: Error shown, ifort: no error
call t(.false., 'comma', '5,') ! gfortran: error; others: no error
call t(.true., 'point', '5,') ! No error shown
call t(.true., 'comma', '5;') ! No error shown
call t(.false., 'point', '5;') ! /!\ gfortran: no error shown, others: error
call t(.true., 'comma', '7 .') ! No error shown
call t(.true., 'point', '7 .') ! No error shown
call t(.true., 'comma', '7 ,') ! /!\ gfortran: error; others: no error
call t(.true., 'point', '7 ,') ! No error shown
call t(.true., 'comma', '7 ;') ! No error shown
call t(.true., 'point', '7 ;') ! No error shown
! print *, '---'
call t(.false., 'comma', '8.', .true.) ! Error shown
call t(.true., 'point', '8.', .true.) ! gfortran/flang: Error shown, ifort: no error
call t(.true., 'comma', '8,', .true.) ! gfortran: error; others: no error
call t(.true., 'point', '8,', .true.) ! No error shown
call t(.true., 'comma', '8;', .true.) ! No erro
Re: [patch, libgfortran] PR114304 - [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"
Hi Jerry, It looks good to me. Noting that this is not a regression, OK for mainline on condition that you keep a sharp eye out for any associated problems. Likewise with backporting to 13-branch. Thanks Paul On Thu, 4 Apr 2024 at 02:34, Jerry D wrote: > Hi all, > > The attached log entry and patch (git show) fixes this issue by adding > logic to handle spaces in eat_separators. One or more spaces by > themselves are a valid separator. So in this case we look at the > character following the spaces to see if it is a comma or semicolon. > > If so, I change it to the valid separator for the given decimal mode, > point or comma. This allows the comma or semicolon to be interpreted as > a null read on the next effective item in the formatted read. > > I chose a permissive approach here that allows reads to proceed when the > input line is mal-formed with an incorrect separator as long as there is > at least one space in front of it. > > New test case included. Regression tested on X86-64. > > OK for trunk? Backport to 13 after some time. > > Regards, > > Jerry
[patch, libgfortran] PR114304 - [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"
Hi all,
The attached log entry and patch (git show) fixes this issue by adding
logic to handle spaces in eat_separators. One or more spaces by
themselves are a valid separator. So in this case we look at the
character following the spaces to see if it is a comma or semicolon.
If so, I change it to the valid separator for the given decimal mode,
point or comma. This allows the comma or semicolon to be interpreted as
a null read on the next effective item in the formatted read.
I chose a permissive approach here that allows reads to proceed when the
input line is mal-formed with an incorrect separator as long as there is
at least one space in front of it.
New test case included. Regression tested on X86-64.
OK for trunk? Backport to 13 after some time.
Regards,
Jerrycommit 7d1a958d6b099ea88b6c51649baf5dbd5e598909
Author: Jerry DeLisle
Date: Wed Apr 3 18:07:30 2024 -0700
libfortran: Fix handling of formatted separators.
PR libfortran/114304
libgfortran/ChangeLog:
* io/list_read.c (eat_separator): Add logic to handle spaces
preceding a comma or semicolon such that that a 'null' read
occurs without error at the end of comma or semicolon
terminated input lines.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr114304.f90: New test.
diff --git a/gcc/testsuite/gfortran.dg/pr114304.f90 b/gcc/testsuite/gfortran.dg/pr114304.f90
new file mode 100644
index 000..57af619246b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr114304.f90
@@ -0,0 +1,49 @@
+! { dg-do run }
+! pr114304
+real :: x(3)
+character(len=1) :: s
+integer :: ios
+
+s = 'x'
+
+open(99, decimal="comma", status='scratch')
+write(99, '(a)') '1,23435 1243,24 13,24 a'
+rewind(99)
+read(99, *, iostat=ios) x, s
+if (ios /= 0 .or. s /= 'a') stop 1
+
+rewind(99)
+write(99, '(a)') '1,23435;1243,24;13,24;a'
+rewind(99)
+read(99, *, iostat=ios) x, s
+if (ios /= 0 .or. s /= 'a') stop 2
+
+! Note: not reading 's'
+rewind(99)
+write(99, '(a)') '1,23435 1243,24 13,24 ,'
+rewind(99)
+read(99, *) x
+if (ios /= 0) stop 3
+
+rewind(99)
+write(99, '(a)') '1,23435;1243,24;13,24 ,'
+rewind(99)
+read(99, *, iostat=ios) x
+if (ios /= 0) stop 4
+
+! Now reading 's'
+s = 'w'
+rewind(99)
+write(99, '(a)') '1,23435 1243,24 13,24 ,'
+rewind(99)
+read(99, *, iostat=ios) x, s
+if (ios /= 0 .or. s /= 'w') stop 5
+
+s = 'w'
+rewind(99)
+write(99, '(a)') '1,23435;1243,24;13,24 ,'
+rewind(99)
+read(99, *, iostat=ios) x, s
+if (ios /= 0 .or. s /= 'w') stop 6
+close(99)
+end
diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
index fb3f7dbc34d..f6f169043bf 100644
--- a/libgfortran/io/list_read.c
+++ b/libgfortran/io/list_read.c
@@ -461,11 +461,30 @@ eat_separator (st_parameter_dt *dtp)
int c, n;
int err = 0;
- eat_spaces (dtp);
dtp->u.p.comma_flag = 0;
+ c = next_char (dtp);
+ if (c == ' ')
+{
+ eat_spaces (dtp);
+ c = next_char (dtp);
+ if (c == ',')
+ {
+ if (dtp->u.p.current_unit->decimal_status == DECIMAL_COMMA)
+ unget_char (dtp, ';');
+ dtp->u.p.comma_flag = 1;
+ eat_spaces (dtp);
+ return err;
+ }
+ if (c == ';')
+ {
+ if (dtp->u.p.current_unit->decimal_status == DECIMAL_POINT)
+ unget_char (dtp, ',');
+ dtp->u.p.comma_flag = 1;
+ eat_spaces (dtp);
+ return err;
+ }
+}
- if ((c = next_char (dtp)) == EOF)
-return LIBERROR_END;
switch (c)
{
case ',':
@@ -476,7 +495,10 @@ eat_separator (st_parameter_dt *dtp)
unget_char (dtp, c);
break;
}
-/* Fall through. */
+ dtp->u.p.comma_flag = 1;
+ eat_spaces (dtp);
+ break;
+
case ';':
dtp->u.p.comma_flag = 1;
eat_spaces (dtp);
