diff mbox

Extend dg-{error,warning,message,bogus} line specification to allow relative line numbers

Message ID 20160922200546.GH7282@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Sept. 22, 2016, 8:05 p.m. UTC
Hi!

This is something I've been unhappy for a long time with, and finally got to
write something for it.
When some test expects more than one error or warning or message on the same
source line, people have to use absolute line number on the dg-* directives
that is not on the right line, as DejaGNU handles just . (current line, the
default if the directive doesn't have all the arguments), 0 (no expected
line) and <number> for absolute line number.

This patch extends it, so one can write relative line numbers, . +/- <number>
as a single argument, say .-1 for the previous line number, .+1 for the next line
number, etc.  While one still has to supply the comment and target/xfail
arguments, the advantage of doing this is that if you adjust the testcase,
say add a line somewhere early, etc., you don't have to renumber all the
line numbers, and from what I saw in some clang testcases, it can be also
useful if multiple errors are issued for consecutive lines with different
wordings depending on some target supports etc. macros, one can e.g. use
  stmt1;	/* { dg-error "..." "" { target c } } */
  stmt2;	/* { dg-error "..." "" { target c } } */
  stmt3;	/* { dg-error "..." "" { target c } } */
		/* { dg-error "..." "" { target c++ } .-3 } */
		/* { dg-error "..." "" { target c++ } .-3 } */
		/* { dg-error "..." "" { target c++ } .-3 } */
which would be more readable than intermix the two sets of diagnostics.

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

2016-09-22  Jakub Jelinek  <jakub@redhat.com>

	* lib/gcc-dg.exp (process-message): Support relative line number
	notation - .+4 or .-1 etc.
	* gcc.dg/dg-test-1.c: New test.


	Jakub

Comments

Mike Stump Sept. 22, 2016, 11:41 p.m. UTC | #1
On Sep 22, 2016, at 1:05 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> This is something I've been unhappy for a long time with

:-)  Me too.

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

Ok.  Thanks.
Dominik Vogt Sept. 23, 2016, 2:43 p.m. UTC | #2
On Thu, Sep 22, 2016 at 10:05:46PM +0200, Jakub Jelinek wrote:
> This is something I've been unhappy for a long time with, and finally got to
> write something for it.


> When some test expects more than one error or warning or message on the same
> source line, people have to use absolute line number on the dg-* directives
> that is not on the right line, as DejaGNU handles just . (current line, the
> default if the directive doesn't have all the arguments), 0 (no expected
> line) and <number> for absolute line number.

Great!

But could this patch be responsible with some dg-error related
test errors on s390x that are present with current HEAD?  E.g.
(Sorry for the linebreaks that vim has inserted).

--
spawn -ignore SIGHUP /home/vogt/src/gcc/build-master/gcc/xgcc
-B/home/vogt/src/\
gcc/build-master/gcc/
/home/vogt/src/gcc/gcc/testsuite/gcc.target/s390/hotpatch\
-compile-1.c -fno-diagnostics-show-caret -fdiagnostics-color=never
-O3 -mzarch \
-mhotpatch=-1,0 -S -m64 -o hotpatch-compile-1.s^M 
cc1: error: arguments to '-mhotpatch=n,m' should be non-negative
integers^M 
compiler exited with status 1 
output is: 
cc1: error: arguments to '-mhotpatch=n,m' should be non-negative
integers^M 
 
FAIL: gcc.target/s390/hotpatch-compile-1.c  (test for errors, line
1) 
FAIL: gcc.target/s390/hotpatch-compile-1.c (test for excess
errors) 
Excess errors: 
cc1: error: arguments to '-mhotpatch=n,m' should be non-negative
integers
--

So, the test expects an error:

  /* { dg-error "arguments to .-mhotpatch=n,m. should be non-negative integers" "" { target *-*-* } 1 } */ 

but no longer matches the error that really occurs?

Ciao

Dominik ^_^  ^_^
Jakub Jelinek Sept. 23, 2016, 2:48 p.m. UTC | #3
On Fri, Sep 23, 2016 at 03:43:11PM +0100, Dominik Vogt wrote:
> Great!
> 
> But could this patch be responsible with some dg-error related
> test errors on s390x that are present with current HEAD?  E.g.
> (Sorry for the linebreaks that vim has inserted).

Very unlikely.  Are you sure it appeared only today and not more than 2
weeks ago with
        PR middle-end/77475
        * toplev.c (process_options): Temporarily set input_location
        to UNKNOWN_LOCATION around targetm.target_option.override () call.
