[RFC] Dejagnu patch to handle multi-line directives

[RFC] Dejagnu patch to handle multi-line directives

[Tom de Vries]

Hi,

one thing that has bothered me on a regular basis is the inability to 
spread long dejagnu directives over multiple lines.

I've written a demonstrator patch (for the dejagnu sources) and tested 
it by splitting this 108 chars line:
...
/* { dg-additional-options "-DSTACK_SIZE=[dg-effective-target-value 
stack_size]" { target { stack_size } } } */
...
into:
...
/* { dg-additional-options }
    { dg-dc "-DSTACK_SIZE=[dg-effective-target-value stack_size]" }
    { dg-dc { target { stack_size } } } */
...

Good idea to fix this?

Good idea to fix this like this?

If so, any other comments, before I suggest this at dejagnu project?

Thanks,
- Tom

Re: [RFC] Dejagnu patch to handle multi-line directives

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

[ attached patch ]

On 06/10/2017 09:57 AM, Tom de Vries wrote:
> Hi,
> 
> one thing that has bothered me on a regular basis is the inability to 
> spread long dejagnu directives over multiple lines.
> 
> I've written a demonstrator patch (for the dejagnu sources) and tested 
> it by splitting this 108 chars line:
> ...
> /* { dg-additional-options "-DSTACK_SIZE=[dg-effective-target-value 
> stack_size]" { target { stack_size } } } */
> ...
> into:
> ...
> /* { dg-additional-options }
>     { dg-dc "-DSTACK_SIZE=[dg-effective-target-value stack_size]" }
>     { dg-dc { target { stack_size } } } */
> ...
> 
> Good idea to fix this?
> 
> Good idea to fix this like this?
> 
> If so, any other comments, before I suggest this at dejagnu project?
> 
> Thanks,
> - Tom
> 


