Re: [RFC 2/5] Introduce the "critical" testsuite command

[Kevin Buettner <kevinb@redhat.com>]

Hi Tom,

Overall, I like this idea, but I think I found an easily fixable problem
in the implementation of the "critical" command - see below.

On Sat, 15 Apr 2023 09:13:33 -0600
Tom Tromey <tom@tromey.com> wrote:

> This patch introduces a new "critical" command for the test suite.
> Unlike require, "critical" can handle multiple arguments and a variety
> of return value conventions.  It is implemented as a Tcl namespace
> ensemble for this reason.
> 
> This version of the patch provides a few such subcommands.  Currently
> these are all implemented via a helper proc, but there's no
> requirement that this always be true.
> ---
>  gdb/testsuite/lib/gdb.exp | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index a1808c699e9..0ae40bceba2 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -9287,6 +9287,34 @@ proc require { args } {
>      }
>  }
>  
> +#
> +# The "critical" command is used to invoke some code that must
> +# succeed.  If the code in question fails, "unsupported" is logged and
> +# the command's caller will return.
> +#
> +# This is implemented as an ensemble namespace so that individual
> +# sub-commands can handle varying return values correctly.
> +#
> +namespace eval critical {
> +    proc _make_wrapper {name bad_result} {
> +	proc $name {args} [format {
> +	    set result [uplevel [list %s {*}$args]]
> +	    if {$result == %s} {
> +		unsupported "error adding gdb index"

This message makes sense for ensure_gdb_index, but I don't think
it makes sense for the others.  Should a suitable message to print
here be passed to _make_wrapper?

> +		return -code return 0
> +	    }
> +	} $name $bad_result]
> +	namespace export $name
> +    }
> +
> +    _make_wrapper ensure_gdb_index -1
> +    _make_wrapper prepare_for_testing_full -1
> +    _make_wrapper prepare_for_testing -1
> +    _make_wrapper runto 0
> +
> +    namespace ensemble create
> +}
> +
>  # Wait up to ::TIMEOUT seconds for file PATH to exist on the target system.
>  # Return 1 if it does exist, 0 otherwise.
>  
> -- 
> 2.39.2
>