[PATCH, Fortran] -- PR fortran/87923 -- fix ICE when resolving I/O tags and simplify io.c
The attached patch fixes PR 87923 while also simplifying the code in
io.c. I do say this patch simplifies io.c because it is true. This
patch causes more deletions than insertions to the file -- a rare
Over the years various special cases have been introduced which are
not necessary. The constraints for I/O statements are verified in
several different places, and in fact some constraints (like whether
an I/O tag is a scalar default-kind character) are checked as many as
three times. This patches simplifies the code by moving all checks not
necessary for matching out of the matching phase and into the
resolution phase. The resolve_tag function already checks several
constraints for I/O tags, including type and kind, which were
previously checked redundantly in other places. This patch also
improves error reporting by providing the correct locus for I/O
statement error messages, and by providing a more detailed error
message when a tag which requires an init-expr is not a valid
The patch also increases test coverage of I/O statements, especially
for I/O tags, by incorporating testcases provided in PRs from the past
which the removed code originally addressed: specifically, PRs 66724
and 66725. With the patch applied on the current master
(c72a1b6f8b2...) all regression tests with check-fortran pass,
including the new ones.
This patch also reorganizes I/O checking code. Checks which were done
in the matching phase which do not affect the match result are moved to the
resolution phase. Checks which were duplicated in both the
matching phase and
resolution phase have been reduced to one check in the resolution phase.
Another section of code which used a global async_io_dt flag to
check for and
assign the asynchronous attribute to variables used in
asynchronous I/O has been
Furthermore, this patch improves error reporting and expands test
- "TAG must be an initialization expression" reported by io.c
(check_io_constraints) is replaced with an error from expr.c
(gfc_reduce_init_expr) indicating _why_ the expression is not a valid
- Several distinct error messages regarding the check for scalar
+ default kind have been unified to one message reported by resolve_tag
* gfortran.h (gfc_resolve_open, gfc_resolve_close): Add
(gfc_resolve_dt): Add code parameter.
* io.c (async_io_dt, check_char_variable, is_char_type): Removed.
(resolve_tag_format): Add locus to error message regarding
array in FORMAT tag.
(check_open_constraints, check_close_constraints): New
at resolution time.
(gfc_match_open, gfc_match_close, match_io): Move checks which don't
affect the match result to new functions check_open_constraints,
(gfc_resolve_open, gfc_resolve_close): Call new functions
check_open_constraints, check_close_constraints after all
tags have been
independently resolved. Remove duplicate constraints
which are already
verified by resolve_tag. Explicitly pass locus to all error reports.
(compare_to_allowed_values): Add locus parameter and
locus all error reports.
(match_open_element, match_close_element, match_file_element,
match_dt_element, match_inquire_element): Remove redundant
for ASYNCHRONOUS and IOMSG tags.
(gfc_resolve_dt): Remove redundant special case for format
Call check_io_constraints, forwarding an I/O list as the io_code
parameter if present.
(check_io_constraints): Change return type to bool. Pass
to error reports. Move generic checks after tag-specific
errors are no longer buffered. Move simplification of
format string to
match_io. Remove redundant checks which are verified by
Remove usage of async_io_dt flag and explicitly mark symbols used in
asynchronous I/O with the asynchronous attribute.
* resolve.c (resolve_transfer, resolve_fl_namelist):
Remove checks for
async_io_dt flag. This is now done in io.c (check_io_constraints).
(gfc_resolve_code): Pass code locus to gfc_resolve_open,
> 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.
> + */
I will commit the patch with these fixes rebased on master after one
final build+test. Thanks again for taking a look.
Re: [PATCH, Fortran] -- PR fortran/87923 -- fix ICE when resolving I/O tags and simplify io.c
On Fri, Apr 10, 2020 at 8:14 AM Rainer Orth <[hidden email]> wrote:
> Hi Fritz,
> 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).
Ah! My mistake... I will fix and look for this in the future.