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

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[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


array_substring.diff (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Thomas Koenig-6
Hi Tobias,

nice to hear from you again!

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

OK. Thanks for the patch!

Regards

        Thomas
Reply | Threaded
Open this post in threaded view
|

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

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

On Mon, 8 Oct 2018 at 23:16, Thomas Koenig <[hidden email]> wrote:

>
> Hi Tobias,
>
> nice to hear from you again!
>
> > 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
Reply | Threaded
Open this post in threaded view
|

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

Jakub Jelinek
In reply to this post by Tobias Burnus
On Mon, Oct 08, 2018 at 09:44:57PM +0200, Tobias Burnus wrote:

> 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?

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
Reply | Threaded
Open this post in threaded view
|

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

David Edelsohn-2
In reply to this post by Paul Richard Thomas
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
<[hidden email]> wrote:

>
> 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
>
> On Mon, 8 Oct 2018 at 23:16, Thomas Koenig <[hidden email]> wrote:
> >
> > Hi Tobias,
> >
> > nice to hear from you again!
> >
> > > 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
Reply | Threaded
Open this post in threaded view
|

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

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


On Thu, 11 Oct 2018 at 15:20, David Edelsohn <[hidden email]> wrote:

>
> 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
> <[hidden email]> wrote:
> >
> > 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
> >
> > On Mon, 8 Oct 2018 at 23:16, Thomas Koenig <[hidden email]> wrote:
> > >
> > > Hi Tobias,
> > >
> > > nice to hear from you again!
> > >
> > > > 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



--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Reply | Threaded
Open this post in threaded view
|

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

Tobias Burnus
In reply to this post by Jakub Jelinek
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

Am 11.10.18 um 12:46 schrieb Jakub Jelinek:

> On Mon, Oct 08, 2018 at 09:44:57PM +0200, Tobias Burnus wrote:
>> 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?
> 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
Reply | Threaded
Open this post in threaded view
|

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

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

On Thu, 11 Oct 2018 at 20:37, Tobias Burnus <[hidden email]> wrote:

>
> 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
>
> Am 11.10.18 um 12:46 schrieb Jakub Jelinek:
> > On Mon, Oct 08, 2018 at 09:44:57PM +0200, Tobias Burnus wrote:
> >> 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?
> > 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



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