| Message ID | a59a0ff7-6fd7-0f0c-4a44-6b59ca6dcfe6@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Apr 22, 2017, at 10:49 AM, Tom de Vries <Tom_deVries@mentor.com> wrote: > > This patch adds a directive dg-save-linenr with argument varname, that saves the line number of the directive in a variable varname, which can be used as line number in dg directives. > RFC: > - good idea? Seems reasonable to me. I'd like to encourage, like it, hate it comments from others and see what others think.
On Sat, 2017-04-22 at 19:49 +0200, Tom de Vries wrote: > Hi, > > there are currently two types of line number supported in > dg-{error,warning,message,bogus} directives: absolute and relative. > With an absolute line number, it's immediately clear what line number > is > meant, but when a line is added at the start of the file, the line > number needs to be updated. With a relative line number, that > problem > is solved, but when relative line numbers become large, it becomes > less > clear what line it refers to, and when adding a line inbetween the > directive using the relative line number and the line it refers to, > the > relative line number still needs to be updated. > > This patch adds a directive dg-save-linenr with argument varname, > that > saves the line number of the directive in a variable varname, which > can > be used as line number in dg directives. > > Testing status: > - tested updated test-case objc.dg/try-catch-12.m > - ran tree-ssa.exp > > RFC: > - good idea? Excellent idea; thanks! There are various places where I'd find this useful. > - naming of directive dg-save-linenr (dg-linenr, dg-save-line-nr, > dg-save-lineno, dg-save-line-number, etc) How about just "dg-line"? (if it's not already taken) or "dg-name-line" / "dg-named-line" ? in that the directive is effectively giving the line a name, giving: [...] extern void some_func (int *); /* { dg-line some_func_decl } */ [...] /* { dg-message "but argument is of type" "" { target *-*-* } some_func_decl } */ > - allowed variable names (currently: start with letter, followed by > alphanumerical or underscore) Seems reasonable; lack of leading digit allows it to be distinguished from absolute and relative numbers. > - should we use a prefix symbol or some such when the variable is > used > (and possibly defined as well)? F.i.: > /* { dg-save-linenr %some_func_decl } *./ > /* { dg-message "but argument is of type" "" { target *-*-* } > %some_func_decl } */ These are sometimes called "sigils". I'd prefer not. > - error message formulation Nit: the new function should have a leading comment, explaining the usage. Thanks again Dave
On Mon, 2017-04-24 at 11:20 -0400, David Malcolm wrote: > On Sat, 2017-04-22 at 19:49 +0200, Tom de Vries wrote: > > Hi, > > > > there are currently two types of line number supported in > > dg-{error,warning,message,bogus} directives: absolute and relative. > > With an absolute line number, it's immediately clear what line > > number > > is > > meant, but when a line is added at the start of the file, the line > > number needs to be updated. With a relative line number, that > > problem > > is solved, but when relative line numbers become large, it becomes > > less > > clear what line it refers to, and when adding a line inbetween the > > directive using the relative line number and the line it refers to, > > the > > relative line number still needs to be updated. > > > > This patch adds a directive dg-save-linenr with argument varname, > > that > > saves the line number of the directive in a variable varname, which > > can > > be used as line number in dg directives. > > > > Testing status: > > - tested updated test-case objc.dg/try-catch-12.m > > - ran tree-ssa.exp > > > > RFC: > > - good idea? > > Excellent idea; thanks! There are various places where I'd find this > useful e.g. the test cases within https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01061.html
On 04/22/2017 11:49 AM, Tom de Vries wrote: > Hi, > > there are currently two types of line number supported in > dg-{error,warning,message,bogus} directives: absolute and relative. With > an absolute line number, it's immediately clear what line number is > meant, but when a line is added at the start of the file, the line > number needs to be updated. With a relative line number, that problem > is solved, but when relative line numbers become large, it becomes less > clear what line it refers to, and when adding a line inbetween the > directive using the relative line number and the line it refers to, the > relative line number still needs to be updated. > > This patch adds a directive dg-save-linenr with argument varname, that > saves the line number of the directive in a variable varname, which can > be used as line number in dg directives. > > Testing status: > - tested updated test-case objc.dg/try-catch-12.m > - ran tree-ssa.exp > > RFC: > - good idea? Yes. > - naming of directive dg-save-linenr (dg-linenr, dg-save-line-nr, > dg-save-lineno, dg-save-line-number, etc) No strong opinions. We often use "save" in various ways within the GCC source for similar concepts, so any of those work for me. > - allowed variable names (currently: start with letter, followed by > alphanumerical or underscore) Seems reasonable. > - should we use a prefix symbol or some such when the variable is used > (and possibly defined as well)? F.i.: > /* { dg-save-linenr %some_func_decl } *./ > /* { dg-message "but argument is of type" "" { target *-*-* } > %some_func_decl } */ I'm not sure what the value here would be. Jeff
Add dg-save-linenr 2017-04-22 Tom de Vries <tom@codesourcery.com> * lib/gcc-dg.exp (cleanup-after-saved-dg-test): Cleanup line number variables. (dg-save-linenr): New proc. (process-message): Handle line number variables. * objc.dg/try-catch-12.m: Use dg-save-linenr. --- gcc/testsuite/lib/gcc-dg.exp | 50 ++++++++++++++++++++++++++++++++---- gcc/testsuite/objc.dg/try-catch-12.m | 8 +++--- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp index 83c38cf..555e083 100644 --- a/gcc/testsuite/lib/gcc-dg.exp +++ b/gcc/testsuite/lib/gcc-dg.exp @@ -902,6 +902,7 @@ if { [info procs saved-dg-test] == [list] } { global keep_saved_temps_suffixes global multiline_expected_outputs global freeform_regexps + global save_linenr_varnames set additional_files "" set additional_sources "" @@ -928,6 +929,13 @@ if { [info procs saved-dg-test] == [list] } { } set multiline_expected_outputs [] set freeform_regexps [] + + if { [info exists save_linenr_varnames] } { + foreach varname $save_linenr_varnames { + eval unset $varname + } + unset save_linenr_varnames + } } proc dg-test { args } { @@ -979,6 +987,24 @@ if { [info procs saved-dg-error] == [list] \ } } +proc dg-save-linenr { linenr varname } { + set org_varname $varname + set varname "saved_linenr_$varname" + eval global $varname + eval set var_defined [info exists $varname] + if { $var_defined } { + eval set deflinenr \$$varname + error "dg-save-linenr var $org_varname defined at line $linenr, but previously defined at line $deflinenr" + return + } + eval set $varname $linenr + if { [info exists save_linenr_varnames] } { + lappend save_linenr_varnames $varname + } else { + set save_linenr_varnames [list $varname] + } +} + # Modify the regular expression saved by a DejaGnu message directive to # include a prefix and to force the expression to match a single line. # MSGPROC is the procedure to call. @@ -988,11 +1014,25 @@ if { [info procs saved-dg-error] == [list] \ 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] + if { [llength $dgargs] == 5 } { + if { [regsub "^\.\[+-\](\[0-9\]+)$" [lindex $dgargs 4] "\\1" num] } { + # Handle relative line specification, .+1 or .-1 etc. + set num [expr [lindex $dgargs 0] [string index [lindex $dgargs 4] 1] $num] + set dgargs [lreplace $dgargs 4 4 $num] + } elseif { [regsub "^(\[a-zA-Z\]\[a-zA-Z0-9_\]*)$" [lindex $dgargs 4] "\\1" varname] } { + # Handle linenr variable defined by dg-save-linenr + set org_varname $varname + set varname "saved_linenr_$varname" + eval global $varname + eval set var_defined [info exists $varname] + if { ! $var_defined } { + set linenr [expr [lindex $dgargs 0]] + error "dg-save-linenr var $org_varname used at line $linenr, but not defined" + return + } + eval set num \$$varname + set dgargs [lreplace $dgargs 4 4 $num] + } } # Process the dg- directive, including adding the regular expression diff --git a/gcc/testsuite/objc.dg/try-catch-12.m b/gcc/testsuite/objc.dg/try-catch-12.m index 61e2703..558ad02 100644 --- a/gcc/testsuite/objc.dg/try-catch-12.m +++ b/gcc/testsuite/objc.dg/try-catch-12.m @@ -9,7 +9,7 @@ - (void) testSpoon; @end -extern void some_func (int *); +extern void some_func (int *); /* { dg-save-linenr some_func_decl } */ @implementation TestMyTests - (void) testSpoon { @@ -21,7 +21,7 @@ extern void some_func (int *); typeof(i) j = 6; typeof(q) k = 66; some_func (&j); /* { dg-warning "discards .volatile. qualifier from pointer target type" } */ - /* { dg-message "but argument is of type" "" { target *-*-* } 12 } */ + /* { dg-message "but argument is of type" "" { target *-*-* } some_func_decl } */ some_func (&k); } @catch (id exc) { @@ -37,7 +37,7 @@ extern void some_func (int *); some_func (&j); /* { dg-warning "discards .volatile. qualifier from pointer target type" } */ /* The following is disabled as it is already checked above and the testsuites seems to count multiple different identical errors on the same line only once */ - /* dg-message "but argument is of type" "" { target *-*-* } 12 */ + /* dg-message "but argument is of type" "" { target *-*-* } some_func_decl */ } @catch (id exc) { @throw; @@ -51,7 +51,7 @@ extern void some_func (int *); some_func (&j); /* { dg-warning "discards .volatile. qualifier from pointer target type" } */ /* The following is disabled as it is already checked above and the testsuites seems to count multiple different identical errors on the same line only once */ - /* dg-message "but argument is of type" "" { target *-*-* } 12 */ + /* dg-message "but argument is of type" "" { target *-*-* } some_func_decl */ some_func (&k); } @catch (id exc) {
Hi, there are currently two types of line number supported in dg-{error,warning,message,bogus} directives: absolute and relative. With an absolute line number, it's immediately clear what line number is meant, but when a line is added at the start of the file, the line number needs to be updated. With a relative line number, that problem is solved, but when relative line numbers become large, it becomes less clear what line it refers to, and when adding a line inbetween the directive using the relative line number and the line it refers to, the relative line number still needs to be updated. This patch adds a directive dg-save-linenr with argument varname, that saves the line number of the directive in a variable varname, which can be used as line number in dg directives. Testing status: - tested updated test-case objc.dg/try-catch-12.m - ran tree-ssa.exp RFC: - good idea? - naming of directive dg-save-linenr (dg-linenr, dg-save-line-nr, dg-save-lineno, dg-save-line-number, etc) - allowed variable names (currently: start with letter, followed by alphanumerical or underscore) - should we use a prefix symbol or some such when the variable is used (and possibly defined as well)? F.i.: /* { dg-save-linenr %some_func_decl } *./ /* { dg-message "but argument is of type" "" { target *-*-* } %some_func_decl } */ - error message formulation Thanks, - Tom