Fortran: List-directed read - accept again tab as alternative to space as separator [PR114304] (was: [patch, libgfortran] PR114304 - [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'")
Checks
Commit Message
Jerry D wrote:
> See attached updated patch.
It turned rather quickly out that this patch – committed as
r14-9822-g93adf88cc6744a – caused regressions.
Namely, real-world code use tab(s) as separator instead of spaces.
[For instance, PR114304 which contains a named-list input file from SPEC
CPU 2017; that example uses tabs before the '=' sign, but the issue is
more generic.]
I think the ISO Fortran standard only permits spaces, but as it feels
natural and is widely supported, tabs are used and should remain supported.
It is not quite clear how '\r' are or should be handled, but as
eat_spaces did use it, I thought I would add one testcase using them as
well.
That test is not affected by my change; it did work before with GCC and
still does – but it does fail with ifort/ifx/flang. I have not thought
deeply whether it should be supported or not – and looking at the
libgfortran source file, it often but (→ testcase) not consistently
requires that an \n follows the \r.
OK for mainline? [And: When the previous patch gets backported, this
surely needs to be included as well.]
Tobias
Comments
On 4/8/24 2:53 AM, Tobias Burnus wrote:
> Jerry D wrote:
>> See attached updated patch.
>
> It turned rather quickly out that this patch – committed as r14-9822-g93adf88cc6744a – caused regressions.
>
> Namely, real-world code use tab(s) as separator instead of spaces.
>
> [For instance, PR114304 which contains a named-list input file from SPEC CPU 2017; that example uses tabs before the '=' sign, but the issue is more generic.]
>
> I think the ISO Fortran standard only permits spaces, but as it feels natural and is widely supported, tabs are used and should remain supported.
>
> It is not quite clear how '\r' are or should be handled, but as eat_spaces did use it, I thought I would add one testcase using them as well.
>
> That test is not affected by my change; it did work before with GCC and still does – but it does fail with ifort/ifx/flang. I have not thought deeply whether it should be supported or not – and
> looking at the libgfortran source file, it often but (→ testcase) not consistently requires that an \n follows the \r.
>
> OK for mainline? [And: When the previous patch gets backported, this surely needs to be included as well.]
>
> Tobias
Good catch. I did not even think about tabs.
OK to commit and I will take care of it when I do the backport to 13.
Thanks!
Jerry
Fortran: Accept again tab as alternative to space as separator [PR114304]
This fixes a side-effect of/regression caused by r14-9822-g93adf88cc6744a,
which was for the same PR.
PR libfortran/114304
libgfortran/ChangeLog:
* io/list_read.c (eat_separator): Accept tab as alternative to space.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr114304-2.f90: New test.
gcc/testsuite/gfortran.dg/pr114304-2.f90 | 82 ++++++++++++++++++++++++++++++++
libgfortran/io/list_read.c | 2 +-
2 files changed, 83 insertions(+), 1 deletion(-)
new file mode 100644
@@ -0,0 +1,82 @@
+! { dg-do run }
+!
+! PR fortran/114304
+!
+! Ensure that '\t' (tab) is supported as separator in list-directed input
+! While not really standard conform, this is widely used in user input and
+! widely supported.
+!
+
+use iso_c_binding
+implicit none
+character(len=*,kind=c_char), parameter :: tab = C_HORIZONTAL_TAB
+
+! Accept '<tab>' as variant to ' ' as separator
+! Check that <carriage_return><new line> and <new_line> are handled
+
+character(len=*,kind=c_char), parameter :: nml_str &
+ = '&inparm'//C_CARRIAGE_RETURN // C_NEW_LINE // &
+ 'first'//tab//'='//tab//' .true.'// C_NEW_LINE // &
+ ' , other'//tab//' ='//tab//'3'//tab//', 2'//tab//'/'
+
+! Check that <carriage_return> is handled,
+
+! Note: For new line, Unix uses \n, Windows \r\n but old Apple systems used '\r'
+!
+! Gfortran does not seem to support all \r, but the following is supported
+! since ages, ! which seems to be a gfortran extension as ifort and flang don't like it.
+
+character(len=*,kind=c_char), parameter :: nml_str2 &
+ = '&inparm'//C_CARRIAGE_RETURN // C_NEW_LINE // &
+ 'first'//C_NEW_LINE//'='//tab//' .true.'// C_CARRIAGE_RETURN // &
+ ' , other'//tab//' ='//tab//'3'//tab//', 2'//tab//'/'
+
+character(len=*,kind=c_char), parameter :: str &
+ = tab//'1'//tab//'2,'//tab//'3'//tab//',4'//tab//','//tab//'5'//tab//'/'
+character(len=*,kind=c_char), parameter :: str2 &
+ = tab//'1'//tab//'2;'//tab//'3'//tab//';4'//tab//';'//tab//'5'//tab//'/'
+logical :: first
+integer :: other(4)
+integer :: ints(6)
+namelist /inparm/ first , other
+
+other = 1
+
+open(99, file="test.inp")
+write(99, '(a)') nml_str
+rewind(99)
+read(99,nml=inparm)
+close(99, status="delete")
+
+if (.not.first .or. any (other /= [3,2,1,1])) stop 1
+
+other = 9
+
+open(99, file="test.inp")
+write(99, '(a)') nml_str2
+rewind(99)
+read(99,nml=inparm)
+close(99, status="delete")
+
+if (.not.first .or. any (other /= [3,2,9,9])) stop 2
+
+ints = 66
+
+open(99, file="test.inp", decimal='point')
+write(99, '(a)') str
+rewind(99)
+read(99,*) ints
+close(99, status="delete")
+
+if (any (ints /= [1,2,3,4,5,66])) stop 3
+
+ints = 77
+
+open(99, file="test.inp", decimal='comma')
+write(99, '(a)') str2
+rewind(99)
+read(99,*) ints
+close(99, status="delete")
+
+if (any (ints /= [1,2,3,4,5,77])) stop 4
+end
@@ -463,7 +463,7 @@ eat_separator (st_parameter_dt *dtp)
dtp->u.p.comma_flag = 0;
c = next_char (dtp);
- if (c == ' ')
+ if (c == ' ' || c == '\t')
{
eat_spaces (dtp);
c = next_char (dtp);