[RFC 0/5] Add new "critical" command to test suite

[RFC 0/5] Add new "critical" command to test suite

[Tom Tromey]

This introduces a new "critical" command to the test suite.  The idea
here is to replace typical uses of "if ... { return }" with a simpler
notation.  Centralizing this also lets us decide -- like "require" --
whether to use untested or unsupported.

This command has a bit of a conceptual overlap with "require".  I tend
to think of "require" as expressing static properties of the test,
whereas "critical" is used to early abort if certain runtime
operations fail; but this isn't enforced.

Anyway, 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.

In this series, I converted gdb.rust to the new scheme, just to show
how it works.  I'm sending it as an RFC for discussion.  If it seems
ok, I thought I'd write some scripts to mass convert the rest of the
test suite.

Tom

[RFC 1/5] Bug fix in prepare_for_testing

[Tom Tromey]

prepare_for_testing should return the result of clean_restart, but it
currently does not.  This patch fixes the oversight.
---
 gdb/testsuite/lib/gdb.exp | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 9ea0334759d..a1808c699e9 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -7677,8 +7677,7 @@ proc prepare_for_testing_full {testname args} {
 	}
 	set executable [lindex $spec 0]
     }
-    clean_restart $executable
-    return 0
+    return [clean_restart $executable]
 }
 
 # Prepares for testing, by calling build_executable, and then clean_restart.
@@ -7688,9 +7687,7 @@ proc prepare_for_testing { testname executable {sources ""} {options {debug}}} {
     if {[build_executable $testname $executable $sources $options] == -1} {
         return -1
     }
-    clean_restart $executable
-
-    return 0
+    return [clean_restart $executable]
 }
 
 # Retrieve the value of EXP in the inferior, represented in format
-- 
2.39.2

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

[Tom Tromey]

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"
+		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

[RFC 3/5] Use "critical ensure_gdb_index" in gdb.rust

[Tom Tromey]

This converts gdb.rust to use "critical ensure_gdb_index" rather than
"if".
---
 gdb/testsuite/gdb.base/with-mf.exp                    |  5 +----
 gdb/testsuite/gdb.dwarf2/gdb-add-index-symlink.exp    | 10 ++--------
 gdb/testsuite/gdb.dwarf2/gdb-add-index.exp            |  5 +----
 gdb/testsuite/gdb.dwarf2/imported-unit-runto-main.exp |  5 +----
 gdb/testsuite/gdb.rust/dwindex.exp                    |  5 +----
 5 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/gdb/testsuite/gdb.base/with-mf.exp b/gdb/testsuite/gdb.base/with-mf.exp
index f26d48e1026..5ae4234056f 100644
--- a/gdb/testsuite/gdb.base/with-mf.exp
+++ b/gdb/testsuite/gdb.base/with-mf.exp
@@ -24,10 +24,7 @@ if {[prepare_for_testing "failed to prepare" $testfile "$srcfile $srcfile2" \
     return -1
 }
 
-if { [ensure_gdb_index $binfile] == -1 } {
-    untested "error adding gdb index"
-    return -1
-}
+critical ensure_gdb_index $binfile
 
 clean_restart $binfile
 
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-add-index-symlink.exp b/gdb/testsuite/gdb.dwarf2/gdb-add-index-symlink.exp
index d6f96ab9f5b..0654249e302 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-add-index-symlink.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-add-index-symlink.exp
@@ -30,10 +30,7 @@ with_test_prefix non-symlink {
 	return -1
     }
 
-    if { [ensure_gdb_index $binfile] == -1 } {
-	untested "error adding gdb index"
-	return -1
-    }
+    critical ensure_gdb_index $binfile
 }
 
 # Regenerate exec without index.
@@ -48,10 +45,7 @@ if { ![file exists $symlink] } {
     file link -symbolic $symlink $binfile
 }
 
