Hi Fritz,
> Thanks very much for the review.
>
> On Thu, Apr 9, 2020 at 5:21 AM Tobias Burnus <tob...@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