change (also mine)?  All such dg-error lines need to be changed to use
line number 0 (i.e. expect the errors not to be on the first line of the
source which makes no sense, but without any source location, as the errors
appear on the command line, not in any sources).

> So, the test expects an error:
> 
>   /* { dg-error "arguments to .-mhotpatch=n,m. should be non-negative integers" "" { target *-*-* } 1 } */ 

So it should be
   /* { dg-error "arguments to .-mhotpatch=n,m. should be non-negative integers" "" { target *-*-* } 0 } */
instead.

	Jakub
Dominik Vogt Sept. 23, 2016, 3:10 p.m. UTC | #4
On Fri, Sep 23, 2016 at 04:48:36PM +0200, Jakub Jelinek wrote:
> On Fri, Sep 23, 2016 at 03:43:11PM +0100, Dominik Vogt wrote:
> > Great!
> > 
> > But could this patch be responsible with some dg-error related
> > test errors on s390x that are present with current HEAD?  E.g.
> > (Sorry for the linebreaks that vim has inserted).
> 
> Very unlikely.  Are you sure it appeared only today and not more than 2
> weeks ago with
>         PR middle-end/77475
>         * toplev.c (process_options): Temporarily set input_location
>         to UNKNOWN_LOCATION around targetm.target_option.override () call.
> change (also mine)?  All such dg-error lines need to be changed to use
> line number 0 (i.e. expect the errors not to be on the first line of the
> source which makes no sense, but without any source location, as the errors
> appear on the command line, not in any sources).

Yeah, thanks a lot for pointing this out, that's the right fix.
Saved me bisecting this.

> > So, the test expects an error:
> > 
> >   /* { dg-error "arguments to .-mhotpatch=n,m. should be non-negative integers" "" { target *-*-* } 1 } */ 
> 
> So it should be
>    /* { dg-error "arguments to .-mhotpatch=n,m. should be non-negative integers" "" { target *-*-* } 0 } */
> instead.

Ciao

Dominik ^_^  ^_^
diff mbox

Patch

--- gcc/testsuite/lib/gcc-dg.exp.jj	2016-06-24 12:59:19.000000000 +0200
+++ gcc/testsuite/lib/gcc-dg.exp	2016-09-22 17:29:21.912995332 +0200
@@ -986,6 +986,13 @@  if { [info procs saved-dg-error] == [lis
 proc process-message { msgproc msgprefix dgargs } {
     upvar dg-messages dg-messages
 
+    # Handle relative line specification, .+1 or .-1 etc.
+    if { [llength $dgargs] == 5
+	 && [regsub "^\.\[+-\](\[0-9\])$" [lindex $dgargs 4] "\\1" num] } {
+	set num [expr [lindex $dgargs 0] [string index [lindex $dgargs 4] 1] $num]
+	set dgargs [lreplace $dgargs 4 4 $num]
+    }
+
     # Process the dg- directive, including adding the regular expression
     # to the new message entry in dg-messages.
     set msgcnt [llength ${dg-messages}]
--- gcc/testsuite/gcc.dg/dg-test-1.c.jj	2016-09-22 17:19:39.984407351 +0200
+++ gcc/testsuite/gcc.dg/dg-test-1.c	2016-09-22 17:22:14.000000000 +0200
@@ -0,0 +1,18 @@ 
+/* Test relative line number specification extensions over what DejaGNU supports.  */
+/* { dg-do compile } */
+/* { dg-options "-Wunused-parameter" } */
+
+void
+foo (void)
+{			/* { dg-error "'a' undeclared" "err1" { target *-*-* } .+1 } */
+  int z = a + b + c + d;/* { dg-error "'b' undeclared" "err2" { target *-*-* } . } */
+}			/* { dg-error "'c' undeclared" "err3" { target *-*-* } .-1 } */
+
+
+/* { dg-error "'d' undeclared" "err4" { target *-*-* } .-4 } */
+/* { dg-warning "unused parameter 'e'" "warn1" { target *-*-* } .+3 } */
+
+void				/* { dg-warning "unused parameter 'f'" "warn2" { target *-*-* } .+1 } */
+bar (int e, int f, int g, int h)/* { dg-warning "unused parameter 'g'" "warn3" { target *-*-* } . } */
+{				/* { dg-warning "unused parameter 'h'" "warn4" { target *-*-* } .-1 } */
+}