Remove some TODOs from gdb.cp tests

  This patch removes many TODOs from the gdb.cp tests.
Going through the patch:

* bs15503.exp - these have been commented out forever and rely on
  libstdc++ debuginfo.  It's better to just remove these.

* classes.exp - the test is wrong, I think, according to the C++ ABI
  that gdb understands; and the test can be fixed and comments removed
  with a simple change to the code.

* ctti.exp - there's no need to bail out any more, as the test works.

* exception.exp - the code relying on the line numbers can't work,
  because gdb never prints that message anyway.
---
 gdb/testsuite/gdb.cp/bs15503.exp   | 32 -------------------
 gdb/testsuite/gdb.cp/classes.cc    |  7 ++--
 gdb/testsuite/gdb.cp/classes.exp   |  7 +---
 gdb/testsuite/gdb.cp/ctti.exp      | 17 ----------
 gdb/testsuite/gdb.cp/exception.exp | 51 +++++++-----------------------
 5 files changed, 15 insertions(+), 99 deletions(-)
  

On 10/07/2023 18:27, Tom Tromey via Gdb-patches wrote:
> This patch removes many TODOs from the gdb.cp tests.
> Going through the patch:
>
> * bs15503.exp - these have been commented out forever and rely on
>    libstdc++ debuginfo.  It's better to just remove these.
Fun fact, they were introduced as commented out already. on commit 
77c26ae3f453b4822124c65a0f4874e9af307dde. If anything, this is even more 
of a reason to remove them
>
> * classes.exp - the test is wrong, I think, according to the C++ ABI
>    that gdb understands; and the test can be fixed and comments removed
>    with a simple change to the code.
>
> * ctti.exp - there's no need to bail out any more, as the test works.
>
> * exception.exp - the code relying on the line numbers can't work,
>    because gdb never prints that message anyway.

If that is dead code, why not remove those and just have a simple gdb_test?

