[Fortran, Patch, PR51815, v3] Fix parsing of substring refs in coarrays.

[Harald Anlauf]

Hi Andre,

On 10/7/24 11:04, Andre Vehreschild wrote:
> Hi Harald,
>
> thank you for your input. I still have some small nits to discuss to make
> everyone happy. Therefore:
>
>> this seems to go into the right direction - except that I am not a
>> great fan of gfc_error_now, as that tries to paper over deficiencies
>> in error recovery.
>
> Me either, but when I remove the gfc_error_now() and only do
>
>> 	  if (gfc_peek_ascii_char () == '(')
>> 	    return MATCH_ERROR;
>
> as you proposed, then no error is given for:
>
>     character(:), allocatable :: x[:]
>     character(:), allocatable :: c
>     c = x(:)(2:5)
>
> I.e. nothing at all.

hmmm, without the hunk in question I do get:

     4 |   c = x(:)(2:5)
       |   1
Error: Unclassifiable statement at (1)


which is the same when doing a return MATCH_ERROR;

When I simply use:

	  if (gfc_peek_ascii_char () == '(')
	    {
	      gfc_error ("Unexpected array/substring ref at %C");
	      return MATCH_ERROR;
	    }

this already generates:

     4 |   c = x(:)(2:5)
       |           1
Error: Unexpected array/substring ref at (1)


 > Therefore at the moment I prefer to stick to the initial> solution
with the gfc_error_now, which not only gives an error in the
> associate, but also when one just does an array/substring-ref outside of
> parentheses. And I like the new error message, because I consider it more
> helpful than just a syntax error or the invalid association target message.
> What do you think?

The motivation for my asking is based on the following naive thinking
(assuming that x is of type character):

x(:)(2:5)    ! could be a rank mismatch when x is an array
x[1](:)(2:5) ! is always a syntax error
x(:)[1](2:5) ! could by diagnosed as a rank mismatch

That is of course wishful thinking on my side.  No compiler
matches this completely, and diagnosing a syntax error is
certainly acceptable behavior.  (Some other brand shows funny
diagnostics coming likely from the resolution phase).

>> Is there a reason that you do not check the return value of
>> gfc_match_array_ref?
>
> What am I to do with the result? We are in an error case independent of the
> result of gfc_match_array_ref. The intention of using that routine here was to
> digest the unexpected input and allow for (easier|better) error recovery.

Do you have an example that shows the use of gfc_match_array_ref here?
Commenting it out doesn't seem to make a difference in the error case
here, unless I missed something.

 > May> be I should just put a comment on it, to make it more clear. Or
is there
> another way to help the parser recover from an error?

Well, I am not the expert to answer that.  Without gfc_error_now,
we're more likely seeing errors coming from the parsing of the
associate, and here I would point to Paul as the one with the most
experience.  I would hope that the parsing of associate would see
if an error was issued for the associate target and allow that error
to be emitted.

> Sorry for the additional round. But this error has been around for so long,
> that it doesn't matter, if we need another day to come up with a solution.

Indeed!  :-)

> Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?

I am fine with your solution.  Diagnostics can be improved later
any time...

> Regards,
> 	Andre

Thanks for your patience!

Harald

>
>> Indeed your suggestion (or the shortened version above) improves
>> the diagnostics ("user experience") also for this variant:
>>
>> subroutine foo
>>      character(:), allocatable :: x[:]
>>      character(:), dimension(:), allocatable :: c[:]
>>      type t
>>         character(:), allocatable :: x[:]
>>         character(:), dimension(:), allocatable :: c[:]
>>      end type t
>>      type(t) :: z
>>      associate (y => x(:)(2:))
>>      end associate
>>      associate (a => c(:)(:)(2:))
>>      end associate
>>      associate (y => z%x(:)(2:))
>>      end associate
>>      associate (a => z%c(:)(:)(2:))
>>      end associate
>> end
>>
>> with several error messages of the kind
>>
>> Error: Invalid association target at (1)
>>
>> or
>>
>> Error: Rank mismatch in array reference at (1) (1/0)
>>
>> looking less technical than a parsing error.
>> I think this is as good as it can be.
>>
>> So OK from my side with either your additional patch or my
>> shortened version.
>>
>> Thanks for the patch!
>>
>> Harald
>>
>>
>>> Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Is this ok?
>>>
>>> Regards and thanks for the review,
>>> 	Andre
>>>
>>> On Tue, 1 Oct 2024 23:31:11 +0200
>>> Harald Anlauf <anlauf@gmx.de> wrote:
>>>
>>>> Hi Andre,
>>>>
>>>> Am 01.10.24 um 09:43 schrieb Andre Vehreschild:
>>>>> Hi all,
>>>>>
>>>>> this rather old PR reported a parsing bug, when a coarray'ed character
>>>>> substring ref is to be parsed, aka CHARACTER(:) :: str[:] ... str(2:5). In
>>>>> this case the parser confused the substring ref with an array-ref, because
>>>>> an array_spec was present. This patch fixes this by requesting only
>>>>> coarray parsing from gfc_match_array_ref when no regular dimension is
>>>>> present. The patch is not involved when an array of coarray'ed strings is
>>>>> parsed (that worked beforehand).
>>>>
>>>> while the patch addresses the issue mentioned in the PR,
>>>>
>>>>> I had to fix the dg-error clauses in the testcase pr102532 because now the
>>>>> error of having to many refs is detected by the parsing stage and no
>>>>> longer by the resolve stage. It has become a simple syntax error. I hope
>>>>> this is ok.
>>>>
>>>> I find the error messages now less helpful to users: before the patch
>>>> we got "Rank mismatch in array reference", which was more suitable
>>>> than the newer version with more or less confusing syntax errors.
>>>>
>>>> I assume you tried to find a better solution - but Intel and NAG
>>>> also give syntax errors - so basically I am fine with the patch.
>>>>
>>>> You may want to wait for a second opinion.  If nobody else responds
>>>> within the next 2 days, you may proceed nevertheless.
>>>>
>>>> Thanks,
>>>> Harald
>>>>
>>>>> Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?
>>>>>
>>>>> Regards,
>>>>> 	Andre
>>>>> --
>>>>> Andre Vehreschild * Email: vehre ad gmx dot de
>>>>
>>>
>>>
>>> --
>>> Andre Vehreschild * Email: vehre ad gmx dot de
>>>
>>
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de