[Patch, Fortran] PR fortran/83522 – reject array-valued substrings

[Tobias Burnus]

Hi all,

as the PR (and the attached testcase) shows, gfortran doesn't handle
noncontiguous access with substrings. Namely, "str_array(:)(1:5)" is
mishandled (segfault while building the "tree" as expr == NULL).

As Jerry has dug up in the standard (F2008, R610 alias F2018, R908),
substrings are only permitted on scalars (or array elements). –
[gfortran has some partial support for noncontiguous memory as actual
argument the now rejected test cases show.]

In words of the standard (here: F2018):

9.4.1  Substrings

A substring is a contiguous portion of a character string (7.4.4).

R908   substring   is   parent-string  ( substring-range )

R909   parent-string
            is   scalar-variable-name
            or  array-element
            or  coindexed-named-object
            or  scalar-structure-component
            or  scalar-constant

The patch adds a check to the variable resolving – it comes too late to
print a nice error message for "string(:)(:)"; that statement gets
matched unsuccessfully and at the end the buffered error message of
match_variable (unexpected character "(") gets printed. – One could
reject it earlier but the code doesn't seem to be that nice. (See PR for
a variant.)

Build and regtested on x86_64-linux.
OK for the trunk?

Cheers

Tobias

[Thomas Koenig]

Hi Tobias,

nice to hear from you again!

Post by Tobias Burnus
Build and regtested on x86_64-linux.
OK for the trunk?

OK. Thanks for the patch!

Regards

Thomas

[Paul Richard Thomas]

