[RFC] Change how value is shown for varobjs of type vector.

[RFC] Change how value is shown for varobjs of type vector.

[Andrew Burgess]

Consider:

> cat vec.c 
char vector __attribute__ ((vector_size (8))) = { 0, 0, 0, 0, 0, 0, 0, 0 };

int
main ()
{
  /* Nothing.  */
}
> gcc -g -o vec.x vec.c
> gdb vec.x
(gdb) start
(gdb) interpreter-exec mi2 "-var-create vector * vector"
^done,name="vector",numchild="8",value="[8]",type="char [8]",has_more="0"

## END ##

I have a patch (below) that changes the value field from the current "[8]" to "{ 0, 0, 0, 0, 0, 0, 0, 0 }".  The varobj still has 8 children, and the top level varobj is still not editable, you have to edit through the children.

This better suits my local usage where vectors are generally used with just a small number of entries, each vector type has a total size of one 64-bit register, the frontend used to view this MI data is just passing the value field through, and my users wanted to visualise the set of values in a single field.

I've not finished the patch yet with changelog or tests because I wasn't sure if this change was going to be acceptable, so I'm looking for some feedback please.  One possibility would be to make this behaviour switchable, if it can't be the default.

Thanks,

Andrew


diff --git a/gdb/varobj.c b/gdb/varobj.c
index 6699699..c72716b 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -3119,10 +3119,13 @@ default_value_is_changeable_p (struct varobj *var)
     {
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
-    case TYPE_CODE_ARRAY:
       r = 0;
       break;
 
+    case TYPE_CODE_ARRAY:
+      r = TYPE_VECTOR (type);
+      break;
+
     default:
       r = 1;
     }
@@ -3507,12 +3510,16 @@ c_value_of_variable (struct varobj *var, enum varobj_display_formats format)
 
     case TYPE_CODE_ARRAY:
       {
-	char *number;
+	if (!TYPE_VECTOR (type))
+	  {
+	    char *number;
 
-	number = xstrprintf ("[%d]", var->num_children);
-	return (number);
+	    number = xstrprintf ("[%d]", var->num_children);
+	    return (number);
+	    /* break; */
+	  }
       }
