[PATCH] Fix up and improve allow_blank_lines testsuite handling (PR other/69006, PR testsuite/88920)

[PATCH] Fix up and improve allow_blank_lines testsuite handling (PR other/69006, PR testsuite/88920)

[Jakub Jelinek]

Hi!

The following patch fixes various testsuite issues related to
allow_blank_lines.

One issue is that even when llvm_binutils effective target is now cached so
it is checked just once (per runtest invocation), if the first test invoked
emits some diagnostics, that diagnostics in the log file appears before the
fatal error: GCC is not configured to support amdgcn-unknown-amdhsa as offload target
diagnostics and related command line, while the FAIL appears after it.  That
is because llvm_binutils is tested in gcc-dg-prune which happens after the
output has been gathered.  Fixed by doing llvm_binutils when gcc-dg.exp is
read rather than on first gcc-dg-prune.

Another issue is that once any test uses dg-allow-blank-lines-in-output
directive (there are several such in the testsuite), all following tests in
the same runtest instance already ignore blank lines rather than checking
that again.  This is because of a bug in the original PR69006 patch, which
contained code like
if { !$allow_blank_lines } {
  do various things
  set allow_blank_lines 0
}
The set allow_blank_lines 0 in there doesn't make a difference, we've
already checked that it is zero and do various things code doesn't touch it.
What we IMHO want is to reset allow_blank_lines to 0 if it was non-zero.
To make it work together with doing llvm_binutils only once, the global now
has multiple values
0 - disallow blank lines
1 - allow them for a single test only, reset after testing it in gcc-dg-prune
2 - allow it for all tests (llvm_binutils)

That change broke the gdc.test testsuite, which relies on allowing blank
lines and like with other tests, just did it once and assumed it will affect
all following tests.  This change sets it to 2 before those tests (so that
it is not reset every time) and restores back at the end of those tests (so
that further tests in the same runtest instance care about extraneous blank
lines).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2019-02-13  Jakub Jelinek  <jakub@redhat.com>

	PR other/69006
	PR testsuite/88920
	* lib/gcc-dg.exp: If llvm_binutils effective target, set
	allow_blank_lines to 2 during initialization.
	(dg-allow-blank-lines-in-output): Set allow_blank_lines to 1 only if
	it was previously zero.
	(gcc-dg-prune): Don't check for llvm_binutils effective target here.
	Clear allow_blank_lines afterwards whenever it was 1.
	* gdc.test/gdc-test.exp (dmd2dg): Don't call
	dg-allow-blank-lines-in-output here.
	(gdc-do-test): Set allow_blank_lines to 3 if it is 0 before running
	the tests and restore it back at the end.