> ---
>   gdb/testsuite/gdb.cp/bs15503.exp   | 32 -------------------
>   gdb/testsuite/gdb.cp/classes.cc    |  7 ++--
>   gdb/testsuite/gdb.cp/classes.exp   |  7 +---
>   gdb/testsuite/gdb.cp/ctti.exp      | 17 ----------
>   gdb/testsuite/gdb.cp/exception.exp | 51 +++++++-----------------------
>   5 files changed, 15 insertions(+), 99 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.cp/bs15503.exp b/gdb/testsuite/gdb.cp/bs15503.exp
> index 29118629d24..a9f043c4f9e 100644
> --- a/gdb/testsuite/gdb.cp/bs15503.exp
> +++ b/gdb/testsuite/gdb.cp/bs15503.exp
> @@ -50,35 +50,3 @@ gdb_test "print s\[0\]"			"\\$\[0-9\]+ =.* 'I'"
>   gdb_test "print s\[s.length()-1\]"	"\\$\[0-9\]+ =.* 'g'"
>   gdb_test "print (const char *) s" \
>       "\\$\[0-9\]+ = $hex \"I am a short stringand now a longer string\""
> -
> -# TODO: tests that do not work with gcc 2.95.3
> -# -- chastain 2004-01-07
> -#
> -# gdb_test "print s.compare(s)"		"\\$\[0-9\]+ = 0"
> -# gdb_test "print s.compare(\"AAA\")"     "\\$\[0-9\]+ = 1"
> -# gdb_test "print s.compare(\"ZZZ\")"     "\\$\[0-9\]+ = -1"
> -
> -# TODO: tests that do not work with gcc 2.95.3 and gcc 3.3.2.
> -# cannot call overloaded non-member operator.  -- chastain 2004-01-07
> -#
> -# gdb_test "print s == s"			"\\$\[0-9\]+ = true"
> -# gdb_test "print s > "AAA"			"\\$\[0-9\]+ = true"
> -# gdb_test "print s < "ZZZ"			"\\$\[0-9\]+ = true"
> -
> -# TODO: GDB doesn't know to convert the string to a const char *, and
> -# instead tries to use the string as a structure initializer.
> -#
> -# gdb_test "print s == \"I am a short stringand now a longer string\"" \
> -#     "\\$\[0-9\]+ = true"
> -
> -gdb_test "print (const char *) s.substr(0,4)"	"\\$\[0-9\]+ = $hex \"I am\""
> -gdb_test "print (const char *) (s=s.substr(0,4))" \
> -    "\\$\[0-9\]+ = $hex \"I am\""
> -
> -# TODO: cannot call overloaded non-member operator again.
> -# -- chastain 2004-01-07
> -#
> -# gdb_test "print (const char *) (s + s)" \
> -#    "\\$\[0-9\]+ = $hex \"I amI am\""
> -# gdb_test "print (const char *) (s + \" \" + s)" \
> -#    "\\$\[0-9\]+ = $hex \"I am I am\""
> diff --git a/gdb/testsuite/gdb.cp/classes.cc b/gdb/testsuite/gdb.cp/classes.cc
> index fdd1a9b4afa..bf5879e403c 100644
> --- a/gdb/testsuite/gdb.cp/classes.cc
> +++ b/gdb/testsuite/gdb.cp/classes.cc
> @@ -646,16 +646,13 @@ Outer Outer::instance;
>   int
>   main()
>   {
> +  int Foo::* pmi = &Foo::y;
> +
>     dummy();
>     inheritance1 ();
>     inheritance3 ();
>     enums1 ();
>   
> -  /* FIXME: pmi gets optimized out.  Need to do some more computation with
> -     it or something.  (No one notices, because the test is xfail'd anyway,
> -     but that probably won't always be true...).  */
> -  int Foo::* pmi = &Foo::y;
> -
>     /* Make sure the AIX linker doesn't remove the variable.  */
>     v_tagless.one = 5;
>   
> diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp
> index 4b1b01df383..ed37daecac7 100644
> --- a/gdb/testsuite/gdb.cp/classes.exp
> +++ b/gdb/testsuite/gdb.cp/classes.exp
> @@ -601,12 +601,7 @@ proc test_pointers_to_class_members {} {
>       gdb_test "ptype Bar::z" "type = int"
>       gdb_test "ptype &Bar::z" "type = int Bar::\\*"
>   
> -    # TODO: this is a bogus test.  It's looking at a variable that
> -    # has not even been declared yet, so it's accessing random junk
> -    # on the stack and comparing that it's NOT equal to a specific
> -    # value.  It's been like this since gdb 4.10 in 1993!
> -    # -- chastain 2004-01-01
> -    gdb_test "print (int)pmi == sizeof(int)" ".* = false"
> +    gdb_test "print (int)pmi == sizeof(int)" ".* = true"
>   }
>   
>   # Test static members.
> diff --git a/gdb/testsuite/gdb.cp/ctti.exp b/gdb/testsuite/gdb.cp/ctti.exp
> index 0c02085cec6..4d117618a75 100644
> --- a/gdb/testsuite/gdb.cp/ctti.exp
> +++ b/gdb/testsuite/gdb.cp/ctti.exp
> @@ -41,17 +41,11 @@ gdb_test "print c" "\\$\[0-9\]+ = 194 .*"
>   gdb_test "print f" "\\$\[0-9\]+ = 9"
>   gdb_test "print i" "\\$\[0-9\]+ = 4"
>   
> -# TODO: this needs more work before actually deploying it.
> -# So bail out here.
> -
> -if {[test_compiler_info gcc-*]} { return }
> -
>   gdb_test_multiple "print add<int>(2,2)" "print add<int>(2,2)" {
>       -re "\\$\[0-9\]+ = 4\r\n$gdb_prompt $" {
>   	pass "print add<int>(2,2)"
>       }
>       -re "No symbol \"add<int>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add<int>(2,2)"
>       }
>   }
> @@ -62,7 +56,6 @@ gdb_test_multiple "print add<float>(2.25,2.25)" "print add<float>(2.25,2.25)" {
>   	pass "print add<float>(2.25,2.25)"
>       }
>       -re "No symbol \"add<float>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add<float>(2.25,2.25)"
>       }
>   }
> @@ -72,7 +65,6 @@ gdb_test_multiple "print add<unsigned char>('A','A')" "print add<unsigned char>(
>   	pass "print add<unsigned char>('A','A')"
>       }
>       -re "No symbol \"add<unsigned char>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add<unsigned char>('A','A')"
>       }
>   }
> @@ -82,7 +74,6 @@ gdb_test_multiple "print add2<int>(2,2)" "print add2<int>(2,2)" {
>   	pass "print add2<int>(2,2)"
>       }
>       -re "No symbol \"add2<int>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add2<int>(2,2)"
>       }
>   }
> @@ -92,7 +83,6 @@ gdb_test_multiple "print add2<float>(2.25,2.25)" "print add2<float>(2.25,2.25)"
>   	pass "print add2<float>(2.25,2.25)"
>       }
>       -re "No symbol \"add2<float>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add2<float>(2.25,2.25)"
>       }
>   }
> @@ -102,7 +92,6 @@ gdb_test_multiple "print add2<unsigned char>('A','A')" "print add2<unsigned char
>   	pass "print add2<unsigned char>('A','A')"
>       }
>       -re "No symbol \"add2<unsigned char>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add2<unsigned char>('A','A')"
>       }
>   }
> @@ -112,7 +101,6 @@ gdb_test_multiple "print add3<int>(2,2)" "print add3<int>(2,2)" {
>   	pass "print add3<int>(2,2)"
>       }
>       -re "No symbol \"add3<int>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add3<int>(2,2)"
>       }
>   }
> @@ -122,7 +110,6 @@ gdb_test_multiple "print add3<float>(2.25,2.25)" "print add3<float>(2.25,2.25)"
>   	pass "print add3<float>(2.25,2.25)"
>       }
>       -re "No symbol \"add3<float>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add3<float>(2.25,2.25)"
>       }
>   }
> @@ -132,7 +119,6 @@ gdb_test_multiple "print add3<unsigned char>('A','A')" "print add3<unsigned char
>   	pass "print add3<unsigned char>('A','A')"
>       }
>       -re "No symbol \"add3<unsigned char>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add3<unsigned char>('A','A')"
>       }
>   }
> @@ -142,7 +128,6 @@ gdb_test_multiple "print add4<int>(2,2)" "print add4<int>(2,2)" {
>   	pass "print add4<int>(2,2)"
>       }
>       -re "No symbol \"add4<int>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add4<int>(2,2)"
>       }
>   }
> @@ -152,7 +137,6 @@ gdb_test_multiple "print add4<float>(2.25,2.25)" "print add4<float>(2.25,2.25)"
>   	pass "print add4<float>(2.25,2.25)"
>       }
>       -re "No symbol \"add4<float>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add4<float>(2.25,2.25)"
>       }
>   }
> @@ -162,7 +146,6 @@ gdb_test_multiple "print add4<unsigned char>('A','A')" "print add4<unsigned char
>   	pass "print add4<unsigned char>('A','A')"
>       }
>       -re "No symbol \"add4<unsigned char>\" in current context.\r\n$gdb_prompt $" {
> -	# TODO: kfail or xfail this
>   	fail "print add4<unsigned char>('A','A')"
>       }
>   }
> diff --git a/gdb/testsuite/gdb.cp/exception.exp b/gdb/testsuite/gdb.cp/exception.exp
> index 4308d327da3..2c7ef7793ee 100644
> --- a/gdb/testsuite/gdb.cp/exception.exp
> +++ b/gdb/testsuite/gdb.cp/exception.exp
> @@ -27,9 +27,6 @@
>   # Static-linked executables use a different mechanism to get the
>   # address of the notification hook in the C++ support library.
>   
> -# TODO: this file has many absolute line numbers.
> -# Replace them with gdb_get_line_number.
> -
>   set ws	"\[\r\n\t \]+"
>   set nl	"\[\r\n\]+"
>   
> @@ -105,15 +102,9 @@ gdb_test "break catcher" "Breakpoint \[0-9\]+ at.*"
>   
>   # Get the first exception thrown
>          
> -set name "continue to first throw"
> -gdb_test_multiple "continue" $name {
> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception thrown\\), throw location.*${srcfile}:30, catch location .*${srcfile}:50\r\n$gdb_prompt $" {
> -	pass $name
> -    }
> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception thrown\\).*\r\n$gdb_prompt $" {
> -	pass $name
> -    }
> -}
> +gdb_test "continue"  \
> +    "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception thrown\\).*" \
> +    "continue to first throw"
>   
>   # Backtrace from the throw point.
>   # This should get to user code.
> @@ -129,15 +120,9 @@ gdb_test_multiple "backtrace" $name {
>   
>   # Continue to the catch.
>   
> -set name "continue to first catch"
> -gdb_test_multiple "continue" $name {
> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\), throw location.*${srcfile}:30, catch location .*${srcfile}:50\r\n$gdb_prompt $" {
> -	pass $name
> -    }
> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\).*\r\n$gdb_prompt $" {
> -	pass $name
> -    }
> -}
> +gdb_test "continue" \
> +    "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\).*" \
> +    "continue to first catch"
>   
>   # Backtrace from the catch point.
>   # This should get to user code.
> @@ -154,15 +139,9 @@ gdb_test "continue" ".*catcher \\(x=13\\).*" "continue to catcher for the first
>   
>   # Continue to second throw.
>   
> -set name "continue to second throw"
> -gdb_test_multiple "continue" $name {
> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception thrown\\), throw location.*${srcfile}:30, catch location .*${srcfile}:58\r\n$gdb_prompt $" {
> -	pass $name
> -    }
> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception thrown\\).*\r\n$gdb_prompt $" {
> -	pass $name
> -    }
> -}
> +gdb_test "continue" \
> +    "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception thrown\\).*" \
> +    "continue to second throw"
>   
>   # Backtrace from the throw point.
>   # This should get to user code.
> @@ -176,15 +155,9 @@ gdb_test_multiple "backtrace" $name {
>   
>   # Continue to second catch.
>   
> -set name "continue to second catch"
> -gdb_test_multiple "continue" $name {
> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\), throw location.*${srcfile}:30, catch location .*${srcfile}:58\r\n$gdb_prompt $" {
> -	pass $name
> -    }
> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\).*\r\n$gdb_prompt $" {
> -	pass $name
> -    }
> -}
> +gdb_test "continue" \
> +    "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\).*" \
> +    "continue to second catch"
>   
>   # Backtrace from the catch point.
>   # This should get to user code.
  

