Re: [PATCH] PR testsuite/69181: ensure expected multiline outputs is cleared per-test

[David Malcolm <dmalcolm at redhat dot com>]

On Sat, 2016-01-09 at 03:07 +0100, Bernd Schmidt wrote:
> On 01/09/2016 01:51 AM, David Malcolm wrote:
> > The root cause here is that the logic to reset the list of expected
> > multiline outputs was being run from:
> >    handle-multiline-outputs, called by
> >      prune.exp's prune_gcc_output
> > and none of that happens if the test is skipped by a target exclusion
> > in dg-do.
> 
> Thanks for tackling this.
> 
> > diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
> > index f9ec206..f778bca 100644
> > --- a/gcc/testsuite/lib/gcc-dg.exp
> > +++ b/gcc/testsuite/lib/gcc-dg.exp
> > @@ -836,6 +836,7 @@ if { [info procs saved-dg-test] == [list] } {
> >   	global testname_with_flags
> >   	global set_target_env_var
> >   	global keep_saved_temps_suffixes
> > +	global multiline_expected_outputs
> >
> >   	if { [ catch { eval saved-dg-test $args } errmsg ] } {
> >   	    set saved_info $errorInfo
> > @@ -871,6 +872,7 @@ if { [info procs saved-dg-test] == [list] } {
> >   	if [info exists testname_with_flags] {
> >   	    unset testname_with_flags
> >   	}
> > +	set multiline_expected_outputs []
> >       }
> >   }
> 
> I looked at this code, and there are two near-identical blocks which 
> reset all these variables. You are modifying only one of them, leaving 
> the one inside the if { catch } thing unchanged - is this intentional?

I'm not particularly strong at Tcl, but am I right in thinking that
given that we have this:

	if { [ catch { eval saved-dg-test $args } errmsg ] } {
	    (A) set and unset various things
	    error $errmsg $saved_info
	}
       (B) set and unset the same various things as (A)

that (B) will always be reached, and that the duplicates in (A) are
redundant? (unless they affect "error")

I see that this pattern was introduced back in r67696 aka
91a385a522a94154f9e0cd940c5937177737af02:

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 39ccaf6..c660eca 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2003-06-09  Mark Mitchell  <mark@codesourcery.com>
+
+       * lib/gcc-dg.exp (dg-test): Clear additional_files and
+       additional_sources.
+
 2003-05-21  David Taylor  <dtaylor@emc.com>
 
        * gcc.dg/Wpadded.c: New file.
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index aade663..1feadc4 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -294,3 +294,31 @@ proc dg-require-gc-sections { args } {
        return
     }
 }
+
+# We need to make sure that additional_files and additional_sources
+# are both cleared out after every test.  It is not enough to clear
+# them out *before* the next test run because gcc-target-compile gets
+# run directly from some .exp files (outside of any test).  (Those
+# uses should eventually be eliminated.) 
+
+# Because the DG framework doesn't provide a hook that is run at the
+# end of a test, we must replace dg-test with a wrapper.
+
+if { [info procs saved-dg-test] == [list] } {
+    rename dg-test saved-dg-test
+
+    proc dg-test { args } {
+       global additional_files
+       global additional_sources
+       global errorInfo
+
+       if { [ catch { eval saved-dg-test $args } errmsg ] } {
+           set saved_info $errorInfo
+           set additional_files ""
+           set additional_sources ""
+           error $errmsg $saved_info
+       }
+       set additional_files ""
+       set additional_sources ""
+    }
+}

and this pattern has been extended over the years.

I *could* add the
  set multiline_expected_outputs []
to the block guarded by the if {}, but it feels like cargo-culting to
me.  Am I missing something?


> Otherwise this looks reasonable IMO.
> 
> 
> Bernd