[-- Attachment #2: 0001-Add-dg-dc-support.patch --]
[-- Type: text/x-patch, Size: 1827 bytes --]

Add dg-dc support

---
 lib/dg.exp | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 53 insertions(+), 4 deletions(-)

diff --git a/lib/dg.exp b/lib/dg.exp
index 7a894cb..67f46ab 100644
--- a/lib/dg.exp
+++ b/lib/dg.exp
@@ -181,15 +181,64 @@ proc dg-format-linenum { linenum } {
 # we return:
 #
 # { dg-prms-id 1 1234 } { dg-build 2 fatal "some comment" }
+#
+# Directive dg-dc (short for dg-directive-continue) can be used for multi-line
+# directives.  This:
+#
+# /* { dg-x a b c } */
+#
+# is equivalent to:
+#
+# /* { dg-x } */
+# /* { dg-dc a b } */
+# /* { dg-dc c } */
+#
+# and to:
+#
+# /* { dg-x a } */
+# /* { dg-dc b c} */
 
 proc dg-get-options { prog } {
     set result ""
-
-    set tmp [grep $prog "{\[ \t\]\+dg-\[-a-z\]\+\[ \t\]\+.*\[ \t\]\+}" line]
+    set cmd_prev ""
+
+    set grep_pattern [join {
+	"{"
+	"\[ \t\]\+"
+	"dg-\[-a-z\]\+"
+	"\[ \t\]\+"
+	"(.*\[ \t\]\+)?"
+	"}"
+    } ""]
+    set tmp [grep $prog $grep_pattern line]
     if {![string match "" $tmp]} {
+	set pattern [join {
+	    "(\[0-9\]+)"
+	    "\[ \t\]+"
+	    "{"
+	    "\[ \t\]+"
+	    "(dg-\[-a-z\]+)"
+	    "\[ \t\]+"
+	    "((.*)\[ \t\]+)?"
+	    "}"
+	    "\[^\}\]*"
+	    "(\n|$)"
+	} ""]
 	foreach i $tmp {
-	    regexp "(\[0-9\]+)\[ \t\]+{\[ \t\]+(dg-\[-a-z\]+)\[ \t\]+(.*)\[ \t\]+}\[^\}\]*(\n|$)" $i i line cmd args
-	    append result " { $cmd $line $args }"
+	    regexp $pattern $i dummy line cmd ws_args args
+	    if { "$cmd" == "dg-dc" } {
+		set args_prev "$args_prev $args"
+	    } else {
+		if { "$cmd_prev" != "" } {
+		    append result " { $cmd_prev $line_prev $args_prev }"
+		}
+		set cmd_prev $cmd
+		set line_prev $line
+		set args_prev "$args"
+	    }
+	}
+	if { "$cmd_prev" != "" } {
+	    append result " { $cmd_prev $line_prev $args_prev }"
 	}
     }
     return $result

Re: [RFC] Dejagnu patch to handle multi-line directives

[Segher Boessenkool]

Hi!
On Sat, Jun 10, 2017 at 10:03:04AM +0200, Tom de Vries wrote:
> >/* { dg-additional-options }
> >    { dg-dc "-DSTACK_SIZE=[dg-effective-target-value stack_size]" }
> >    { dg-dc { target { stack_size } } } */
> >...
> >
> >Good idea to fix this?

I like it.  What is the exact semantics though?  What directives does
this not work on?

>  proc dg-get-options { prog } {
>      set result ""
> -
> -    set tmp [grep $prog "{\[ \t\]\+dg-\[-a-z\]\+\[ \t\]\+.*\[ \t\]\+}" line]
> +    set cmd_prev ""
> +
> +    set grep_pattern [join {
> +	"{"
> +	"\[ \t\]\+"
> +	"dg-\[-a-z\]\+"
> +	"\[ \t\]\+"
> +	"(.*\[ \t\]\+)?"
> +	"}"
> +    } ""]
> +    set tmp [grep $prog $grep_pattern line]

If you use {} instead of "" you don't need all these backslashes.  If you
use expanded syntax (see "man tcl re_syntax") you can make it even more
readable (and you don't need the join) (but it is a short regexp anyway).
You might want to use \s instead of [ \t].


Segher

Re: [RFC] Dejagnu patch to handle multi-line directives

[Richard Sandiford]

Tom de Vries <Tom_deVries@mentor.com> writes:
> [ attached patch ]
>
> On 06/10/2017 09:57 AM, Tom de Vries wrote:
>> Hi,
>> 
>> one thing that has bothered me on a regular basis is the inability to 
>> spread long dejagnu directives over multiple lines.
>> 
>> I've written a demonstrator patch (for the dejagnu sources) and tested 
>> it by splitting this 108 chars line:
>> ...
>> /* { dg-additional-options "-DSTACK_SIZE=[dg-effective-target-value 
>> stack_size]" { target { stack_size } } } */
>> ...
>> into:
>> ...
>> /* { dg-additional-options }
>>     { dg-dc "-DSTACK_SIZE=[dg-effective-target-value stack_size]" }
>>     { dg-dc { target { stack_size } } } */
>> ...
>> 
>> Good idea to fix this?
>> 
>> Good idea to fix this like this?
>> 
>> If so, any other comments, before I suggest this at dejagnu project?
>> 
>> Thanks,
>> - Tom
>> 
>
> Add dg-dc support
>
> ---
>  lib/dg.exp | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/lib/dg.exp b/lib/dg.exp
> index 7a894cb..67f46ab 100644
> --- a/lib/dg.exp
> +++ b/lib/dg.exp
> @@ -181,15 +181,64 @@ proc dg-format-linenum { linenum } {
>  # we return:
>  #
>  # { dg-prms-id 1 1234 } { dg-build 2 fatal "some comment" }
> +#
> +# Directive dg-dc (short for dg-directive-continue) can be used for multi-line
> +# directives.  This:
> +#
> +# /* { dg-x a b c } */
> +#
> +# is equivalent to:
> +#
> +# /* { dg-x } */
> +# /* { dg-dc a b } */
> +# /* { dg-dc c } */
> +#
> +# and to:
> +#
> +# /* { dg-x a } */
> +# /* { dg-dc b c} */
>  
>  proc dg-get-options { prog } {
>      set result ""
> -
> -    set tmp [grep $prog "{\[ \t\]\+dg-\[-a-z\]\+\[ \t\]\+.*\[ \t\]\+}" line]
> +    set cmd_prev ""
> +
> +    set grep_pattern [join {
> +	"{"
> +	"\[ \t\]\+"
> +	"dg-\[-a-z\]\+"
> +	"\[ \t\]\+"
> +	"(.*\[ \t\]\+)?"
> +	"}"
> +    } ""]
> +    set tmp [grep $prog $grep_pattern line]
>      if {![string match "" $tmp]} {
> +	set pattern [join {
> +	    "(\[0-9\]+)"
> +	    "\[ \t\]+"
> +	    "{"
> +	    "\[ \t\]+"
> +	    "(dg-\[-a-z\]+)"
> +	    "\[ \t\]+"
> +	    "((.*)\[ \t\]+)?"
> +	    "}"
> +	    "\[^\}\]*"
> +	    "(\n|$)"
> +	} ""]
>  	foreach i $tmp {
> -	    regexp "(\[0-9\]+)\[ \t\]+{\[ \t\]+(dg-\[-a-z\]+)\[ \t\]+(.*)\[ \t\]+}\[^\}\]*(\n|$)" $i i line cmd args
> -	    append result " { $cmd $line $args }"
> +	    regexp $pattern $i dummy line cmd ws_args args
> +	    if { "$cmd" == "dg-dc" } {
> +		set args_prev "$args_prev $args"
> +	    } else {
> +		if { "$cmd_prev" != "" } {
> +		    append result " { $cmd_prev $line_prev $args_prev }"
> +		}
> +		set cmd_prev $cmd
> +		set line_prev $line
> +		set args_prev "$args"
> +	    }
> +	}
> +	if { "$cmd_prev" != "" } {
> +	    append result " { $cmd_prev $line_prev $args_prev }"
>  	}
>      }
>      return $result

I realise there's probably more that can go wrong with it, but how
about instead treating unbalanced { ... } as a sign that the directive
continues to the next line?  This would allow:

/* { dg-additional-options
      "-DSTACK_SIZE=[dg-effective-target-value stack_size]"
      { target { stack_size } } } */

To support per-line comments we probably need to drop the characters
before the start column, as in:

! { dg-additional-options
!     "-DSTACK_SIZE=[dg-effective-target-value stack_size]"
!     { target { stack_size } } }

Only lightly tested.

Thanks,
Richard


--- utils.exp.1	2017-06-12 08:07:34.004143966 +0100
+++ utils.exp	2017-06-12 08:54:13.609875614 +0100
@@ -161,6 +161,8 @@
 #            second is the pattern,
 #            third are any options.
 #     Options: line  - puts line numbers of match in list
+#              tcl   - match trailing characters until a complete Tcl
+#                      script is read
 #
 proc grep { args } {
 
@@ -178,20 +180,51 @@
     } else {
 	set options ""
     }
+    set line_p [expr { [lsearch $options line] >= 0 }]
+    set tcl_p [expr { [lsearch $options tcl] >= 0 }]
 
     set i 0
     set fd [open $file r]
-    while { [gets $fd cur_line]>=0 } {
+    while { $i >= 0 && [gets $fd cur_line] >= 0 } {
 	incr i
-	if [regexp -- "$pattern" $cur_line match] {
-	    if ![string match "" $options] {
-		foreach opt $options {
-		    case $opt in {
-			"line" {
-			    lappend grep_out [concat $i $match]
+	if [regexp -indices -- "$pattern" $cur_line indices] {
+	    set line $i
+	    set start [lindex $indices 0]
+	    set end [lindex $indices 1]
+	    set match [string range $cur_line $start $end]
+	    if { $tcl_p } {
+		incr end
+		while { ![info complete $match] } {
+		    set next [string first "\}" $cur_line $end]
+		    if { $next >= 0 } {
+			append match [string range $cur_line $end $next]
+			set end [expr $next + 1]
+		    } else {
+			append match [string trimright \
+					  [string range $cur_line $end end]]
+			# Expect the same number of characters of indentation.
+			# If we don't get a line that we expect, still treat
+			# this as a match (by breaking), so that it gets
+			# reported where appropriate.
+			if { [gets $fd cur_line] < 0 } {
+			    break
 			}
+			incr i
+			if { [string length $cur_line] < $start } {
+			    break
+			}
+			set cur_line [string trimleft \
+					  [string range $cur_line $start end]]
+			if { [string is space $cur_line] } {
+			    break
+			}
+			append match " "
+			set end 0
 		    }
 		}
+	    }
+	    if { $line_p } {
+		lappend grep_out [concat $line $match]
 	    } else {
 		lappend grep_out $match
 	    }
--- dg.exp.1	2017-06-12 08:06:55.292935737 +0100
+++ dg.exp	2017-06-12 08:32:14.309351359 +0100
@@ -186,7 +186,7 @@
 proc dg-get-options { prog } {
     set result ""
 
-    set tmp [grep $prog "{\[ \t\]\+dg-\[-a-z\]\+\[ \t\]\+.*\[ \t\]\+}" line]
+    set tmp [grep $prog "\{\[ \t\]\+dg-\[-a-z\]\+\[ \t\]" line tcl]
     if ![string match "" $tmp] {
 	foreach i $tmp {
 	    #send_user "Found: $i\n"

Re: [RFC] Dejagnu patch to handle multi-line directives

[Pedro Alves]

On 06/12/2017 08:59 AM, Richard Sandiford wrote:
> I realise there's probably more that can go wrong with it, but how
> about instead treating unbalanced { ... } as a sign that the directive
> continues to the next line?  This would allow:
> 
> /* { dg-additional-options
>       "-DSTACK_SIZE=[dg-effective-target-value stack_size]"
>       { target { stack_size } } } */

In a TCL .exp file you'd split the lines with a '\' continuation
character.  Wouldn't that be more natural?  Like:

 /* { dg-additional-options                                   \
       "-DSTACK_SIZE=[dg-effective-target-value stack_size]"  \
       { target { stack_size } } } */

Might be less magical and simpler to implement too.

Thanks,
Pedro Alves

Re: [RFC] Dejagnu patch to handle multi-line directives

[Richard Sandiford]

Pedro Alves <palves@redhat.com> writes:
> On 06/12/2017 08:59 AM, Richard Sandiford wrote:
>> I realise there's probably more that can go wrong with it, but how
>> about instead treating unbalanced { ... } as a sign that the directive
>> continues to the next line?  This would allow:
>> 
>> /* { dg-additional-options
>>       "-DSTACK_SIZE=[dg-effective-target-value stack_size]"
>>       { target { stack_size } } } */
>
> In a TCL .exp file you'd split the lines with a '\' continuation
> character.  Wouldn't that be more natural?  Like:
>
>  /* { dg-additional-options                                   \
>        "-DSTACK_SIZE=[dg-effective-target-value stack_size]"  \
>        { target { stack_size } } } */

It'd be more normal to omit \ in a braced list though, so I think
the version without is more natural Tcl style.

> Might be less magical and simpler to implement too.

The reason I avoided \ was because it's a "native" continuation marker
for C and C++, but not for Fortran, Ada and others.  So using \ would
change the way the comment is treated by some front ends and not others.

E.g. things like:

//  a \
//  b

trigger:

  warning: multi-line comment [-Wcomment]

(Maybe moot anyway given Mike's response.)

Thanks,
Richard

Re: [RFC] Dejagnu patch to handle multi-line directives

[Mike Stump]

On Jun 10, 2017, at 12:57 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> 
> one thing that has bothered me on a regular basis is the inability to spread long dejagnu directives over multiple lines.

I'm not terribly in favor of this.  I'd like to retain the ability to grep and sed single line things.  It makes exploring and finding things easier.  Also, if we bulk convert to a new framework system for running test cases, the conversion is easier.

Re: [RFC] Dejagnu patch to handle multi-line directives

[Rainer Orth]

Mike Stump <mikestump@comcast.net> writes:

> On Jun 10, 2017, at 12:57 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> 
>> one thing that has bothered me on a regular basis is the inability to
>> spread long dejagnu directives over multiple lines.
>
> I'm not terribly in favor of this.  I'd like to retain the ability to grep
> and sed single line things.  It makes exploring and finding things easier.
> Also, if we bulk convert to a new framework system for running test cases,
> the conversion is easier.

I have to agree.  Besides, extremely long lines with DejaGnu directives
often are a sign that something is amiss, e.g. long lists of targets
instead of an effective-target keyword describing what's common between
them or Tom's example which is way more readably handled via
dg-add-options as he's discovered in the meantime ;-)

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University