>> * exception.exp - the code relying on the line numbers can't work,
>> because gdb never prints that message anyway.

Bruno> If that is dead code, why not remove those and just have a simple gdb_test?

Yeah, I did do that, e.g.:

>> # Continue to second catch.
>> -set name "continue to second catch"
>> -gdb_test_multiple "continue" $name {
>> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\), throw location.*${srcfile}:30, catch location .*${srcfile}:58\r\n$gdb_prompt $" {
>> -	pass $name
>> -    }
>> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\).*\r\n$gdb_prompt $" {
>> -	pass $name
>> -    }
>> -}
>> +gdb_test "continue" \
>> +    "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\).*" \
>> +    "continue to second catch"
>> # Backtrace from the catch point.
>> # This should get to user code.

Tom
  

On 11/07/2023 15:17, Tom Tromey wrote:
>>> * exception.exp - the code relying on the line numbers can't work,
>>> because gdb never prints that message anyway.
> Bruno> If that is dead code, why not remove those and just have a simple gdb_test?
>
> Yeah, I did do that, e.g.:
>
>>> # Continue to second catch.
>>> -set name "continue to second catch"
>>> -gdb_test_multiple "continue" $name {
>>> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\), throw location.*${srcfile}:30, catch location .*${srcfile}:58\r\n$gdb_prompt $" {
>>> -	pass $name
>>> -    }
>>> -    -re "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\).*\r\n$gdb_prompt $" {
>>> -	pass $name
>>> -    }
>>> -}
>>> +gdb_test "continue" \
>>> +    "Continuing.${ws}Catchpoint \[0-9\]+ \\(exception caught\\).*" \
>>> +    "continue to second catch"
>>> # Backtrace from the catch point.
>>> # This should get to user code.

Great! With these changes, looks good to me :)

Reviewed-By: Bruno Larsen <blarsen@redhat.com>