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.00000000 ios= 0
point, isreal = T , testinput = "3.3;"r= 3.29999995 ios= 0
point, isreal = T , testinput = "3,3;"r= 3.00000000 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.00000000 ios= 0
point, isreal = T , testinput = "4.4 ;"r= 4.40000010 ios=0
point, isreal = T , testinput = "9 ;"r= 9.00000000 ios= 0
point, isreal = T , testinput = "4,4 ;"r= 4.00000000 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