-      /* break; */
+      /* fall through  */
 
     default:
       {

Re: [RFC] Change how value is shown for varobjs of type vector.

[Andrew Burgess]

*ping*

On 26/09/2012 4:23 PM, Andrew Burgess wrote:
> Consider:
> 
>> cat vec.c 
> char vector __attribute__ ((vector_size (8))) = { 0, 0, 0, 0, 0, 0, 0, 0 };
> 
> int
> main ()
> {
>   /* Nothing.  */
> }
>> gcc -g -o vec.x vec.c
>> gdb vec.x
> (gdb) start
> (gdb) interpreter-exec mi2 "-var-create vector * vector"
> ^done,name="vector",numchild="8",value="[8]",type="char [8]",has_more="0"
> 
> ## END ##
> 
> I have a patch (below) that changes the value field from the current "[8]" to "{ 0, 0, 0, 0, 0, 0, 0, 0 }".  The varobj still has 8 children, and the top level varobj is still not editable, you have to edit through the children.
> 
> This better suits my local usage where vectors are generally used with just a small number of entries, each vector type has a total size of one 64-bit register, the frontend used to view this MI data is just passing the value field through, and my users wanted to visualise the set of values in a single field.
> 
> I've not finished the patch yet with changelog or tests because I wasn't sure if this change was going to be acceptable, so I'm looking for some feedback please.  One possibility would be to make this behaviour switchable, if it can't be the default.
> 
> Thanks,
> 
> Andrew
> 
> 
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 6699699..c72716b 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -3119,10 +3119,13 @@ default_value_is_changeable_p (struct varobj *var)
>      {
>      case TYPE_CODE_STRUCT:
>      case TYPE_CODE_UNION:
> -    case TYPE_CODE_ARRAY:
>        r = 0;
>        break;
>  
> +    case TYPE_CODE_ARRAY:
> +      r = TYPE_VECTOR (type);
> +      break;
> +
>      default:
>        r = 1;
>      }
> @@ -3507,12 +3510,16 @@ c_value_of_variable (struct varobj *var, enum varobj_display_formats format)
>  
>      case TYPE_CODE_ARRAY:
>        {
> -	char *number;
> +	if (!TYPE_VECTOR (type))
> +	  {
> +	    char *number;
>  
> -	number = xstrprintf ("[%d]", var->num_children);
> -	return (number);
> +	    number = xstrprintf ("[%d]", var->num_children);
> +	    return (number);
> +	    /* break; */
> +	  }
>        }
> -      /* break; */
> +      /* fall through  */
>  
>      default:
>        {
> 
> 
> 
> 
> 
> 
> 

Re: [RFC] Change how value is shown for varobjs of type vector.

[Joel Brobecker]

> I have a patch (below) that changes the value field from the current
> "[8]" to "{ 0, 0, 0, 0, 0, 0, 0, 0 }".  The varobj still has 8
> children, and the top level varobj is still not editable, you have to
> edit through the children.

I personally do not have a strong objection to this change, but I have
to admit that I am not particularly fond of the idea. I think that
this issue should be handled at the front-end level (Eg. Eclipse),
and in a manor that is general to all arrays. One possible suggestion
would be to have the IDE detect small arrays of integrals, and in
that case display them in a more convenient way right away. But
perhaps it isn't easy, in which case maybe a change in GDB would make
better sense. I just don't like it because it breaks a little bit
the consistency of the value field.

-- 
Joel

RE: [RFC] Change how value is shown for varobjs of type vector.

[Marc Khouzam]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Joel Brobecker
> Sent: Sunday, October 14, 2012 1:25 PM
> To: Andrew Burgess
> Cc: gdb-patches@sourceware.org
> Subject: Re: [RFC] Change how value is shown for varobjs of 
> type vector.
> 
> > I have a patch (below) that changes the value field from the current
> > "[8]" to "{ 0, 0, 0, 0, 0, 0, 0, 0 }".  The varobj still has 8
> > children, and the top level varobj is still not editable, 
> you have to
> > edit through the children.
> 
> I personally do not have a strong objection to this change, but I have
> to admit that I am not particularly fond of the idea. I think that
> this issue should be handled at the front-end level (Eg. Eclipse),
> and in a manor that is general to all arrays. One possible suggestion
> would be to have the IDE detect small arrays of integrals, and in
> that case display them in a more convenient way right away. But
> perhaps it isn't easy, in which case maybe a change in GDB would make
> better sense. I just don't like it because it breaks a little bit
> the consistency of the value field.

I have to admit that Eclipse does not explicitly handle the case
of the vector_size __attribute__.  Whatever GDB returns to
-var-create is shown directly.  So, currently, we'll show [8] as
the value.  However, showing { 0, 0, 0, 0, 0, 0, 0, 0 } would not
match our standard behavior.  For arrays, we show the address
of the array as the unexpanded value, and then show the content 
in a 'details pane' when the user explicitly selects that array.  
We get the content that you are looking for using
  -data-evaluate-expression vector
  ^done,value="{0, 0, 0, 0, 0, 0, 0, 0}"

So, I agree with Joel that it would be better to let the front-end
deal with this situation (although this is not my call to make :) )

Although I haven't tried, I believe we can figure out this case
should be handled like the array case by improving Eclipse's type
parser to handle the vector_size __attribute__.

Thanks

Marc

Re: [RFC] Change how value is shown for varobjs of type vector.

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Andrew> I have a patch (below) that changes the value field from the current
Andrew> "[8]" to "{ 0, 0, 0, 0, 0, 0, 0, 0 }".  The varobj still has 8
Andrew> children, and the top level varobj is still not editable, you have to
Andrew> edit through the children.

Joel> I personally do not have a strong objection to this change, but I have
Joel> to admit that I am not particularly fond of the idea. I think that
Joel> this issue should be handled at the front-end level (Eg. Eclipse),

I think so too.

Marc> Although I haven't tried, I believe we can figure out this case
Marc> should be handled like the array case by improving Eclipse's type
Marc> parser to handle the vector_size __attribute__.

One thought I had was that, if gdb does not currently emit enough
information for Eclipse to treat vectors specially, then we could add a
"displayhint" field even to non-dynamic varobjs (or of course emit the
needed info in some other way).

Tom

Re: [RFC] Change how value is shown for varobjs of type vector.

[Andrew Burgess]

Joel, Marc, Tom,

Thanks for taking the time to review this patch.  I'll move this
functionality into eclipse.

Thanks,
Andrew



On 26/09/2012 4:23 PM, Andrew Burgess wrote:
> Consider:
> 
>> cat vec.c 
> char vector __attribute__ ((vector_size (8))) = { 0, 0, 0, 0, 0, 0, 0, 0 };
> 
> int
> main ()
> {
>   /* Nothing.  */
> }
>> gcc -g -o vec.x vec.c
>> gdb vec.x
> (gdb) start
> (gdb) interpreter-exec mi2 "-var-create vector * vector"
> ^done,name="vector",numchild="8",value="[8]",type="char [8]",has_more="0"
> 
> ## END ##
> 
> I have a patch (below) that changes the value field from the current "[8]" to "{ 0, 0, 0, 0, 0, 0, 0, 0 }".  The varobj still has 8 children, and the top level varobj is still not editable, you have to edit through the children.
> 
> This better suits my local usage where vectors are generally used with just a small number of entries, each vector type has a total size of one 64-bit register, the frontend used to view this MI data is just passing the value field through, and my users wanted to visualise the set of values in a single field.
> 
> I've not finished the patch yet with changelog or tests because I wasn't sure if this change was going to be acceptable, so I'm looking for some feedback please.  One possibility would be to make this behaviour switchable, if it can't be the default.
> 
> Thanks,
> 
> Andrew
> 
> 
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 6699699..c72716b 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -3119,10 +3119,13 @@ default_value_is_changeable_p (struct varobj *var)
>      {
>      case TYPE_CODE_STRUCT:
>      case TYPE_CODE_UNION:
> -    case TYPE_CODE_ARRAY:
>        r = 0;
>        break;
>  
> +    case TYPE_CODE_ARRAY:
> +      r = TYPE_VECTOR (type);
> +      break;
> +
>      default:
>        r = 1;
>      }
> @@ -3507,12 +3510,16 @@ c_value_of_variable (struct varobj *var, enum varobj_display_formats format)
>  
>      case TYPE_CODE_ARRAY:
>        {
> -	char *number;
> +	if (!TYPE_VECTOR (type))
> +	  {
> +	    char *number;
>  
> -	number = xstrprintf ("[%d]", var->num_children);
> -	return (number);
> +	    number = xstrprintf ("[%d]", var->num_children);
> +	    return (number);
> +	    /* break; */
> +	  }
>        }
> -      /* break; */
> +      /* fall through  */
>  
>      default:
>        {
> 
> 
> 
> 
> 
> 
>