Re: [PATCH 10/11] Match dynamic="1" in the output of -var-list-children

[Keith Seitz <keiths@redhat.com>]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> I am not satisfied with the regexp construction for each child in
> mi_child_regexp.

Agreed.

> mi_list_varobj_children "struct_declarations" {
>      {struct_declarations.integer integer 0 int}
> } "test"
>
> will be rewritten to:
>
> mi_list_varobj_children "struct_declarations" {
>      {struct_declarations.integer integer 0 {type int}}
> } "test"
>
> if we want to match "dynamic" attribute, we can write:
>
> mi_list_varobj_children "struct_declarations" {
>      {struct_declarations.integer integer 0 {type int dynamic 1}}
> } "test"
>

That sounds reasonable. A slightly easier syntax to parse would use a 
list of lists for this optional argument. That would then generically 
allow flags to be used, further simplifying it to "{{type int} dynamic}".

But then just about anything is better than what we have now, which 
relies on the /length/ of the list to determine what to add to the 
regexp. Yuck!

> Since mi_list_varobj_children has been widely used in test suite, I'd
> like to defer the change after this patch series.

Agreed.

Just one little thing below. Otherwise, this looks okay to me.

Keith

> 2013-11-24  Yao Qi  <yao@codesourcery.com>
>
> 	* lib/mi-support.exp (mi_child_regexp): Append 'dynamic="1"' to
> 	children_exp.
> ---
>   gdb/testsuite/lib/mi-support.exp |   16 ++++++++++------
>   1 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
> index 0c3cdbe..99ff9a5 100644
> --- a/gdb/testsuite/lib/mi-support.exp
> +++ b/gdb/testsuite/lib/mi-support.exp
> @@ -1410,21 +1410,25 @@ proc mi_child_regexp {children add_child} {
>   	set name [lindex $item 0]
>   	set exp [lindex $item  1]
>   	set numchild [lindex $item 2]
> +
> +	set line "$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\""
> +
>   	if {[llength $item] == 5} {
>   	    set type [lindex $item 3]
>   	    set value [lindex $item 4]
>
> -	    lappend children_exp\
> -		"$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\",value=\"$value\",type=\"$type\"(,thread-id=\"\[0-9\]+\")?}"
> +	    append line ",value=\"$value\",type=\"$type\""
>   	} elseif {[llength $item] == 4} {
>   	    set type [lindex $item 3]
>
> -	    lappend children_exp\
> -		"$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\",type=\"$type\"(,thread-id=\"\[0-9\]+\")?}"
> +	    append line ",type=\"$type\""
>   	} else {
> -	    lappend children_exp\
> -		"$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\"(,thread-id=\"\[0-9\]+\")?}"
>   	}

This 'else' block is now empty and can be removed.

> +
> +	append line \
> +	    "(,thread-id=\"\[0-9\]+\")?(,dynamic=\"1\")?}"
> +
> +	lappend children_exp $line
>       }
>       return [join $children_exp ","]
>   }
>