| Message ID | 20240924062120.91CD920432@pchp3.se.axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | gfortran testsuite: Remove unit-files in files having open-statements, PR116701 | expand |
On 9/23/24 11:21 PM, Hans-Peter Nilsson wrote: > Here's a general approach to handle PR116701. I considered > adding manual deletions as quoted below and mentioned in the > PR, but seeing the handling of "integer 8" in > fortran-torture-execute I decided to follow that example: > better scan the source for open-statements than relying on > manual annotations and people remembering to add them for > new test-cases. > > I hope the inclusion of gfortran-dg.exp in > fortran-torture.exp is not controversial, but there's no > fortran-specific testsuite file common to dg and > classic-torture and also this placement is still in the > "Utility routines" section of gfortran-dg.exp. (BTW, the C > torture-tests changed to the dg framework some time ago - no > more .x-files there and dg-directives actually work - there > are some in gfortran.fortran-torture that are apparently > ignored!) Explain this change of including gfortran-dg.exp in fortran-torture.exp. What does it mean in the case I do 'make -k -j4 check-fortran'? Does gfortran-dg-exp get performed twice? Forgive my ignorance of the testsuite incantations. Regards, Jerry
Thanks for the review! > Date: Tue, 24 Sep 2024 17:10:27 -0700 > Cc: Jerry D <jvdelisle2@gmail.com> > From: Jerry D <jvdelisle2@gmail.com> > On 9/23/24 11:21 PM, Hans-Peter Nilsson wrote: > > I hope the inclusion of gfortran-dg.exp in > > fortran-torture.exp is not controversial, but there's no > > fortran-specific testsuite file common to dg and > > classic-torture and also this placement is still in the > > "Utility routines" section of gfortran-dg.exp. (BTW, the C > > torture-tests changed to the dg framework some time ago - no > > more .x-files there and dg-directives actually work - there > > are some in gfortran.fortran-torture that are apparently > > ignored!) > > Explain this change of including gfortran-dg.exp in fortran-torture.exp. I need to put the new proc in a file, to be used by both dg and classic-torture. I picked among the untility-carrying files gfortran-dg.exp, as it looked more fitting than e.g. fortran-modules.exp. Since it's not previously included there, I included that file in fortran-torture.exp. By including that file, not just the new proc gfortran-dg-rmunits but also the other procs in that file are available. Since they don't collide with the fortran-torture machinery, that should have no effect. > What does it mean in the case I do 'make -k -j4 check-fortran'? Does > gfortran-dg-exp get performed twice? (I assume you mean "are the gfortran.dg tests run twice" as other interpretations make less sense to me.) No. > Forgive my ignorance of the > testsuite incantations. There's nothing but load_lib and proc definitions in gfortran-dg.exp, specifically no "top-level code" running tests like execute.exp or dg.exp, so including it should have no such effect...but I see that the files it include *do* have top-level code (setting global variables for use by the testsuite machinery, *not* running tests). Perhaps I should ignore that misnomer and put gfortran-dg-rmunits in fortran-modules.exp in order to put pollution worries to rest. After all, that file already has the utility proc igrep, used in gfortran-dg-rmunits. So, new version coming up. brgds, H-P
On 9/24/24 5:46 PM, Hans-Peter Nilsson wrote: > Thanks for the review! > >> Date: Tue, 24 Sep 2024 17:10:27 -0700 >> Cc: Jerry D <jvdelisle2@gmail.com> >> From: Jerry D <jvdelisle2@gmail.com> >> On 9/23/24 11:21 PM, Hans-Peter Nilsson wrote: >>> I hope the inclusion of gfortran-dg.exp in >>> fortran-torture.exp is not controversial, but there's no >>> fortran-specific testsuite file common to dg and >>> classic-torture and also this placement is still in the >>> "Utility routines" section of gfortran-dg.exp. (BTW, the C >>> torture-tests changed to the dg framework some time ago - no >>> more .x-files there and dg-directives actually work - there >>> are some in gfortran.fortran-torture that are apparently >>> ignored!) >> >> Explain this change of including gfortran-dg.exp in fortran-torture.exp. > > I need to put the new proc in a file, to be used by both dg > and classic-torture. I picked among the untility-carrying > files gfortran-dg.exp, as it looked more fitting than > e.g. fortran-modules.exp. Since it's not previously > included there, I included that file in fortran-torture.exp. > > By including that file, not just the new proc > gfortran-dg-rmunits but also the other procs in that file > are available. Since they don't collide with the > fortran-torture machinery, that should have no effect. > >> What does it mean in the case I do 'make -k -j4 check-fortran'? Does >> gfortran-dg-exp get performed twice? > > (I assume you mean "are the gfortran.dg tests run twice" as > other interpretations make less sense to me.) > Your interpretation of my typo is correct. Along with Andre I like auto cleanup. On new test cases we try to have them self delete whether they pass or fail. So your changes are ok with me. > No. > >> Forgive my ignorance of the >> testsuite incantations. > > There's nothing but load_lib and proc definitions in > gfortran-dg.exp, specifically no "top-level code" running > tests like execute.exp or dg.exp, so including it should > have no such effect...but I see that the files it include > *do* have top-level code (setting global variables for use > by the testsuite machinery, *not* running tests). > > Perhaps I should ignore that misnomer and put > gfortran-dg-rmunits in fortran-modules.exp in order to put > pollution worries to rest. After all, that file already has > the utility proc igrep, used in gfortran-dg-rmunits. So, > new version coming up. > > brgds, H-P
>Your interpretation of my typo is correct. Along with Andre I like auto cleanup. On new test cases we try to have them self delete whether they pass or fail. > so why don't we issue the cleanup then, regardless? >So your changes are ok with me. > >> No. >> >>>
On 25 September 2024 22:08:15 CEST, rep.dot.nop@gmail.com wrote: > >>Your interpretation of my typo is correct. Along with Andre I like auto cleanup. On new test cases we try to have them self delete whether they pass or fail. >> > >so why don't we issue the cleanup then, regardless? <https://xkcd.com/1172/> > >>So your changes are ok with me.
diff --git a/gcc/testsuite/lib/fortran-torture.exp b/gcc/testsuite/lib/fortran-torture.exp index 66f5bc822232..c45db285f867 100644 --- a/gcc/testsuite/lib/fortran-torture.exp +++ b/gcc/testsuite/lib/fortran-torture.exp @@ -23,6 +23,7 @@ load_lib target-supports.exp load_lib fortran-modules.exp load_lib target-utils.exp +load_lib gfortran-dg.exp # Return the list of options to use for fortran torture tests. # The default option list can be overridden by @@ -332,6 +333,8 @@ proc fortran-torture-execute { src } { catch { remote_file build delete $executable } } $status "$testcase execution, $option" + + gfortran-dg-rmunits $src } cleanup-modules "" } diff --git a/gcc/testsuite/lib/gfortran-dg.exp b/gcc/testsuite/lib/gfortran-dg.exp index fcba95dc3961..eb54844607dc 100644 --- a/gcc/testsuite/lib/gfortran-dg.exp +++ b/gcc/testsuite/lib/gfortran-dg.exp @@ -160,6 +160,7 @@ proc gfortran-dg-runtest { testcases flags default-extra-flags } { foreach flags_t $option_list { verbose "Testing $nshort, $flags $flags_t" 1 dg-test $test "$flags $flags_t" ${default-extra-flags} + gfortran-dg-rmunits $test cleanup-modules "" } } @@ -233,3 +234,25 @@ proc gfortran-dg-debug-runtest { target_compile trivial opt_opts testcases } { } } } + +# If the code has any "open" statements for numbered units, make sure +# no corresponding output file remains. Redundant remove operations +# are ok. + +proc gfortran-dg-rmunits { src } { + set openpat {open *\( *(?:unit *= *)?([0-9]+)} + set openmatches [igrep $src $openpat] + if {![string match "" $openmatches]} { + # verbose -log "Found \"$openmatches\"" + set deleted_units {} + foreach openmatch $openmatches { + regexp -nocase -- "$openpat" $openmatch match unit + if {[lsearch $deleted_units $unit] < 0} { + set rmfile "fort.$unit" + verbose -log "Deleting $rmfile" + remote_file target delete "fort.$unit" + lappend deleted_units $unit + } + } + } +}
Here's a general approach to handle PR116701. I considered adding manual deletions as quoted below and mentioned in the PR, but seeing the handling of "integer 8" in fortran-torture-execute I decided to follow that example: better scan the source for open-statements than relying on manual annotations and people remembering to add them for new test-cases. I hope the inclusion of gfortran-dg.exp in fortran-torture.exp is not controversial, but there's no fortran-specific testsuite file common to dg and classic-torture and also this placement is still in the "Utility routines" section of gfortran-dg.exp. (BTW, the C torture-tests changed to the dg framework some time ago - no more .x-files there and dg-directives actually work - there are some in gfortran.fortran-torture that are apparently ignored!) There's one further cleanup possible, removing the manual removal in open_errors_2.f90 (which should have used "target", not "build"). Works for (no regressions, manual check verifies functionality) cris-elf as well as native x86_64-linux-gnu. Ok to commit? -- >8 -- PR testsuite/116701 shows that left-behind files from unnamed gfortran open statements (named unit.N, where N = unit number) can interfere with the result of a subsequent run. While that's unlikely to happen for a "real" fortran target or a test with a deleting close-statement, test-cases should not rely on previous test-cases passing and not execute along different execution paths depending on earlier runs, even if the difference is benevolent. Most but not all fortran test-cases go through gfortran-dg-runtest (gfortran.dg) or fortran-torture-execute (gfortran.fortran-torture). However, the exceptions, with more complex framework and call-chains, either don't run or don't have open-statements, so a more complex solution doesn't seem worthwhile. If test-cases with open-statements are added later to those parts of the test-suite, calls to gfortran-dg-rmunits at the right spot may be added or worst case, "manual" cleanup-calls added, like: ! { dg-final { remote_file target delete "fort.10" } } PR testsuite/116701 * lib/gfortran-dg.exp (gfortran-dg-rmunits): New proc. (gfortran-dg-runtest): Call gfortran-dg-rmunits after executing test. * lib/fortran-torture.exp: Include gfortran-dg.exp. (fortran-torture-execute): Call gfortran-dg-rmunits after executing test. --- gcc/testsuite/lib/fortran-torture.exp | 3 +++ gcc/testsuite/lib/gfortran-dg.exp | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+)