-if { [ensure_gdb_index $symlink] == -1 } {
-    fail "Unable to call gdb-add-index with a symlink to a symfile"
-    return -1
-}
+critical ensure_gdb_index $symlink
 
 # Ok, we have a copy of $binfile with an index.
 # Restart gdb and verify the index was used.
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-add-index.exp b/gdb/testsuite/gdb.dwarf2/gdb-add-index.exp
index 4ecf4408c38..849680314da 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-add-index.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-add-index.exp
@@ -25,10 +25,7 @@ if { [prepare_for_testing "failed to prepare" "${testfile}" \
     return -1
 }
 
-if { [ensure_gdb_index $binfile] == -1 } {
-    untested "error adding gdb index"
-    return -1
-}
+critical ensure_gdb_index $binfile
 
 # Ok, we have a copy of $binfile with an index.
 # Restart gdb and verify the index was used.
diff --git a/gdb/testsuite/gdb.dwarf2/imported-unit-runto-main.exp b/gdb/testsuite/gdb.dwarf2/imported-unit-runto-main.exp
index dea3f3fe461..1227a66f643 100644
--- a/gdb/testsuite/gdb.dwarf2/imported-unit-runto-main.exp
+++ b/gdb/testsuite/gdb.dwarf2/imported-unit-runto-main.exp
@@ -72,10 +72,7 @@ if { [prepare_for_testing "failed to prepare" ${testfile} \
     return -1
 }
 
-if { [ensure_gdb_index $binfile] == -1 } {
-    untested "error adding gdb index"
-    return -1
-}
+critical ensure_gdb_index $binfile
 
 clean_restart ${binfile}
 
diff --git a/gdb/testsuite/gdb.rust/dwindex.exp b/gdb/testsuite/gdb.rust/dwindex.exp
index 1a82c2e92fe..0b7e3911ab1 100644
--- a/gdb/testsuite/gdb.rust/dwindex.exp
+++ b/gdb/testsuite/gdb.rust/dwindex.exp
@@ -25,10 +25,7 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
     return -1
 }
 
-if {[ensure_gdb_index $binfile -dwarf-5] == -1} {
-    untested "error adding gdb index"
-    return -1
-}
+critical ensure_gdb_index $binfile -dwarf-5
 
 gdb_exit
 gdb_start
-- 
2.39.2

[RFC 4/5] Use "critical prepare_for_testing" in gdb.rust

[Tom Tromey]

This converts gdb.rust to use "critical prepare_for_testing" rather
than "if".
---
 gdb/testsuite/gdb.rust/completion.exp  | 5 ++---
 gdb/testsuite/gdb.rust/dwindex.exp     | 5 ++---
 gdb/testsuite/gdb.rust/finish.exp      | 5 ++---
 gdb/testsuite/gdb.rust/fnfield.exp     | 5 ++---
 gdb/testsuite/gdb.rust/generics.exp    | 5 ++---
 gdb/testsuite/gdb.rust/main-crash.exp  | 6 ++----
 gdb/testsuite/gdb.rust/methods.exp     | 5 ++---
 gdb/testsuite/gdb.rust/modules.exp     | 5 ++---
 gdb/testsuite/gdb.rust/onetwoeight.exp | 5 ++---
 gdb/testsuite/gdb.rust/pp.exp          | 5 ++---
 gdb/testsuite/gdb.rust/rawids.exp      | 5 ++---
 gdb/testsuite/gdb.rust/rust-start.exp  | 5 ++---
 gdb/testsuite/gdb.rust/rust-style.exp  | 6 ++----
 gdb/testsuite/gdb.rust/simple.exp      | 5 ++---
 gdb/testsuite/gdb.rust/traits.exp      | 5 ++---
 gdb/testsuite/gdb.rust/unicode.exp     | 5 ++---
 gdb/testsuite/gdb.rust/union.exp       | 5 ++---
 gdb/testsuite/gdb.rust/unsized.exp     | 5 ++---
 gdb/testsuite/gdb.rust/watch.exp       | 5 ++---
 19 files changed, 38 insertions(+), 59 deletions(-)

diff --git a/gdb/testsuite/gdb.rust/completion.exp b/gdb/testsuite/gdb.rust/completion.exp
index 76c45fda7ba..91f8b230f4a 100644
--- a/gdb/testsuite/gdb.rust/completion.exp
+++ b/gdb/testsuite/gdb.rust/completion.exp
@@ -21,9 +21,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/dwindex.exp b/gdb/testsuite/gdb.rust/dwindex.exp
index 0b7e3911ab1..e3fe49fbce1 100644
--- a/gdb/testsuite/gdb.rust/dwindex.exp
+++ b/gdb/testsuite/gdb.rust/dwindex.exp
@@ -21,9 +21,8 @@ require {can_compile rust}
 
 standard_testfile .rs
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 critical ensure_gdb_index $binfile -dwarf-5
 
diff --git a/gdb/testsuite/gdb.rust/finish.exp b/gdb/testsuite/gdb.rust/finish.exp
index 373f8d72f5d..630b093b440 100644
--- a/gdb/testsuite/gdb.rust/finish.exp
+++ b/gdb/testsuite/gdb.rust/finish.exp
@@ -20,9 +20,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "BREAK"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/fnfield.exp b/gdb/testsuite/gdb.rust/fnfield.exp
index 48c6179d7d6..93b00f11b8a 100644
--- a/gdb/testsuite/gdb.rust/fnfield.exp
+++ b/gdb/testsuite/gdb.rust/fnfield.exp
@@ -20,9 +20,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/generics.exp b/gdb/testsuite/gdb.rust/generics.exp
index 7afed993a2d..9eba8f5786e 100644
--- a/gdb/testsuite/gdb.rust/generics.exp
+++ b/gdb/testsuite/gdb.rust/generics.exp
@@ -20,9 +20,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/main-crash.exp b/gdb/testsuite/gdb.rust/main-crash.exp
index caede88a1fa..450af9ed18c 100644
--- a/gdb/testsuite/gdb.rust/main-crash.exp
+++ b/gdb/testsuite/gdb.rust/main-crash.exp
@@ -20,10 +20,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile main.rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
-	 {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "BREAK"]
 # The bug was that this would crash.
diff --git a/gdb/testsuite/gdb.rust/methods.exp b/gdb/testsuite/gdb.rust/methods.exp
index dd95429de7d..67792986fad 100644
--- a/gdb/testsuite/gdb.rust/methods.exp
+++ b/gdb/testsuite/gdb.rust/methods.exp
@@ -20,9 +20,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "set breakpoint 1 here"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/modules.exp b/gdb/testsuite/gdb.rust/modules.exp
index 076ef5c2dd4..ddb9a05c58f 100644
--- a/gdb/testsuite/gdb.rust/modules.exp
+++ b/gdb/testsuite/gdb.rust/modules.exp
@@ -20,9 +20,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/onetwoeight.exp b/gdb/testsuite/gdb.rust/onetwoeight.exp
index ef56bcafda2..ad536d27900 100644
--- a/gdb/testsuite/gdb.rust/onetwoeight.exp
+++ b/gdb/testsuite/gdb.rust/onetwoeight.exp
@@ -26,9 +26,8 @@ if {[lindex $v 0] == 1 && [lindex $v 1] < 43} {
 }
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "BREAK"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/pp.exp b/gdb/testsuite/gdb.rust/pp.exp
index ef66717607b..0f8fee20fb9 100644
--- a/gdb/testsuite/gdb.rust/pp.exp
+++ b/gdb/testsuite/gdb.rust/pp.exp
@@ -21,9 +21,8 @@ require allow_rust_tests allow_python_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set remote_python_file [gdb_remote_download host \
 			    ${srcdir}/${subdir}/${testfile}.py]
diff --git a/gdb/testsuite/gdb.rust/rawids.exp b/gdb/testsuite/gdb.rust/rawids.exp
index 55e1b7ffbdd..0c9ecaabe32 100644
--- a/gdb/testsuite/gdb.rust/rawids.exp
+++ b/gdb/testsuite/gdb.rust/rawids.exp
@@ -21,9 +21,8 @@ require {can_compile rust}
 require {rust_at_least 1.30}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/rust-start.exp b/gdb/testsuite/gdb.rust/rust-start.exp
index ca2608465dd..5ce7b9bb053 100644
--- a/gdb/testsuite/gdb.rust/rust-start.exp
+++ b/gdb/testsuite/gdb.rust/rust-start.exp
@@ -24,9 +24,8 @@ require {can_compile rust}
 require !use_gdb_stub
 
 standard_testfile simple.rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 # Verify that "start" lands inside the right procedure.
 if {[gdb_start_cmd] < 0} {
diff --git a/gdb/testsuite/gdb.rust/rust-style.exp b/gdb/testsuite/gdb.rust/rust-style.exp
index 0555ea4b8a4..71a484ae933 100644
--- a/gdb/testsuite/gdb.rust/rust-style.exp
+++ b/gdb/testsuite/gdb.rust/rust-style.exp
@@ -24,10 +24,8 @@ save_vars { env(TERM) } {
     setenv TERM ansi
 
     standard_testfile .rs
-    if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
-	     {debug rust}]} {
-	return -1
-    }
+    critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+	{debug rust}
 
     set line [gdb_get_line_number "breakpoint"]
     if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp
index 08ebed3f103..dd73f50af81 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -20,9 +20,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/traits.exp b/gdb/testsuite/gdb.rust/traits.exp
index e52749733d7..a614065a129 100644
--- a/gdb/testsuite/gdb.rust/traits.exp
+++ b/gdb/testsuite/gdb.rust/traits.exp
@@ -20,9 +20,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set readelf_program [gdb_find_readelf]
 set result [catch "exec $readelf_program --debug-dump=info $binfile" output]
diff --git a/gdb/testsuite/gdb.rust/unicode.exp b/gdb/testsuite/gdb.rust/unicode.exp
index 97b37af316a..8063d7ec1b9 100644
--- a/gdb/testsuite/gdb.rust/unicode.exp
+++ b/gdb/testsuite/gdb.rust/unicode.exp
@@ -26,9 +26,8 @@ require {rust_at_least 1.53}
 setenv LC_ALL C.UTF-8
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/union.exp b/gdb/testsuite/gdb.rust/union.exp
index 6074717d276..d4a12156f3e 100644
--- a/gdb/testsuite/gdb.rust/union.exp
+++ b/gdb/testsuite/gdb.rust/union.exp
@@ -20,9 +20,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/unsized.exp b/gdb/testsuite/gdb.rust/unsized.exp
index f6d39dd6248..61c671dce6a 100644
--- a/gdb/testsuite/gdb.rust/unsized.exp
+++ b/gdb/testsuite/gdb.rust/unsized.exp
@@ -20,9 +20,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
 if {![runto ${srcfile}:$line]} {
diff --git a/gdb/testsuite/gdb.rust/watch.exp b/gdb/testsuite/gdb.rust/watch.exp
index ae359420aad..f9bf6ad2269 100644
--- a/gdb/testsuite/gdb.rust/watch.exp
+++ b/gdb/testsuite/gdb.rust/watch.exp
@@ -20,9 +20,8 @@ require allow_rust_tests
 require {can_compile rust}
 
 standard_testfile .rs
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
-    return -1
-}
+critical prepare_for_testing "failed to prepare" $testfile $srcfile \
+    {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
 if {![runto ${srcfile}:$line]} {
-- 
2.39.2

[RFC 5/5] Use "critical runto" in gdb.rust

[Tom Tromey]

This converts gdb.rust to use "critical runto" rather than "if".
---
 gdb/testsuite/gdb.rust/completion.exp  | 5 +----
 gdb/testsuite/gdb.rust/finish.exp      | 5 +----
 gdb/testsuite/gdb.rust/fnfield.exp     | 5 +----
 gdb/testsuite/gdb.rust/generics.exp    | 5 +----
 gdb/testsuite/gdb.rust/main-crash.exp  | 5 +----
 gdb/testsuite/gdb.rust/methods.exp     | 5 +----
 gdb/testsuite/gdb.rust/modules.exp     | 5 +----
 gdb/testsuite/gdb.rust/onetwoeight.exp | 5 +----
 gdb/testsuite/gdb.rust/pp.exp          | 5 +----
 gdb/testsuite/gdb.rust/rawids.exp      | 5 +----
 gdb/testsuite/gdb.rust/rust-style.exp  | 5 +----
 gdb/testsuite/gdb.rust/simple.exp      | 5 +----
 gdb/testsuite/gdb.rust/traits.exp      | 5 +----
 gdb/testsuite/gdb.rust/unicode.exp     | 5 +----
 gdb/testsuite/gdb.rust/union.exp       | 5 +----
 gdb/testsuite/gdb.rust/unsized.exp     | 5 +----
 gdb/testsuite/gdb.rust/watch.exp       | 5 +----
 17 files changed, 17 insertions(+), 68 deletions(-)

diff --git a/gdb/testsuite/gdb.rust/completion.exp b/gdb/testsuite/gdb.rust/completion.exp
index 91f8b230f4a..e49720bb1ac 100644
--- a/gdb/testsuite/gdb.rust/completion.exp
+++ b/gdb/testsuite/gdb.rust/completion.exp
@@ -25,9 +25,6 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "complete break pars" ".*"
diff --git a/gdb/testsuite/gdb.rust/finish.exp b/gdb/testsuite/gdb.rust/finish.exp
index 630b093b440..8dc114cefae 100644
--- a/gdb/testsuite/gdb.rust/finish.exp
+++ b/gdb/testsuite/gdb.rust/finish.exp
@@ -24,10 +24,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "BREAK"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 # This 'finish' used to crash.  See PR rust/30090.  Also, this does
 # not currently print the correct value, because rustc does not use
diff --git a/gdb/testsuite/gdb.rust/fnfield.exp b/gdb/testsuite/gdb.rust/fnfield.exp
index 93b00f11b8a..bd8aab6dd40 100644
--- a/gdb/testsuite/gdb.rust/fnfield.exp
+++ b/gdb/testsuite/gdb.rust/fnfield.exp
@@ -24,10 +24,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "print foo.f()" " = 6" "call impl function"
 gdb_test "print (foo.f)()" " = 5" "call function field"
diff --git a/gdb/testsuite/gdb.rust/generics.exp b/gdb/testsuite/gdb.rust/generics.exp
index 9eba8f5786e..358fb31f465 100644
--- a/gdb/testsuite/gdb.rust/generics.exp
+++ b/gdb/testsuite/gdb.rust/generics.exp
@@ -24,10 +24,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "print identity::<u32>(23u32)" " = 23"
 gdb_test "ptype identity::<u32>(23u32)" " = u32"
diff --git a/gdb/testsuite/gdb.rust/main-crash.exp b/gdb/testsuite/gdb.rust/main-crash.exp
index 450af9ed18c..621718c24f8 100644
--- a/gdb/testsuite/gdb.rust/main-crash.exp
+++ b/gdb/testsuite/gdb.rust/main-crash.exp
@@ -25,10 +25,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
 
 set line [gdb_get_line_number "BREAK"]
 # The bug was that this would crash.
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 # Test that gdb is alive.
 gdb_test "print 23" " = 23"
diff --git a/gdb/testsuite/gdb.rust/methods.exp b/gdb/testsuite/gdb.rust/methods.exp
index 67792986fad..7dd0b7807b1 100644
--- a/gdb/testsuite/gdb.rust/methods.exp
+++ b/gdb/testsuite/gdb.rust/methods.exp
@@ -24,10 +24,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "set breakpoint 1 here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "print x" " = methods::HasMethods \\{value: 0\\}"
 
diff --git a/gdb/testsuite/gdb.rust/modules.exp b/gdb/testsuite/gdb.rust/modules.exp
index ddb9a05c58f..550e9ba3101 100644
--- a/gdb/testsuite/gdb.rust/modules.exp
+++ b/gdb/testsuite/gdb.rust/modules.exp
@@ -24,10 +24,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 # Currently a closure type is not described by rustc.
 # https://github.com/rust-lang/rust/issues/33121
diff --git a/gdb/testsuite/gdb.rust/onetwoeight.exp b/gdb/testsuite/gdb.rust/onetwoeight.exp
index ad536d27900..2fc4b499d46 100644
--- a/gdb/testsuite/gdb.rust/onetwoeight.exp
+++ b/gdb/testsuite/gdb.rust/onetwoeight.exp
@@ -30,10 +30,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "BREAK"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "print x" " = 340282366920938463463374607431768211455"
 gdb_test "print y" " = 170141183460469231731687303715884105727"
diff --git a/gdb/testsuite/gdb.rust/pp.exp b/gdb/testsuite/gdb.rust/pp.exp
index 0f8fee20fb9..2fdc5b5f14b 100644
--- a/gdb/testsuite/gdb.rust/pp.exp
+++ b/gdb/testsuite/gdb.rust/pp.exp
@@ -29,10 +29,7 @@ set remote_python_file [gdb_remote_download host \
 gdb_test_no_output "source ${remote_python_file}" "load python file"
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "print outer" " = pp::Outer \\(x\\(5\\)\\)"
 gdb_test "print outer.0" " = x\\(5\\)"
diff --git a/gdb/testsuite/gdb.rust/rawids.exp b/gdb/testsuite/gdb.rust/rawids.exp
index 0c9ecaabe32..09b71079ce6 100644
--- a/gdb/testsuite/gdb.rust/rawids.exp
+++ b/gdb/testsuite/gdb.rust/rawids.exp
@@ -25,10 +25,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "print r#if" " = 23"
 gdb_test "print r#thread" " = 27"
diff --git a/gdb/testsuite/gdb.rust/rust-style.exp b/gdb/testsuite/gdb.rust/rust-style.exp
index 71a484ae933..cce5a78cd5d 100644
--- a/gdb/testsuite/gdb.rust/rust-style.exp
+++ b/gdb/testsuite/gdb.rust/rust-style.exp
@@ -28,10 +28,7 @@ save_vars { env(TERM) } {
 	{debug rust}
 
     set line [gdb_get_line_number "breakpoint"]
-    if {![runto ${srcfile}:$line]} {
-	untested "could not run to breakpoint"
-	return -1
-    }
+    critical runto ${srcfile}:$line
 
     set vfield [style value variable]
     set v2field [style value2 variable]
diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp
index dd73f50af81..5b8a08814a8 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -24,10 +24,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "print a" " = \\(\\)"
 gdb_test "ptype a" " = \\(\\)"
diff --git a/gdb/testsuite/gdb.rust/traits.exp b/gdb/testsuite/gdb.rust/traits.exp
index a614065a129..fc06260bad9 100644
--- a/gdb/testsuite/gdb.rust/traits.exp
+++ b/gdb/testsuite/gdb.rust/traits.exp
@@ -36,10 +36,7 @@ if {![regexp DW_AT_containing_type $output]} {
 }
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "print *td" " = 23.5"
 gdb_test "print *tu" " = 23"
diff --git a/gdb/testsuite/gdb.rust/unicode.exp b/gdb/testsuite/gdb.rust/unicode.exp
index 8063d7ec1b9..9e5af414107 100644
--- a/gdb/testsuite/gdb.rust/unicode.exp
+++ b/gdb/testsuite/gdb.rust/unicode.exp
@@ -30,10 +30,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "print 𝕯" " = 98" "print D"
 gdb_test "print \"𝕯\"" " = \"𝕯\"" "print D in string"
diff --git a/gdb/testsuite/gdb.rust/union.exp b/gdb/testsuite/gdb.rust/union.exp
index d4a12156f3e..1e65c664f59 100644
--- a/gdb/testsuite/gdb.rust/union.exp
+++ b/gdb/testsuite/gdb.rust/union.exp
@@ -24,10 +24,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "print u" " = union::Union {f1: -1, f2: 255}"
 
diff --git a/gdb/testsuite/gdb.rust/unsized.exp b/gdb/testsuite/gdb.rust/unsized.exp
index 61c671dce6a..275491e432a 100644
--- a/gdb/testsuite/gdb.rust/unsized.exp
+++ b/gdb/testsuite/gdb.rust/unsized.exp
@@ -24,10 +24,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 gdb_test "ptype us" " = .*V<\\\[u8\\\]>.*"
 
diff --git a/gdb/testsuite/gdb.rust/watch.exp b/gdb/testsuite/gdb.rust/watch.exp
index f9bf6ad2269..c4170b2a248 100644
--- a/gdb/testsuite/gdb.rust/watch.exp
+++ b/gdb/testsuite/gdb.rust/watch.exp
@@ -24,10 +24,7 @@ critical prepare_for_testing "failed to prepare" $testfile $srcfile \
     {debug rust}
 
 set line [gdb_get_line_number "set breakpoint here"]
-if {![runto ${srcfile}:$line]} {
-    untested "could not run to breakpoint"
-    return -1
-}
+critical runto ${srcfile}:$line
 
 # Just setting a watchpoint was enough to trigger the bug.
 gdb_test "watch -location y" ".*\[wW\]atchpoint .* -location .*"
-- 
2.39.2

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

[Kevin Buettner]

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
>