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 only have three nits:
* 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 …
* 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."
* 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.
+
+ */
Otherwise: LGTM.
Thanks,
Tobias
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
init-expr.
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.
OK for master?
commit 5a403f4e8e77123994ca1ed05e8f10877423fe55
Author: Fritz Reese <fore...@gcc.gnu.org>
Date: Mon Apr 6 12:13:48 2020 -0400
Fix fortran/87923 -- ICE(s) when resolving I/O tags.
2020-04-06 Fritz Reese <fore...@gcc.gnu.org>
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
simplified.
Furthermore, this patch improves error reporting and expands test
coverage of
I/O tags:
- "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
initialization expression.
- Several distinct error messages regarding the check for scalar
+ character
+ default kind have been unified to one message reported by resolve_tag
or check_*_constraints.
gcc/fortran/ChangeLog:
PR fortran/87923
* gfortran.h (gfc_resolve_open, gfc_resolve_close): Add
locus parameter.
(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
zero-sized
array in FORMAT tag.
(check_open_constraints, check_close_constraints): New
functions called
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,
check_close_constraints, check_io_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
provide explicit
locus all error reports.
(match_open_element, match_close_element, match_file_element,
match_dt_element, match_inquire_element): Remove redundant
special cases
for ASYNCHRONOUS and IOMSG tags.
(gfc_resolve_dt): Remove redundant special case for format
expression.
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
explicit locus
to error reports. Move generic checks after tag-specific
checks, since
errors are no longer buffered. Move simplification of
format string to
match_io. Remove redundant checks which are verified by
resolve_tag.
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,
gfc_resolve_close, gfc_resolve_dt.
gcc/testsuite/ChangeLog:
PR fortran/87923
* gfortran.dg/f2003_io_8.f03: Fix expected error messages.
* gfortran.dg/io_constraints_8.f90: Likewise.
* gfortran.dg/iomsg_2.f90: Likewise.
* gfortran.dg/pr66725.f90: Likewise.
* gfortran.dg/pr88205.f90: Likewise.
* gfortran.dg/write_check4.f90: Likewise.
* gfortran.dg/asynchronous_5.f03: New test.
* gfortran.dg/io_constraints_15.f90: Likewise.
* gfortran.dg/io_constraints_16.f90: Likewise.
* gfortran.dg/io_constraints_17.f90: Likewise.
* gfortran.dg/io_constraints_18.f90: Likewise.
* gfortran.dg/io_tags_1.f90: Likewise.
* gfortran.dg/io_tags_10.f90: Likewise.
* gfortran.dg/io_tags_2.f90: Likewise.
* gfortran.dg/io_tags_3.f90: Likewise.
* gfortran.dg/io_tags_4.f90: Likewise.
* gfortran.dg/io_tags_5.f90: Likewise.
* gfortran.dg/io_tags_6.f90: Likewise.
* gfortran.dg/io_tags_7.f90: Likewise.
* gfortran.dg/io_tags_8.f90: Likewise.
* gfortran.dg/io_tags_9.f90: Likewise.
* gfortran.dg/write_check5.f90: Likewise.
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
