Re: [PATCH, Fortran] -- PR fortran/87923 -- fix ICE when resolving I/O tags and simplify io.c

[Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>]

Hi Fritz,

> Thanks very much for the review.
>
> On Thu, Apr 9, 2020 at 5:21 AM Tobias Burnus <tobias@codesourcery.com> wrote:
>>
>> Hi,
>>
>> On 4/6/20 7:25 PM, Fritz Reese via Fortran wrote:
>>
>> > The attached patch fixes PR 87923 while also simplifying the code in
>> > io.c.
>>
>> Thanks for the work, which looks great; it is a bit lengthy
>> but mostly moving code or mechanical (%C → %L).
>> It also has an impressive amount of new test cases!
>
> I also wished the patch could be easier on the eyes, but alas
> sometimes this is the price of progress. :-)
>
>> * First line in git commit "Fix fortran/87923 -- ICE(s) when resolving
>> I/O tags."
>>    It helps with doing patch archeology if they are the same – or if the
>> GIT one
>>    is a substring of the email subject. (If it is about a PR, searching
>> for the PR
>>    usually works, but also not al emails have the PR number in the subject.)
>>    For this patch, you use:
>>    email: "[PATCH, Fortran] -- PR fortran/87923 -- fix ICE when resolving
>> I/O tags and simplify io.c"
>>    GIT: "Fix fortran/87923 -- ICE(s) when resolving I/O tags."
>
> Yes, that is a good point. I will alter the commit summary to match
> the email subject.
>
>> * Please check whether the ChangeLog lines are too long; I didn't count
>>    but it looks as if they might be too long. For sure, they
>>    were too long for your mail program …
>
> I copied the git commit message from the log, which git formats with
> an extra level of indentation. I wrapped the raw ChangeLog entries and
> commit message to 80 characters, but after the extra indentation my
> mail client indeed wrapped the lines during post-processing. I suppose
> I should wrap these each to 76 to account for git's indentation. That
> certainly makes "git log" look better.
>
>> * In the following comment, you have two empty tailing lines:
>>
>> +   Tag expressions are already resolved by resolve_tag, which includes
>> +   verifying the type, that they are scalar, and verifying that BT_CHARACTER
>> +   tags are of default kind.
>> +
>> +   */
>
> Oops!
>
>
> I will commit the patch with these fixes rebased on master after one
> final build+test. Thanks again for taking a look.

one new testcases comes up as UNRESOLVED everywhere:

+UNRESOLVED: gfortran.dg/asynchronous_5.f03   -O   scan-tree-dump-not original "volatile.*?ivar_noasync"
+UNRESOLVED: gfortran.dg/asynchronous_5.f03   -O   scan-tree-dump-times original "volatile.*?ccvar_async" 1
+UNRESOLVED: gfortran.dg/asynchronous_5.f03   -O   scan-tree-dump-times original "volatile.*?darrvar_async" 1
+UNRESOLVED: gfortran.dg/asynchronous_5.f03   -O   scan-tree-dump-times original "volatile.*?dvar_async" 1
+UNRESOLVED: gfortran.dg/asynchronous_5.f03   -O   scan-tree-dump-times original "volatile.*?ivar_async" 1
+UNRESOLVED: gfortran.dg/asynchronous_5.f03   -O   scan-tree-dump-times original "volatile.*?lvar_async" 1
+UNRESOLVED: gfortran.dg/asynchronous_5.f03   -O   scan-tree-dump-times original "volatile.*?rvar_async" 1

gfortran.dg/asynchronous_5.f03   -O  : dump file does not exist

It has several scan-tree-dump* checks, but no corresponding
-fdump-tree-* option.  Please fix (and make sure not to look only for
FAILs during regtesting in the future).

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University