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