Re: [PATCH v2] gfortran testsuite: Remove unit-files in files having open-statements, PR116701

Wed Sep 25 19:47:00 GMT 2024

On 25 September 2024 13:51:07 CEST, Andre Vehreschild <vehre@gmx.de> wrote:
>Hi Hans-Peter,
>
>preface: I am not a testsuite nor an m4 expert.
>
>So I may be wrong in arguing that your changes look reasonable. I like the
>"automatic" clean-up process very much. So by me, ok for mainline. But you may
>want to wait for one other ok from some one who has more experience in
>the gfortran testsuite (yes, still wondering who that might be :-( ).

LGTM
Ok for trunk.

thanks,

>
>Thanks for the patch,
>	Andre
>
>On Wed, 25 Sep 2024 04:06:14 +0200
>Hans-Peter Nilsson <hp@axis.com> wrote:
>
>> Changes since v1:
>> - Rename gfortran-dg-rmunits to fortran-delete-unit-files.
>> - Move it to lib/fortran-modules.exp.
>> - Tweak commit message accordingly and mention cause of placement of
>>   the proc.
>> - Tweak proc comment to mention why keeping removals unique despite
>>   comment.
>>
>> 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 cris-elf (no regressions).  Version v1 was also
>> similarly regtested on native x86_64-linux-gnu.  Manual
>> checks have verified the unit-removal.
>>
>> 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
>> fortran-delete-unit-files at the right spot may be added or
>> worst case, "manual" cleanup-calls added, like:
>> ! { dg-final { remote_file target delete "fort.10" } }
>> Put the new proc in fortran-modules.exp since that's where other
>> common fortran-testsuite dejagnu-library functions are located.
>>
>> 	PR testsuite/116701
>> 	* lib/fortran-modules.exp (fortran-delete-unit-files): New proc.
>> 	* lib/gfortran-dg.exp (gfortran-dg-runtest): Call
>> 	fortran-delete-unit-files after executing test.
>> 	* lib/fortran-torture.exp (fortran-torture-execute): Ditto.
>> ---
>>  gcc/testsuite/lib/fortran-modules.exp | 21 +++++++++++++++++++++
>>  gcc/testsuite/lib/fortran-torture.exp |  2 ++
>>  gcc/testsuite/lib/gfortran-dg.exp     |  1 +
>>  3 files changed, 24 insertions(+)
>>
>> diff --git a/gcc/testsuite/lib/fortran-modules.exp
>> b/gcc/testsuite/lib/fortran-modules.exp index 158b16bada91..a7196f13ed22
>> 100644 --- a/gcc/testsuite/lib/fortran-modules.exp
>> +++ b/gcc/testsuite/lib/fortran-modules.exp
>> @@ -172,3 +172,24 @@ proc igrep { args } {
>>      }
>>      return $grep_out
>>  }
>> +
>> +# If the code has any "open" statements for numbered units, make sure
>> +# no corresponding output file remains.  Redundant remove operations
>> +# are ok, but duplicate removals look sloppy, so track for uniqueness.
>> +proc fortran-delete-unit-files { 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
>> +	    }
>> +	}
>> +    }
>> +}
>> diff --git a/gcc/testsuite/lib/fortran-torture.exp
>> b/gcc/testsuite/lib/fortran-torture.exp index 66f5bc822232..0727fb4fb0a6
>> 100644 --- a/gcc/testsuite/lib/fortran-torture.exp
>> +++ b/gcc/testsuite/lib/fortran-torture.exp
>> @@ -332,6 +332,8 @@ proc fortran-torture-execute { src } {
>>  	    catch { remote_file build delete $executable }
>>          }
>>  	$status "$testcase execution, $option"
>> +
>> +	fortran-delete-unit-files $src
>>      }
>>      cleanup-modules ""
>>  }
>> diff --git a/gcc/testsuite/lib/gfortran-dg.exp
>> b/gcc/testsuite/lib/gfortran-dg.exp index fcba95dc3961..2edc09e5c995 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}
>> +	    fortran-delete-unit-files $test
>>  	    cleanup-modules ""
>>  	}
>>      }
>
>
>--
>Andre Vehreschild * Email: vehre ad gmx dot de