--- gcc/testsuite/lib/gcc-dg.exp.jj	2019-01-17 17:11:17.262455056 +0100
+++ gcc/testsuite/lib/gcc-dg.exp	2019-02-12 20:38:51.554804981 +0100
@@ -346,12 +346,18 @@ proc gcc-dg-test { prog do_what extra_to
 # However, there are some ways for them to validly occur.
 set allow_blank_lines 0
 
+if { [check_effective_target_llvm_binutils] } {
+    set allow_blank_lines 2
+}
+
 # A command for use by testcases to mark themselves as expecting
 # blank lines in the output.
 
 proc dg-allow-blank-lines-in-output { args } {
     global allow_blank_lines
-    set allow_blank_lines 1
+    if { !$allow_blank_lines } {
+	set allow_blank_lines 1
+    }
 }
 
 proc gcc-dg-prune { system text } {
@@ -363,12 +369,14 @@ proc gcc-dg-prune { system text } {
 
     # Complain about blank lines in the output (PR other/69006)
     global allow_blank_lines
-    if { !$allow_blank_lines && ![check_effective_target_llvm_binutils]} {
+    if { !$allow_blank_lines } {
 	set num_blank_lines [llength [regexp -all -inline "\n\n" $text]]
 	if { $num_blank_lines } {
 	    global testname_with_flags
 	    fail "$testname_with_flags $num_blank_lines blank line(s) in output"
 	}
+    }
+    if { $allow_blank_lines == 1 } {
 	set allow_blank_lines 0
     }
 
--- gcc/testsuite/gdc.test/gdc-test.exp.jj	2019-01-02 00:12:15.140056932 +0100
+++ gcc/testsuite/gdc.test/gdc-test.exp	2019-02-13 09:52:54.149256688 +0100
@@ -277,9 +277,6 @@ proc dmd2dg { base test } {
     set out_line "// { dg-prune-output .* }"
     puts $fdout $out_line
 
-    # Since GCC 6-20160131 blank lines are not allowed in the output by default.
-    dg-allow-blank-lines-in-output { 1 }
-
     # Compilable files are successful if an output is generated.
     # Fail compilable are successful if an output is not generated.
     # Runnable must compile, link, and return 0 to be successful by default.
@@ -360,6 +357,13 @@ proc gdc-do-test { } {
     # Initialize `dg'.
     dg-init
 
+    # Allow blank linkes in output for all of gdc.test.
+    global allow_blank_lines
+    set save_allow_blank_lines $allow_blank_lines
+    if { !$allow_blank_lines } {
+	set allow_blank_lines 2
+    }
+
     # Create gdc.test link so test names include that subdir.
     catch { file link $subdir . }
 
@@ -430,6 +434,8 @@ proc gdc-do-test { } {
         file delete $filename
     }
 
+    set allow_blank_lines $save_allow_blank_lines
+
     # All done.
     dg-finish
 }

	Jakub

Re: [PATCH] Fix up and improve allow_blank_lines testsuite handling (PR other/69006, PR testsuite/88920)

[Andrew Stubbs]

On 13/02/2019 09:09, Jakub Jelinek wrote:
> To make it work together with doing llvm_binutils only once, the global now
> has multiple values
> 0 - disallow blank lines
> 1 - allow them for a single test only, reset after testing it in gcc-dg-prune
> 2 - allow it for all tests (llvm_binutils)

FWIW, this scheme and patch looks good to me, although probably the 
meaning of the numbers ought to be documented somewhere in the code.

Andrew

[PATCH] Fix up and improve allow_blank_lines testsuite handling (PR other/69006, PR testsuite/88920, take 2)

[Jakub Jelinek]

On Wed, Feb 13, 2019 at 10:22:14AM +0000, Andrew Stubbs wrote:
> On 13/02/2019 09:09, Jakub Jelinek wrote:
> > To make it work together with doing llvm_binutils only once, the global now
> > has multiple values
> > 0 - disallow blank lines
> > 1 - allow them for a single test only, reset after testing it in gcc-dg-prune
> > 2 - allow it for all tests (llvm_binutils)
> 
> FWIW, this scheme and patch looks good to me, although probably the meaning
> of the numbers ought to be documented somewhere in the code.

Here is an updated patch that documents it.  Bootstrapped/regtested on
x86_64-linux and i686-linux, ok for trunk?

2019-02-13  Jakub Jelinek  <jakub@redhat.com>

	PR other/69006
	PR testsuite/88920
	* lib/gcc-dg.exp: If llvm_binutils effective target, set
	allow_blank_lines to 2 during initialization.
	(dg-allow-blank-lines-in-output): Set allow_blank_lines to 1 only if
	it was previously zero.
	(gcc-dg-prune): Don't check for llvm_binutils effective target here.
	Clear allow_blank_lines afterwards whenever it was 1.
	* gdc.test/gdc-test.exp (dmd2dg): Don't call
	dg-allow-blank-lines-in-output here.
	(gdc-do-test): Set allow_blank_lines to 3 if it is 0 before running
	the tests and restore it back at the end.

--- gcc/testsuite/lib/gcc-dg.exp.jj	2019-01-17 17:11:17.262455056 +0100
+++ gcc/testsuite/lib/gcc-dg.exp	2019-02-13 13:15:39.701772603 +0100
@@ -344,14 +344,24 @@ proc gcc-dg-test { prog do_what extra_to
 # Global: should blank lines be allowed in the output?
 # By default, they should not be.  (PR other/69006)
 # However, there are some ways for them to validly occur.
+# If this variable is 0, blank lines are not allowed in output,
+# if it is 1, they are allowed for a single testcase only and gcc-dg-prune
+# will clear it again after checking it, if it is 2, they are disabled
+# for all tests.
 set allow_blank_lines 0
 
+if { [check_effective_target_llvm_binutils] } {
+    set allow_blank_lines 2
+}
+
 # A command for use by testcases to mark themselves as expecting
 # blank lines in the output.
 
 proc dg-allow-blank-lines-in-output { args } {
     global allow_blank_lines
-    set allow_blank_lines 1
+    if { !$allow_blank_lines } {
+	set allow_blank_lines 1
+    }
 }
 
 proc gcc-dg-prune { system text } {
@@ -363,12 +373,14 @@ proc gcc-dg-prune { system text } {
 
     # Complain about blank lines in the output (PR other/69006)
     global allow_blank_lines
-    if { !$allow_blank_lines && ![check_effective_target_llvm_binutils]} {
+    if { !$allow_blank_lines } {
 	set num_blank_lines [llength [regexp -all -inline "\n\n" $text]]
 	if { $num_blank_lines } {
 	    global testname_with_flags
 	    fail "$testname_with_flags $num_blank_lines blank line(s) in output"
 	}
+    }
+    if { $allow_blank_lines == 1 } {
 	set allow_blank_lines 0
     }
 
--- gcc/testsuite/gdc.test/gdc-test.exp.jj	2019-01-02 00:12:15.140056932 +0100
+++ gcc/testsuite/gdc.test/gdc-test.exp	2019-02-13 09:52:54.149256688 +0100
@@ -277,9 +277,6 @@ proc dmd2dg { base test } {
     set out_line "// { dg-prune-output .* }"
     puts $fdout $out_line
 
-    # Since GCC 6-20160131 blank lines are not allowed in the output by default.
-    dg-allow-blank-lines-in-output { 1 }
-
     # Compilable files are successful if an output is generated.
     # Fail compilable are successful if an output is not generated.
     # Runnable must compile, link, and return 0 to be successful by default.
@@ -360,6 +357,13 @@ proc gdc-do-test { } {
     # Initialize `dg'.
     dg-init
 
+    # Allow blank linkes in output for all of gdc.test.
+    global allow_blank_lines
+    set save_allow_blank_lines $allow_blank_lines
+    if { !$allow_blank_lines } {
+	set allow_blank_lines 2
+    }
+
     # Create gdc.test link so test names include that subdir.
     catch { file link $subdir . }
 
@@ -430,6 +434,8 @@ proc gdc-do-test { } {
         file delete $filename
     }
 
+    set allow_blank_lines $save_allow_blank_lines
+
     # All done.
     dg-finish
 }


	Jakub

Re: [PATCH] Fix up and improve allow_blank_lines testsuite handling (PR other/69006, PR testsuite/88920, take 2)

[Iain Buclaw]

On Wed, 13 Feb 2019 at 14:37, Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Wed, Feb 13, 2019 at 10:22:14AM +0000, Andrew Stubbs wrote:
> > On 13/02/2019 09:09, Jakub Jelinek wrote:
> > > To make it work together with doing llvm_binutils only once, the global now
> > > has multiple values
> > > 0 - disallow blank lines
> > > 1 - allow them for a single test only, reset after testing it in gcc-dg-prune
> > > 2 - allow it for all tests (llvm_binutils)
> >
> > FWIW, this scheme and patch looks good to me, although probably the meaning
> > of the numbers ought to be documented somewhere in the code.
>
> Here is an updated patch that documents it.  Bootstrapped/regtested on
> x86_64-linux and i686-linux, ok for trunk?
>

I have no objections.

Iain.

Re: [PATCH] Fix up and improve allow_blank_lines testsuite handling (PR other/69006, PR testsuite/88920, take 2)

[David Malcolm]

On Wed, 2019-02-13 at 20:08 +0100, Iain Buclaw wrote:
> On Wed, 13 Feb 2019 at 14:37, Jakub Jelinek <jakub@redhat.com> wrote:
> > 
> > On Wed, Feb 13, 2019 at 10:22:14AM +0000, Andrew Stubbs wrote:
> > > On 13/02/2019 09:09, Jakub Jelinek wrote:
> > > > To make it work together with doing llvm_binutils only once,
> > > > the global now
> > > > has multiple values
> > > > 0 - disallow blank lines
> > > > 1 - allow them for a single test only, reset after testing it
> > > > in gcc-dg-prune
> > > > 2 - allow it for all tests (llvm_binutils)
> > > 
> > > FWIW, this scheme and patch looks good to me, although probably
> > > the meaning
> > > of the numbers ought to be documented somewhere in the code.
> > 
> > Here is an updated patch that documents it.  Bootstrapped/regtested
> > on
> > x86_64-linux and i686-linux, ok for trunk?
> > 
> 
> I have no objections.
> 
> Iain.

Likewise, LGTM (FWIW).

Dave

Re: [PATCH] Fix up and improve allow_blank_lines testsuite handling (PR other/69006, PR testsuite/88920)

[Mike Stump]

On Feb 13, 2019, at 1:09 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> ok for trunk?

Ok.

Re: [PATCH] Fix up and improve allow_blank_lines testsuite handling (PR other/69006, PR testsuite/88920, take 2)

[Mike Stump]

On Feb 13, 2019, at 5:37 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Here is an updated patch that documents it.  Bootstrapped/regtested on
> x86_64-linux and i686-linux, ok for trunk?

Ok.