diff mbox series

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'")

Message ID 529f47ca-c398-4e20-a0ef-507842f989d4@baylibre.com
State New
Headers show
Series 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'") | expand

Commit Message

Tobias Burnus April 8, 2024, 9:53 a.m. UTC 
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

Jerry D April 8, 2024, 6:21 p.m. UTC | #1 
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
diff mbox series

Patch

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(-)

diff --git a/gcc/testsuite/gfortran.dg/pr114304-2.f90 b/gcc/testsuite/gfortran.dg/pr114304-2.f90
new file mode 100644
index 00000000000..5ef5874f528
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr114304-2.f90
@@ -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
diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
index b56f2a4e6d6..5bbbef26c26 100644
--- a/libgfortran/io/list_read.c
+++ b/libgfortran/io/list_read.c
@@ -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);