This seems to have caused errors in the testsuite. I guess that the
problem is that the testcases are invalid :-(

From David Edelsohn:
Error: Substring reference of nonscalar not permitted at (1)

arrayio_11.f90
arrayio_12.f90
associate_23.f90
data_char_1.f90
deferred_character_2.f90
deferred_character_22.f90
deferred_character_23.f90
deferred_character_8.f90
deferred_type_component_3.f90

Cheers

Paul

Post by Thomas Koenig
Hi Tobias,
nice to hear from you again!

Post by Tobias Burnus
Build and regtested on x86_64-linux.
OK for the trunk?

OK. Thanks for the patch!
Regards
Thomas

--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

[David Edelsohn]

Apparently this change also breaks CPU SPEC (465.tonto?).

Is this really a correct change? Even if the testcases really are
wrong, it seems that this change is going to break a lot of code in
the wild. This seems like a very bad choice, even if it pedantically
is correct.

Thanks, David

On Wed, Oct 10, 2018 at 7:12 PM Paul Richard Thomas

Post by Paul Richard Thomas
This seems to have caused errors in the testsuite. I guess that the
problem is that the testcases are invalid :-(
Error: Substring reference of nonscalar not permitted at (1)
arrayio_11.f90
arrayio_12.f90
associate_23.f90
data_char_1.f90
deferred_character_2.f90
deferred_character_22.f90
deferred_character_23.f90
deferred_character_8.f90
deferred_type_component_3.f90
Cheers
Paul

Post by Thomas Koenig
Hi Tobias,
nice to hear from you again!

Post by Tobias Burnus
Build and regtested on x86_64-linux.
OK for the trunk?

OK. Thanks for the patch!
Regards
Thomas

--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

[David Edelsohn]

Another data point: I tried some of the testcases with IBM XL Fortran
compiler and it didn't complain or fail. I have no idea if this
behavior is an IBM extension or how other Fortran compilers behave.

GNU Fortran probably should be compatible with other Fortran
compilers, even if the behavior is not pedantically conformant with
the standard.

Thanks, David

On Wed, Oct 10, 2018 at 7:12 PM Paul Richard Thomas

Post by Paul Richard Thomas
This seems to have caused errors in the testsuite. I guess that the
problem is that the testcases are invalid :-(
Error: Substring reference of nonscalar not permitted at (1)
arrayio_11.f90
arrayio_12.f90
associate_23.f90
data_char_1.f90
deferred_character_2.f90
deferred_character_22.f90
deferred_character_23.f90
deferred_character_8.f90
deferred_type_component_3.f90
Cheers
Paul

Post by Thomas Koenig
Hi Tobias,
nice to hear from you again!

Post by Tobias Burnus
Build and regtested on x86_64-linux.
OK for the trunk?

OK. Thanks for the patch!
Regards
Thomas

--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

[Paul Richard Thomas]

The section from the standard in comment #2 is being misinterpreted.
R609 and R610 concern substrings in the context of section 6.4, which
concerns SCALARS!

Section 6.5 concerns arrays and you will find there:

R618 array-section is data-ref [ ( substring-range ) ] or
complex-part-designator

C624 (R618) Exactly one part-ref shall have nonzero rank, and either
the final part-ref shall have a section-
subscript-list with nonzero rank, another part-ref shall have nonzero
rank, or the complex-part-designator
shall be an array.

Further lines appear later in which in which substrings of array
sections are explicitly mentioned: eg.
If a substring-range appears in an array-section, each element is the
designated substring of the corresponding element of the array
section. The value of a subscript in an array element shall be within
the bounds for its dimension.

Thus the patch is contrary to the standard and must be removed as soon
as possible.

Paul

Post by David Edelsohn
Another data point: I tried some of the testcases with IBM XL Fortran
compiler and it didn't complain or fail. I have no idea if this
behavior is an IBM extension or how other Fortran compilers behave.
GNU Fortran probably should be compatible with other Fortran
compilers, even if the behavior is not pedantically conformant with
the standard.
Thanks, David
On Wed, Oct 10, 2018 at 7:12 PM Paul Richard Thomas

Post by Paul Richard Thomas
This seems to have caused errors in the testsuite. I guess that the
problem is that the testcases are invalid :-(
Error: Substring reference of nonscalar not permitted at (1)
arrayio_11.f90
arrayio_12.f90
associate_23.f90
data_char_1.f90
deferred_character_2.f90
deferred_character_22.f90
deferred_character_23.f90
deferred_character_8.f90
deferred_type_component_3.f90
Cheers
Paul

Post by Thomas Koenig
Hi Tobias,
nice to hear from you again!

Post by Tobias Burnus
Build and regtested on x86_64-linux.
OK for the trunk?

OK. Thanks for the patch!
Regards
Thomas

--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

[Jakub Jelinek]

Post by Tobias Burnus
as the PR (and the attached testcase) shows, gfortran doesn't handle
noncontiguous access with substrings. Namely, "str_array(:)(1:5)" is
mishandled (segfault while building the "tree" as expr == NULL).
As Jerry has dug up in the standard (F2008, R610 alias F2018, R908),
substrings are only permitted on scalars (or array elements). – [gfortran
has some partial support for noncontiguous memory as actual argument the now
rejected test cases show.]
9.4.1  Substrings
A substring is a contiguous portion of a character string (7.4.4).
R908   substring   is   parent-string  ( substring-range )
R909   parent-string
            is   scalar-variable-name
            or  array-element
            or  coindexed-named-object
            or  scalar-structure-component
            or  scalar-constant
The patch adds a check to the variable resolving – it comes too late to
print a nice error message for "string(:)(:)"; that statement gets matched
unsuccessfully and at the end the buffered error message of match_variable
(unexpected character "(") gets printed. – One could reject it earlier but
the code doesn't seem to be that nice. (See PR for a variant.)
Build and regtested on x86_64-linux.
OK for the trunk?

As I wrote in the PR, I believe it is incorrect and should be reverted.
While for
character(kind=1,length=4) :: f(4,5)
e.g.
f(:,:)(1:2)
is not matching the substring non-terminal, it is IMNSHO matching the
array-section non-terminal:
R918 array-section
            is data-ref [ ( substring-range ) ]
            or complex-part-designator
C925 (R918) Exactly one part-ref shall have nonzero rank, and either the final
part-ref shall have a section-subscript-list with nonzero rank, another
part-ref shall have nonzero rank, or the complex-part-designator
shall be an array.
C926 (R918) If a substring-range appears, the rightmost part-name shall be of type
character.
and in that case:

An array element is a scalar. An array section is an array. If a substring-range
appears in an array-section, each element is the designated substring of the
corresponding element of the array section.

applies.

Jakub

[Tobias Burnus]

In light of the somehow missed test-suite fails and the discussion about
rejecting too much, I have reverted the patch with commit r83522.

Tobias

Post by Jakub Jelinek

Post by Tobias Burnus
as the PR (and the attached testcase) shows, gfortran doesn't handle
noncontiguous access with substrings. Namely, "str_array(:)(1:5)" is
mishandled (segfault while building the "tree" as expr == NULL).
As Jerry has dug up in the standard (F2008, R610 alias F2018, R908),
substrings are only permitted on scalars (or array elements). – [gfortran
has some partial support for noncontiguous memory as actual argument the now
rejected test cases show.]
9.4.1  Substrings
A substring is a contiguous portion of a character string (7.4.4).
R908   substring   is   parent-string  ( substring-range )
R909   parent-string
            is   scalar-variable-name
            or  array-element
            or  coindexed-named-object
            or  scalar-structure-component
            or  scalar-constant
The patch adds a check to the variable resolving – it comes too late to
print a nice error message for "string(:)(:)"; that statement gets matched
unsuccessfully and at the end the buffered error message of match_variable
(unexpected character "(") gets printed. – One could reject it earlier but
the code doesn't seem to be that nice. (See PR for a variant.)
Build and regtested on x86_64-linux.
OK for the trunk?

As I wrote in the PR, I believe it is incorrect and should be reverted.
While for
character(kind=1,length=4) :: f(4,5)
e.g.
f(:,:)(1:2)
is not matching the substring non-terminal, it is IMNSHO matching the
R918 array-section
            is data-ref [ ( substring-range ) ]
            or complex-part-designator
C925 (R918) Exactly one part-ref shall have nonzero rank, and either the final
part-ref shall have a section-subscript-list with nonzero rank, another
part-ref shall have nonzero rank, or the complex-part-designator
shall be an array.
C926 (R918) If a substring-range appears, the rightmost part-name shall be of type
character.
An array element is a scalar. An array section is an array. If a substring-range
appears in an array-section, each element is the designated substring of the
corresponding element of the array section.
applies.
Jakub

[Paul Richard Thomas]

Hi Tobias,

I am sorry that your return has proven to be a bit bloody. Thanks for
reverting it. I am afraid that Jerry missed the context completely.

Regards

Paul

Post by Tobias Burnus
In light of the somehow missed test-suite fails and the discussion about
rejecting too much, I have reverted the patch with commit r83522.
Tobias

Post by Jakub Jelinek

Post by Tobias Burnus
as the PR (and the attached testcase) shows, gfortran doesn't handle
noncontiguous access with substrings. Namely, "str_array(:)(1:5)" is
mishandled (segfault while building the "tree" as expr == NULL).
As Jerry has dug up in the standard (F2008, R610 alias F2018, R908),
substrings are only permitted on scalars (or array elements). – [gfortran
has some partial support for noncontiguous memory as actual argument the now
rejected test cases show.]
9.4.1 Substrings
A substring is a contiguous portion of a character string (7.4.4).
R908 substring is parent-string ( substring-range )
R909 parent-string
is scalar-variable-name
or array-element
or coindexed-named-object
or scalar-structure-component
or scalar-constant
The patch adds a check to the variable resolving – it comes too late to
print a nice error message for "string(:)(:)"; that statement gets matched
unsuccessfully and at the end the buffered error message of match_variable
(unexpected character "(") gets printed. – One could reject it earlier but
the code doesn't seem to be that nice. (See PR for a variant.)
Build and regtested on x86_64-linux.
OK for the trunk?

As I wrote in the PR, I believe it is incorrect and should be reverted.
While for
character(kind=1,length=4) :: f(4,5)
e.g.
f(:,:)(1:2)
is not matching the substring non-terminal, it is IMNSHO matching the
R918 array-section
is data-ref [ ( substring-range ) ]
or complex-part-designator
C925 (R918) Exactly one part-ref shall have nonzero rank, and either the final
part-ref shall have a section-subscript-list with nonzero rank, another
part-ref shall have nonzero rank, or the complex-part-designator
shall be an array.
C926 (R918) If a substring-range appears, the rightmost part-name shall be of type
character.
An array element is a scalar. An array section is an array. If a substring-range
appears in an array-section, each element is the designated substring of the
corresponding element of the array section.
applies.
Jakub