Re: [PATCH] MIPS: Enable NewABI tests for SDE targets

[Richard Sandiford <rdsandiford at googlemail dot com>]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Thu, 2 Aug 2012, Richard Sandiford wrote:
>> > Index: binutils-fsf-trunk-quilt/ld/testsuite/lib/ld-lib.exp
>> > ===================================================================
>> > --- binutils-fsf-trunk-quilt.orig/ld/testsuite/lib/ld-lib.exp	2012-07-24 15:29:41.000000000 +0100
>> > +++ binutils-fsf-trunk-quilt/ld/testsuite/lib/ld-lib.exp	2012-07-26 03:08:32.951798913 +0100
>> > @@ -431,7 +431,7 @@ proc ld_simple_link_defsyms {} {
>> >      return $flags
>> >  }
>> > 
>> > -# run_dump_test FILE
>> > +# run_dump_test FILE (optional:) EXTRA_OPTIONS
>> >  # Copied from gas testsuite, tweaked and further extended.
>> >  #
>> >  # Assemble a .s file, then run some utility on it and check the output.
>> > @@ -456,6 +456,12 @@ proc ld_simple_link_defsyms {} {
>> >  # list ends with the first line that doesn't match the above syntax
>> >  # (hmm, not great for error detection).
>> >  #
>> > +# The optional EXTRA_OPTIONS argument to `run_dump_test' is a list of
>> > +# two-element lists.  The first element of each is an option name, and
>> > +# the second additional arguments to be added on to the end of the
>> > +# option list as given in FILE.d.  (If omitted, no additional options
>> > +# are added.)
>> > +#
>> >  # The interesting options are:
>> >  #
>> >  #   name: TEST-NAME
>> > @@ -503,6 +509,11 @@ proc ld_simple_link_defsyms {} {
>> >  #       More than one "source" directive can be given, which is useful
>> >  #       when testing linking.
>> >  #
>> > +#   dump: DUMP
>> > +#	Match against DUMP.d.  If omitted, this defaults to FILE.d.  This
>> > +#	is useful if several .d files differ by options only.  Options are
>> > +#	always read from FILE.d.
>> > +#
>> >  #   xfail: TARGET
>> >  #       The test is expected to fail on TARGET.  This may occur more than
>> >  #       once.
>> > @@ -534,7 +545,7 @@ proc ld_simple_link_defsyms {} {
>> >  # regexps in FILE.d.  `regexp_diff' is defined in binutils-common.exp;
>> >  # see further comments there.
>> >  #
>> > -proc run_dump_test { name } {
>> > +proc run_dump_test { name {extra_options {}} } {
>> >      global subdir srcdir
>> >      global OBJDUMP NM AS OBJCOPY READELF LD
>> >      global OBJDUMPFLAGS NMFLAGS ASFLAGS OBJCOPYFLAGS READELFFLAGS LDFLAGS
>> 
>> I think extra_options ought to be a one-level list, i.e.
>> {key1 value key2 value2 ...}.  That makes the calls simpler
>> (because there's one fewer [list ...]) and:
>> 
>> > +    foreach i $extra_options {
>> > +	set opt_name [lindex $i 0]
>> > +	set opt_val [lindex $i 1]
>> > +	if ![info exists opts($opt_name)] {
>> > +	    perror "unknown option $opt_name given in extra_opts"
>> > +	    unresolved $subdir/$name
>> > +	    return
>> > +	}
>> > +	# Add extra option to end of existing option, adding space
>> > +	# if necessary.
>> > +	if { ![regexp "warning|error" $opt_name]
>> > +	     && [string length $opts($opt_name)] } {
>> > +	    append opts($opt_name) " "
>> > +	}
>> > +	append opts($opt_name) $opt_val
>> > +    }
>> 
>> reduces to:
>> 
>>     foreach { opt_name opt_val } $extra_options {
>> 	if ![info exists opts($opt_name)] {
>> 	    perror "unknown option $opt_name given in extra_opts"
>> 	    unresolved $subdir/$name
>> 	    return
>> 	}
>> 	# Add extra option to end of existing option, adding space
>> 	# if necessary.
>> 	if { ![regexp "warning|error" $opt_name]
>> 	     && [string length $opts($opt_name)] } {
>> 	    append opts($opt_name) " "
>> 	}
>> 	append opts($opt_name) $opt_val
>>     }
>> 
>> OK with that change, thanks.  OK for the follow-up too.
>
>  A list of lists seems more in the spirit of TCL to me, while a vector is 
> more C-like

Not sure about that.  "foreach { a b .. } ..." is provided exactly to make
flat lists easy to handle.  [string map ...] takes a flat list rather than
a list of lists.  Flat lists are also what is used by [array set ...]
and [array get ...], so that one easy way of handling unordered key-value
list arguments is:

    array set foo $argument
    if { [info exists $foo(key)] } ...

If you're lucky enough to be able to rely on Tcl 8.5, then dict operates
in just the same way.  Flat lists can even be used as dicts directly,
without any conversion.

I don't know of any native Tcl operation that uses lists of lists instead.

> However please note that implementing your suggestion will make the
> two run_dump_test implementations confusingly different -- do you
> still want me to proceed?

That's a clincher though.  Please go with your original patch.

Richard