This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA 2/3] Linespec rewrite: Cleanups


On 03/19/2012 09:21 PM, Keith Seitz wrote:

> Hi,
> 
> This second patch deals with cleanups in the codebase and test suite: removing unused functions, updating test suite error messages, and the like.
> 
> Keith
> 


> 2012-03-19  Keith Seitz  <keiths@redhat.com>
> 
>     * gdb.base/advance.exp: Update error message for
>     "advance malformed" test.
>     * gdb.base/break.exp: Likewise for "breakpoint with
>     trailing garbage" test.
>     * gdb.base/hbreak2.exp: Likewise for "hardware breakpoint
>     with trailing garbage" test.
>     * gdb.base/sepdebug.exp: Likewise for "breakpoint with
>     trailng garbage" test.
>     * gdb.base/until.exp: Likewise for "malformed until" test.
>     * gdb.base/list.exp: Update "list 'list0.c:main'", which contains
>     invalid quoting.
>     * gdb.cp/overload.exp: Fix invalidly quoted linespec.
>     * gdb.cp/ovldbreak.exp: Create the breakpoint table
>     for "breakpoint info (after setting on all)".
>     * gdb.cp/static-method.exp: Remove redundant (and invalid
>     linespec) test "list '$srcfile:$test'".
>     * gdb.cp/userdef.exp: Remove quoting for "break A2::operator+"
>     tests.


Some of these adjust to new expected output.  It'd be better if those
were committed along with the patch that actually changes GDB's output.
IOW, it'd be better if those hunks were part of patch 1.

Other hunk adjust the tests to not try specs that are supposedly "invalid" in
the first place, and that we will no longer support.  It'd be best to split
those out into a separate patch, and one that is applied _before_ the linespec
rewrite.  It'd be very good for posterity if all those changes that supposedly
stop feeding GDB invalid input were clearly justified in the corresponding
patch description.

In particular:

> --- a/gdb/testsuite/gdb.base/list.exp
> +++ b/gdb/testsuite/gdb.base/list.exp
> @@ -468,7 +468,7 @@ proc test_list_filename_and_function {} {
>      pass "list filename:function ($testcnt tests)"
>  
>      # Test with quoting.
> -    gdb_test "list 'list0.c:main'" "int main.*"
> +    gdb_test "list 'list0.c':'main'" "int main.*"


this

> diff --git a/gdb/testsuite/gdb.cp/overload.exp b/gdb/testsuite/gdb.cp/overload.exp
> index 5116e5f..f3e56c7 100644
> --- a/gdb/testsuite/gdb.cp/overload.exp
> +++ b/gdb/testsuite/gdb.cp/overload.exp
> @@ -306,7 +306,6 @@ gdb_test "list \"foo::overloadfnarg(int, int (*)(int))\"" \
>  gdb_test "list ${srcfile}:intToChar" "int intToChar.*"
>  gdb_test "list ${srcfile}:intToChar(char)" "int intToChar.*"
>  gdb_test "list ${srcfile}:'intToChar(char)'" "int intToChar.*"
> -gdb_test "list '${srcfile}:intToChar(char)'" "int intToChar.*"
>  gdb_test "list '${srcfile}':intToChar(char)" "int intToChar.*"
>  gdb_test "list '${srcfile}':'intToChar(char)'" "int intToChar.*"

and this

>  
>  # Run through each breakpoint.
>  proc continue_to_bp_overloaded {bpnumber might_fail line argtype argument} {
> diff --git a/gdb/testsuite/gdb.cp/static-method.exp b/gdb/testsuite/gdb.cp/static-method.exp
> index 6e086f3..576666c 100644
> --- a/gdb/testsuite/gdb.cp/static-method.exp
> +++ b/gdb/testsuite/gdb.cp/static-method.exp
> @@ -95,11 +95,6 @@ foreach test $methods {
>  	&& !$have_gcc_45682_fixed} {
>  	setup_xfail gcc/45682 "*-*-*"
>      }
> -    gdb_test "list '${srcfile}:$test'" $result
> -    if {[string compare $test "xxx::${ans}::A::func"] == 0
> -	&& !$have_gcc_45682_fixed} {
> -	setup_xfail gcc/45682 "*-*-*"
> -    }


and this

>      gdb_test "list '${srcfile}':'$test'" $result
>      if {[string compare $test "xxx::${ans}::A::func"] == 0
>  	&& !$have_gcc_45682_fixed} {
> diff --git a/gdb/testsuite/gdb.cp/userdef.exp b/gdb/testsuite/gdb.cp/userdef.exp
> index 9c7cb57..90eb99d 100644
> --- a/gdb/testsuite/gdb.cp/userdef.exp
> +++ b/gdb/testsuite/gdb.cp/userdef.exp
> @@ -136,8 +136,8 @@ gdb_test "print one += 7" "\\\$\[0-9\]* = {x = 9, y = 10}"
>  gdb_test "print two = one" "\\\$\[0-9\]* = {x = 9, y = 10}"
>  
>  # Check that GDB tolerates whitespace in operator names.
> -gdb_test "break A2::'operator+'" ".*Breakpoint $decimal at.*"
> -gdb_test "break A2::'operator +'" ".*Breakpoint $decimal at.*"
> +gdb_test "break A2::operator+" ".*Breakpoint $decimal at.*"
> +gdb_test "break A2::operator +" ".*Breakpoint $decimal at.*"
>  


and this.


> diff --git a/gdb/testsuite/gdb.cp/ovldbreak.exp b/gdb/testsuite/gdb.cp/ovldbreak.exp
> index 3e35b79..2c27f2d 100644
> --- a/gdb/testsuite/gdb.cp/ovldbreak.exp
> +++ b/gdb/testsuite/gdb.cp/ovldbreak.exp
> @@ -320,23 +320,17 @@ gdb_expect {
>      }
>  }
>
> -gdb_test "info break" \
> -    "Num     Type\[\t \]+Disp Enb Address\[\t \]+What.*
> -\[0-9\]+\[\t \]+breakpoint     keep y\[\t \]+<MULTIPLE>\[\t \]*\r
> -\[0-9\]+.1\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(double\\) at.*$srcfile:140\r
> -\[0-9\]+.2\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(float\\) at.*$srcfile:137\r
> -\[0-9\]+.3\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\((unsigned long|long unsigned)( int)?\\) at.*$srcfile:134\r
> -\[0-9\]+.4\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(long( int)?\\) at.*$srcfile:131\r
> -\[0-9\]+.5\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\((unsigned|unsigned int)\\) at.*$srcfile:128\r
> -\[0-9\]+.6\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(int\\) at.*$srcfile:125\r
> -\[0-9\]+.7\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\((unsigned short|short unsigned)( int)?\\) at.*$srcfile:122\r
> -\[0-9\]+.8\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(short( int)?\\) at.*$srcfile:119\r
> -\[0-9\]+.9\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(unsigned char\\) at.*$srcfile:116\r
> -\[0-9\]+.10\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(signed char\\) at.*$srcfile:113\r
> -\[0-9\]+.11\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(char\\) at.*$srcfile:110\r
> -\[0-9\]+.12\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\((void|)\\) at.*$srcfile:107" \
> -    "breakpoint info (after setting on all)"
> +# Create the breakpoint table for "info breakpoint".
> +set bptable "Num     Type\[\t \]+Disp Enb Address\[\t \]+What.*\[\r\n]+"
> +append bptable "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep\[\t \]y\[\t \]+<MULTIPLE>.*\[\r\n\]+"
> +foreach ovld {void char signed_char unsigned_char short_int \
> +		  unsigned_short_int int unsigned_int long_int \
> +		  unsigned_long_int float double} {
> +  append bptable [format "\[0-9\]+.\[0-9\]+\[\t \]+y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(%s\\) at.*$srcfile:%d\[\r\n\]+" \
> +		      $types($ovld) $line($ovld)]
> +}
>
> +gdb_test "info break" $bptable "breakpoint info (after setting on all)"

Is this change related to linespecs?  Looks like it should be split to its
own standalone patch.

> ChangeLog
> 2012-03-19  Keith Seitz  <keiths@redhat.com>
>
>     * cp-support.c (SKIP_SPACE): Remove.
>     (cp_validate_operator): Remove.
>     * cp-support.h (cp_validate_operator): Remove declaration.
>
> testsuite/ChangeLog

Dead code removal is best split into its own patch, in order to
have functional and non-functional changes in separate commits.

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]