[MI] lvalues and variable_editable

[MI] lvalues and variable_editable

[Nick Roberts]

I've been wondering what the difference is between varobj_value_is_changeable_p
and variable_editable.  Recent discussion on the gdb mailing list makes
me think that, in essence, a value is only changeable but not editable when
it's an lvalue -- but currently varobj.c doesn't capture this difference.

Here are two experimental patches:

1) varobj.c: Test if the value of variable object is an lvalue.  I think this
   can only occur for root values.

2) mi-cmd-var.c: Output a field with -var-create and -var-list-children
   when noneditable.

The testsuite only needs to change for 2).

It would seem sensible to define variable_editable using
varobj_value_is_changeable_p and I don't think language dependent versions
aren't needed.

WDYT?

-- 
Nick                                           http://www.inet.net.nz/~nickrob



*** varobj.c	14 Apr 2007 21:51:29 +1200	1.89
--- varobj.c	26 Jun 2007 23:12:52 +1200	
*************** c_type_of_child (struct varobj *parent, 
*** 2131,2136 ****
--- 2131,2151 ----
  static int
  c_variable_editable (struct varobj *var)
  {
+   struct expression *exp;
+   struct value *value;
+ 
+   if (is_root_p (var))
+     {
+       if (!gdb_evaluate_expression (var->root->exp, &value))
+ 	{
+ 	  /* We cannot proceed without a valid expression. */
+ 	  xfree (exp);
+ 	  return 0;
+ 	}
+       if (!VALUE_LVAL(value))
+ 	return 0;
+     }
+ 
    switch (TYPE_CODE (get_value_type (var)))
      {
      case TYPE_CODE_STRUCT:


*** mi-cmd-var.c	14 Jun 2007 10:12:15 +1200	1.33
--- mi-cmd-var.c	26 Jun 2007 21:32:26 +1200	
*************** print_varobj (struct varobj *var, enum p
*** 66,71 ****
--- 66,74 ----
        xfree (type);
      }
  
+   if (!(varobj_get_attributes (var) & 0x00000001))
+     ui_out_field_string (uiout, "attr", "noneditable");
+ 
    if (varobj_get_frozen (var))
      ui_out_field_int (uiout, "frozen", 1);
  }

Re: [MI] lvalues and variable_editable

[Daniel Jacobowitz]

On Tue, Jun 26, 2007 at 11:36:16PM +1200, Nick Roberts wrote:
> 
> I've been wondering what the difference is between varobj_value_is_changeable_p
> and variable_editable.  Recent discussion on the gdb mailing list makes
> me think that, in essence, a value is only changeable but not editable when
> it's an lvalue -- but currently varobj.c doesn't capture this difference.
> 
> Here are two experimental patches:
> 
> 1) varobj.c: Test if the value of variable object is an lvalue.  I think this
>    can only occur for root values.

I'm still a bit confused but I think this is because I read "lvalue"
and think of the C language meaning.  An lvalue is something which can
occur on the left hand side of an assignment.  So it can happen for
children too, for instance.

Right now a value is "changeable" unless it is a fake child
(e.g. "public"), a struct, a union, or an array.  This makes some
sense since the value we print out for those cases is not useful for
editing.  For instance "{...}".  Their children will, I believe, be
changeable.

A value is editable in C unless it is a struct, union, array,
function, or method.  C++ adds the fake access children again.

So the only things which are editable but not changeable are functions
and methods.  That doesn't seem like a useful distinction.

Back in revision 1.1, fake children and structs and unions were not
changeable.  Those and arrays, functions, methods, and members were
not editable.  That doesn't make considerably more sense to me either.

The difference in usage seems to be that we forbid attempts to modify
non-editable variables, and we omit reporting changes for
non-changeable variables.

> It would seem sensible to define variable_editable using
> varobj_value_is_changeable_p and I don't think language dependent versions
> aren't needed.

I agree.  But is there anything which should be one and not the other?
Maybe we can dispense with changeable entirely.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI] lvalues and variable_editable

[Nick Roberts]

 > > Here are two experimental patches:
 > > 
 > > 1) varobj.c: Test if the value of variable object is an lvalue.  I think
 > >    this can only occur for root values.
 > 
 > I'm still a bit confused but I think this is because I read "lvalue"
 > and think of the C language meaning.  An lvalue is something which can
 > occur on the left hand side of an assignment.  So it can happen for
 > children too, for instance.

AFAIK lvalue isn't a language specific context.  Here's how to specify an
lvalue for a root variable:

-var-create - * "i1 + i2"

Can you give an example of how you would specify an lvalue for children?

 > Right now a value is "changeable" unless it is a fake child
 > (e.g. "public"), a struct, a union, or an array.  This makes some
 > sense since the value we print out for those cases is not useful for
 > editing.  For instance "{...}".  Their children will, I believe, be
 > changeable.
 > 
 > A value is editable in C unless it is a struct, union, array,
 > function, or method.  C++ adds the fake access children again.

Yes.  varobj_value_is_changeable_p uses the test "if (CPLUS_FAKE_CHILD (var))"
in a language independent way, so variable_editable could also.

 > So the only things which are editable but not changeable are functions
 > and methods.  That doesn't seem like a useful distinction.
 >
 > Back in revision 1.1, fake children and structs and unions were not
 > changeable.  Those and arrays, functions, methods, and members were
 > not editable.  That doesn't make considerably more sense to me either.
 > 
 > The difference in usage seems to be that we forbid attempts to modify
 > non-editable variables, and we omit reporting changes for
 > non-changeable variables.
 > 
 > > It would seem sensible to define variable_editable using
 > > varobj_value_is_changeable_p and I don't think language dependent versions
 > > aren't needed.
 > 
 > I agree.  But is there anything which should be one and not the other?

My initial premise was: "a value is only changeable but not editable when
it's an lvalue."  

 > Maybe we can dispense with changeable entirely.

I don't think so.  The attribute changeable tells GDB whether it needs to check
if the value has changed. The attribute editable tells the frontend whether the
user should be allowed to try to edit the value, which was the source of Sascha
Radike's bug report (BUG: MI reporting wrong attributes for casted variables).

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [MI] lvalues and variable_editable

[Nick Roberts]

I mean:

 >  > I'm still a bit confused but I think this is because I read "lvalue"
 >  > and think of the C language meaning.  An lvalue is something which can
 >  > occur on the left hand side of an assignment.  So it can happen for
 >  > children too, for instance.
 > 
 > AFAIK lvalue isn't a language specific context.  Here's how to specify an
                                          ^^^^^^^                         ^^
                                          concept
 > lvalue for a root variable:
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^
   a root variable that's _not_ an lvalue:
 > 
 > -var-create - * "i1 + i2"
 > 
 > Can you give an example of how you would specify an lvalue for children?
                                                    ^^^^^^^^^^^^^^^^^^^^^^
                                            children that _aren't_ lvalues?


-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [MI] lvalues and variable_editable

[Daniel Jacobowitz]

On Wed, Jul 04, 2007 at 03:11:45PM +1200, Nick Roberts wrote:
>  > Can you give an example of how you would specify an lvalue for children?
>                                                     ^^^^^^^^^^^^^^^^^^^^^^
>                                             children that _aren't_ lvalues?

Not offhand.  I thought you said all children were non-editable;
that's what I was meaning to disagree with, anyway.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI] lvalues and variable_editable

[Nick Roberts]

Daniel Jacobowitz writes:
 > On Wed, Jul 04, 2007 at 03:11:45PM +1200, Nick Roberts wrote:
 > >  > Can you give an example of how you would specify an lvalue for children?
 > >                                                     ^^^^^^^^^^^^^^^^^^^^^^
 > >                                             children that _aren't_ lvalues?
 > 
 > Not offhand.  I thought you said all children were non-editable;
 > that's what I was meaning to disagree with, anyway.

I probably missed a negative in my first post too ("Test if the value of
variable object is --not-- an lvalue.")

Anyway let me submit a more complete patch, in due course, for consideration
after the release.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [MI] lvalues and variable_editable

[Daniel Jacobowitz]

On Wed, Jul 04, 2007 at 03:35:15PM +1200, Nick Roberts wrote:
> Anyway let me submit a more complete patch, in due course, for consideration
> after the release.

Great!

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI] lvalues and variable_editable

[Nick Roberts]

(Me)

 > Anyway let me submit a more complete patch, in due course, for consideration
 > after the release.

This is what I have in mind.  There are no regressions, at least with MI (I
can't test Insight).  If there's still time, this could go in before the
branch.  I also have a (much smaller) change, that I've submitted earlier, for
after the release which adds the editable field to the output of -var-create
and -var-list-children, .

-- 
Nick                                           http://www.inet.net.nz/~nickrob


2007-07-09  Nick Roberts  <nickrob@snap.net.nz>

	* varobj.c (c_variable_editable, cplus_variable_editable)
	(java_variable_editable, variable_editable): Delete.
	(variable_editable_p): Replace above functions with one language
	independent function.  Check for an lvalue.  Derive from
	varobj_value_is_changeable_p.
	(varobj_get_attributes, varobj_set_value): Use variable_editable_p.
	(struct language_specific): Delete variable_editable field.

*** varobj.c	14 Apr 2007 21:51:29 +1200	1.89
--- varobj.c	04 Jul 2007 17:45:21 +1200	
*************** static struct value *value_of_root (stru
*** 217,229 ****
  
  static struct value *value_of_child (struct varobj *parent, int index);
  
- static int variable_editable (struct varobj *var);
- 
  static char *my_value_of_variable (struct varobj *var);
  
  static char *value_get_print_value (struct value *value,
  				    enum varobj_display_formats format);
  
  static int varobj_value_is_changeable_p (struct varobj *var);
  
  static int is_root_p (struct varobj *var);
--- 217,229 ----
  
  static struct value *value_of_child (struct varobj *parent, int index);
  
  static char *my_value_of_variable (struct varobj *var);
  
  static char *value_get_print_value (struct value *value,
  				    enum varobj_display_formats format);
  
+ static int variable_editable_p (struct varobj *var);
+ 
  static int varobj_value_is_changeable_p (struct varobj *var);
  
  static int is_root_p (struct varobj *var);
*************** static struct value *c_value_of_child (s
*** 242,249 ****
  
  static struct type *c_type_of_child (struct varobj *parent, int index);
  
- static int c_variable_editable (struct varobj *var);
- 
  static char *c_value_of_variable (struct varobj *var);
  
  /* C++ implementation */
--- 242,247 ----
*************** static struct value *cplus_value_of_chil
*** 262,269 ****
  
  static struct type *cplus_type_of_child (struct varobj *parent, int index);
  
- static int cplus_variable_editable (struct varobj *var);
- 
  static char *cplus_value_of_variable (struct varobj *var);
  
  /* Java implementation */
--- 260,265 ----
*************** static struct value *java_value_of_child
*** 280,287 ****
  
  static struct type *java_type_of_child (struct varobj *parent, int index);
  
- static int java_variable_editable (struct varobj *var);
- 
  static char *java_value_of_variable (struct varobj *var);
  
  /* The language specific vector */
--- 276,281 ----
*************** struct language_specific
*** 310,318 ****
    /* The type of the INDEX'th child of PARENT. */
    struct type *(*type_of_child) (struct varobj * parent, int index);
  
-   /* Is VAR editable? */
-   int (*variable_editable) (struct varobj * var);
- 
    /* The current value of VAR. */
    char *(*value_of_variable) (struct varobj * var);
  };
--- 304,309 ----
*************** static struct language_specific language
*** 328,334 ****
     c_value_of_root,
     c_value_of_child,
     c_type_of_child,
-    c_variable_editable,
     c_value_of_variable}
    ,
    /* C */
--- 319,324 ----
*************** static struct language_specific language
*** 340,346 ****
     c_value_of_root,
     c_value_of_child,
     c_type_of_child,
-    c_variable_editable,
     c_value_of_variable}
    ,
    /* C++ */
--- 330,335 ----
*************** static struct language_specific language
*** 352,358 ****
     cplus_value_of_root,
     cplus_value_of_child,
     cplus_type_of_child,
-    cplus_variable_editable,
     cplus_value_of_variable}
    ,
    /* Java */
--- 341,346 ----
*************** static struct language_specific language
*** 364,370 ****
     java_value_of_root,
     java_value_of_child,
     java_type_of_child,
-    java_variable_editable,
     java_value_of_variable}
  };
  
--- 352,357 ----
*************** varobj_get_attributes (struct varobj *va
*** 817,823 ****
  {
    int attributes = 0;
  
!   if (var->root->is_valid && variable_editable (var))
      /* FIXME: define masks for attributes */
      attributes |= 0x00000001;	/* Editable */
  
--- 804,810 ----
  {
    int attributes = 0;
  
!   if (var->root->is_valid && variable_editable_p (var))
      /* FIXME: define masks for attributes */
      attributes |= 0x00000001;	/* Editable */
  
*************** varobj_set_value (struct varobj *var, ch
*** 848,854 ****
    struct value *value;
    int saved_input_radix = input_radix;
  
!   if (var->value != NULL && variable_editable (var))
      {
        char *s = expression;
        int i;
--- 835,841 ----
    struct value *value;
    int saved_input_radix = input_radix;
  
!   if (var->value != NULL && variable_editable_p (var))
      {
        char *s = expression;
        int i;
*************** value_of_child (struct varobj *parent, i
*** 1760,1773 ****
    return value;
  }
  
- /* Is this variable editable? Use the variable's type to make
-    this determination. */
- static int
- variable_editable (struct varobj *var)
- {
-   return (*var->root->lang->variable_editable) (var);
- }
- 
  /* GDB already has a command called "value_of_variable". Sigh. */
  static char *
  my_value_of_variable (struct varobj *var)
--- 1747,1752 ----
*************** value_get_print_value (struct value *val
*** 1799,1804 ****
--- 1778,1808 ----
    return thevalue;
  }
  
+ 
+ static int
+ variable_editable_pv (struct varobj *var)
+ {
+   struct expression *exp;
+   struct value *value;
+ 
+   if (CPLUS_FAKE_CHILD (var))
+     return 0;
+ 
+   if (is_root_p (var))
+     {
+       if (!gdb_evaluate_expression (var->root->exp, &value))
+ 	{
+ 	  /* We cannot proceed without a valid expression. */
+ 	  xfree (exp);
+ 	  return 0;
+ 	}
+       if (!VALUE_LVAL(value))
+ 	return 0;
+     }
+ 
+   return varobj_value_is_changeable_p (var);
+ }
+ 
  /* Return non-zero if changes in value of VAR
     must be detected and reported by -var-update.
     Return zero is -var-update should never report
*************** value_get_print_value (struct value *val
*** 1811,1817 ****
  static int
  varobj_value_is_changeable_p (struct varobj *var)
  {
-   int r;
    struct type *type;
  
    if (CPLUS_FAKE_CHILD (var))
--- 1815,1820 ----
*************** varobj_value_is_changeable_p (struct var
*** 1819,1837 ****
  
    type = get_value_type (var);
  
    switch (TYPE_CODE (type))
      {
      case TYPE_CODE_STRUCT:
      case TYPE_CODE_UNION:
      case TYPE_CODE_ARRAY:
!       r = 0;
        break;
  
      default:
!       r = 1;
      }
- 
-   return r;
  }
  
  /* Given the value and the type of a variable object,
--- 1822,1842 ----
  
    type = get_value_type (var);
  
+ 
    switch (TYPE_CODE (type))
      {
      case TYPE_CODE_STRUCT:
      case TYPE_CODE_UNION:
      case TYPE_CODE_ARRAY:
!     case TYPE_CODE_FUNC:
!     case TYPE_CODE_METHOD:
!       return 0;
        break;
  
      default:
!       return 1;
!       break;
      }
  }
  
  /* Given the value and the type of a variable object,
*************** c_type_of_child (struct varobj *parent, 
*** 2128,2152 ****
    return type;
  }
  
- static int
- c_variable_editable (struct varobj *var)
- {
-   switch (TYPE_CODE (get_value_type (var)))
-     {
-     case TYPE_CODE_STRUCT:
-     case TYPE_CODE_UNION:
-     case TYPE_CODE_ARRAY:
-     case TYPE_CODE_FUNC:
-     case TYPE_CODE_METHOD:
-       return 0;
-       break;
- 
-     default:
-       return 1;
-       break;
-     }
- }
- 
  static char *
  c_value_of_variable (struct varobj *var)
  {
--- 2133,2138 ----
*************** cplus_type_of_child (struct varobj *pare
*** 2477,2491 ****
    return type;
  }
  
- static int
- cplus_variable_editable (struct varobj *var)
- {
-   if (CPLUS_FAKE_CHILD (var))
-     return 0;
- 
-   return c_variable_editable (var);
- }
- 
  static char *
  cplus_value_of_variable (struct varobj *var)
  {
--- 2463,2468 ----
*************** java_type_of_child (struct varobj *paren
*** 2563,2574 ****
    return cplus_type_of_child (parent, index);
  }
  
- static int
- java_variable_editable (struct varobj *var)
- {
-   return cplus_variable_editable (var);
- }
- 
  static char *
  java_value_of_variable (struct varobj *var)
  {
--- 2540,2545 ----

Re: [MI] lvalues and variable_editable

[Daniel Jacobowitz]

On Mon, Jul 09, 2007 at 05:51:42PM +1200, Nick Roberts wrote:
> (Me)
> 
>  > Anyway let me submit a more complete patch, in due course, for consideration
>  > after the release.
> 
> This is what I have in mind.  There are no regressions, at least with MI (I
> can't test Insight).  If there's still time, this could go in before the
> branch.  I also have a (much smaller) change, that I've submitted earlier, for
> after the release which adds the editable field to the output of -var-create
> and -var-list-children, .

Thanks for doing this.  I have two questions for you.

- Why do variable_editable_p and varobj_value_is_changeable_p have to
be different?  That is, do we need varobj_value_is_changeable_p to be
true for any non-lvals.  If not, we can eliminate one of them.

- Why do you need to re-evaluate the expression?  I think we can use
var->value, and report anything with a NULL value as non-editable.
No point editing it if we can't save it somewhere.

Also, I think varobj_value_is_changeable_p was missing from your
changelog (if I've correctly understood where one hunk of that patch
goes), and the patch had "variable_editable_pv" in it.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI] lvalues and variable_editable

[Nick Roberts]

 > - Why do variable_editable_p and varobj_value_is_changeable_p have to
 > be different?  That is, do we need varobj_value_is_changeable_p to be
 > true for any non-lvals.  If not, we can eliminate one of them.

Taking my earlier example:

-var-create - * "i1 + i2"

As it's not an lvalue it's not editable (variable_editable_p returns 0)
but it's value can surely change (variable_changeable_p returns 1).  So GDB
has to check for this and report any change when -var-update is issued.

While

int m[10]

-var-create - * m

is not editable and not changeable.

Does this show that variable_editable_p and varobj_value_is_changeable_p
are necessarily different?  (One is used to tell the user he can't edit the
value, the other to tell GDB not to bother check for a change in value.)

 > - Why do you need to re-evaluate the expression?  I think we can use
 > var->value, and report anything with a NULL value as non-editable.
 > No point editing it if we can't save it somewhere.

Perhaps you don't.  Currently you can do things like:

(gdb) 
-var-create - * 1/0
^done,name="var1",numchild="0",value="",type="int"
(gdb) 
-var-create - * 0/0
^done,name="var2",numchild="0",value="",type="int"

I was just being cautious, and cut and pasting.

 > Also, I think varobj_value_is_changeable_p was missing from your
 > changelog (if I've correctly understood where one hunk of that patch
 > goes),

OK.

 >        and the patch had "variable_editable_pv" in it.

Sticky fingers, sorry about that.  My version of varobj.c compiles and doesn't
have this, of course.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [MI] lvalues and variable_editable

[Vladimir Prus]

Daniel Jacobowitz wrote:

> On Mon, Jul 09, 2007 at 05:51:42PM +1200, Nick Roberts wrote:
>> (Me)
>> 
>>  > Anyway let me submit a more complete patch, in due course, for
>>  > consideration after the release.
>> 
>> This is what I have in mind.  There are no regressions, at least with MI
>> (I
>> can't test Insight).  If there's still time, this could go in before the
>> branch.  I also have a (much smaller) change, that I've submitted
>> earlier, for after the release which adds the editable field to the
>> output of -var-create and -var-list-children, .
> 
> Thanks for doing this.  I have two questions for you.
> 
> - Why do variable_editable_p and varobj_value_is_changeable_p have to
> be different?  That is, do we need varobj_value_is_changeable_p to be
> true for any non-lvals.  If not, we can eliminate one of them.

I would expect being able to create varobj for "a + b" and have it
updated as I step, and being not editable. So variable_editable_p will
be false, whereas varobj_value_is_changeable_p will be true.

- Volodya

Re: [MI] lvalues and variable_editable

[Vladimir Prus]

Nick Roberts wrote:

> --- 1747,1752 ----
> *************** value_get_print_value (struct value *val
> *** 1799,1804 ****
> --- 1778,1808 ----
> return thevalue;
> }
> 
> +
> + static int
> + variable_editable_pv (struct varobj *var)

Can we probably use "varobj_editable_pv". Using
varobj in some places, and "variable" in others
makes for confusing code.

> + {
> +   struct expression *exp;
> +   struct value *value;
> +
> +   if (CPLUS_FAKE_CHILD (var))
> +     return 0;
> +
> +   if (is_root_p (var))
> +     {
> +       if (!gdb_evaluate_expression (var->root->exp, &value))
> +       {
> +         /* We cannot proceed without a valid expression. */
> +         xfree (exp);
> +         return 0;
> +       }

Why do you evaluate this? The expression should already be
evaluated when creating varobj. So, using var->value should
be sufficient.

Also, why is_root_p check? It is possible to create varobj for
an expression the creates rvalue of structure type. The children of 
such varobj won't be lvalues, and won't be editable, but this code
won't catch this case.


> *************** varobj_value_is_changeable_p (struct var
> *** 1819,1837 ****
> --- 1822,1842 ----
> 
> type = get_value_type (var);
> 
> +
> switch (TYPE_CODE (type))
> {
> case TYPE_CODE_STRUCT:
> case TYPE_CODE_UNION:
> case TYPE_CODE_ARRAY:
> !     case TYPE_CODE_FUNC:
> !     case TYPE_CODE_METHOD:
> !       return 0;

In current gdb, assuming this declaration:

        void (*fp)();

I can create varobj for *fp:

        -var-create V * *fp

and V will be updated if fp changes. With your patch,
I get this:

        -var-create V * *fp
        ~"varobj.c:2180: internal-error: c_value_of_variable: Assertion `varobj_value_is_changeable_p (var)' failed.\n"
        ~"A problem internal to GDB has been detected,\n"
        ~"further debugging may prove unreliable.\n"
        ~"Quit this debugging session? (y or n) "


So, probably TYPE_CODE_FUNC should be handled in variable_editable_p.
I'm not sure about TYPE_CODE_METHOD -- I don't know how to construct
an object of that type using any possible expression.

- Volodya

Re: [MI] lvalues and variable_editable

[Nick Roberts]

 > > + static int
 > > + variable_editable_pv (struct varobj *var)
 > 
 > Can we probably use "varobj_editable_pv". Using
 > varobj in some places, and "variable" in others
 > makes for confusing code.

(I meant variable_editable_p.)

It looks like you renamed type_changeable to varobj_value_is_changeable_p
following Apple's version.  I agree that there should be some consistency

variable_editable_p,  variable_changeable_p?

var_editable_p, var_changeable_p?

or, probably better

varobj_editable_p,  varobj_changeable_p?

 > > + {
 > > +   struct expression *exp;
 > > +   struct value *value;
 > > +
 > > +   if (CPLUS_FAKE_CHILD (var))
 > > +     return 0;
 > > +
 > > +   if (is_root_p (var))
 > > +     {
 > > +       if (!gdb_evaluate_expression (var->root->exp, &value))
 > > +       {
 > > +         /* We cannot proceed without a valid expression. */
 > > +         xfree (exp);
 > > +         return 0;
 > > +       }
 > 
 > Why do you evaluate this? The expression should already be
 > evaluated when creating varobj. So, using var->value should
 > be sufficient.

I've already replied to Daniel on this question.

 > Also, why is_root_p check? It is possible to create varobj for
 > an expression the creates rvalue of structure type. The children of 
 > such varobj won't be lvalues, and won't be editable, but this code
 > won't catch this case.

I'm not sure what you mean by "won't catch this case" but this is in
variable_editable_p which is called by varobj_set_value.  If the user
tries to assign a value to a child this check means GDB won't need to
test if it's not an lvalue.

 > 
 > > *************** varobj_value_is_changeable_p (struct var
 > > *** 1819,1837 ****
 > > --- 1822,1842 ----
 > > 
 > > type = get_value_type (var);
 > > 
 > > +
 > > switch (TYPE_CODE (type))
 > > {
 > > case TYPE_CODE_STRUCT:
 > > case TYPE_CODE_UNION:
 > > case TYPE_CODE_ARRAY:
 > > !     case TYPE_CODE_FUNC:
 > > !     case TYPE_CODE_METHOD:
 > > !       return 0;
 > 
 > In current gdb, assuming this declaration:
 > 
 >         void (*fp)();
 > 
 > I can create varobj for *fp:
 > 
 >         -var-create V * *fp
 > 
 > and V will be updated if fp changes. With your patch,
 > I get this:
 > 
 >         -var-create V * *fp
 >         ~"varobj.c:2180: internal-error: c_value_of_variable: Assertion `varobj_value_is_changeable_p (var)' failed.\n"
 >         ~"A problem internal to GDB has been detected,\n"
 >         ~"further debugging may prove unreliable.\n"
 >         ~"Quit this debugging session? (y or n) "

OK.  I had just thought about fp being TYPE_CODE_PTR.

 > So, probably TYPE_CODE_FUNC should be handled in variable_editable_p.
 > I'm not sure about TYPE_CODE_METHOD -- I don't know how to construct
 > an object of that type using any possible expression.

That's where they came from.  OK, I'll investigate.  It occurs to me that you
might be create problems with pointers to structs, unions and arrays too.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [MI] lvalues and variable_editable

[Daniel Jacobowitz]

On Tue, Jul 10, 2007 at 12:37:55AM +1200, Nick Roberts wrote:
> Taking my earlier example:
> 
> -var-create - * "i1 + i2"
> 
> As it's not an lvalue it's not editable (variable_editable_p returns 0)
> but it's value can surely change (variable_changeable_p returns 1).  So GDB
> has to check for this and report any change when -var-update is issued.

Yeah, I see.  Thanks.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI] lvalues and variable_editable

[Vladimir Prus]

On Tuesday 10 July 2007 04:49, Nick Roberts wrote:
>  > > + static int
>  > > + variable_editable_pv (struct varobj *var)
>  > 
>  > Can we probably use "varobj_editable_pv". Using
>  > varobj in some places, and "variable" in others
>  > makes for confusing code.
> 
> (I meant variable_editable_p.)
> 
> It looks like you renamed type_changeable to varobj_value_is_changeable_p
> following Apple's version.  I agree that there should be some consistency
> 
> variable_editable_p,  variable_changeable_p?
> 
> var_editable_p, var_changeable_p?
> 
> or, probably better
> 
> varobj_editable_p,  varobj_changeable_p?

This last seems best to me. I think that the "varobj_value_changeable"
is more accurate than varobj_changeable, because it specifically means
that var->value never changes, but that's probably not important.

>  > Also, why is_root_p check? It is possible to create varobj for
>  > an expression the creates rvalue of structure type. The children of 
>  > such varobj won't be lvalues, and won't be editable, but this code
>  > won't catch this case.
> 
> I'm not sure what you mean by "won't catch this case" but this is in
> variable_editable_p which is called by varobj_set_value.  If the user
> tries to assign a value to a child this check means GDB won't need to
> test if it's not an lvalue.

Well, ideally if I have a varobj for rvalue structure, I want the children
of such varobj to be reported as non-editable. It does not seem to happen,
IIUC.

>  > > *************** varobj_value_is_changeable_p (struct var
>  > > *** 1819,1837 ****
>  > > --- 1822,1842 ----
>  > > 
>  > > type = get_value_type (var);
>  > > 
>  > > +
>  > > switch (TYPE_CODE (type))
>  > > {
>  > > case TYPE_CODE_STRUCT:
>  > > case TYPE_CODE_UNION:
>  > > case TYPE_CODE_ARRAY:
>  > > !     case TYPE_CODE_FUNC:
>  > > !     case TYPE_CODE_METHOD:
>  > > !       return 0;
>  > 
>  > In current gdb, assuming this declaration:
>  > 
>  >         void (*fp)();
>  > 
>  > I can create varobj for *fp:
>  > 
>  >         -var-create V * *fp
>  > 
>  > and V will be updated if fp changes. With your patch,
>  > I get this:
>  > 
>  >         -var-create V * *fp
>  >         ~"varobj.c:2180: internal-error: c_value_of_variable: Assertion `varobj_value_is_changeable_p (var)' failed.\n"
>  >         ~"A problem internal to GDB has been detected,\n"
>  >         ~"further debugging may prove unreliable.\n"
>  >         ~"Quit this debugging session? (y or n) "
> 
> OK.  I had just thought about fp being TYPE_CODE_PTR.
> 
>  > So, probably TYPE_CODE_FUNC should be handled in variable_editable_p.
>  > I'm not sure about TYPE_CODE_METHOD -- I don't know how to construct
>  > an object of that type using any possible expression.
> 
> That's where they came from.  OK, I'll investigate.  It occurs to me that you
> might be create problems with pointers to structs, unions and arrays too.

We should not have any problems, because we never try to get varobj->value
for object of struct, union or array type.

- Volodya



> 

Re: [MI] lvalues and variable_editable

[Nick Roberts]

 > > varobj_editable_p,  varobj_changeable_p?
 > 
 > This last seems best to me. I think that the "varobj_value_changeable"
 > is more accurate than varobj_changeable, because it specifically means
 > that var->value never changes, but that's probably not important.

OK I'll do this for the patch in question.

 > >  > Also, why is_root_p check? It is possible to create varobj for
 > >  > an expression the creates rvalue of structure type. The children of 
 > >  > such varobj won't be lvalues, and won't be editable, but this code
 > >  > won't catch this case.
 > > 
 > > I'm not sure what you mean by "won't catch this case" but this is in
 > > variable_editable_p which is called by varobj_set_value.  If the user
 > > tries to assign a value to a child this check means GDB won't need to
 > > test if it's not an lvalue.
 > 
 > Well, ideally if I have a varobj for rvalue structure, I want the children
 > of such varobj to be reported as non-editable. It does not seem to happen,
 > IIUC.

Remember that an rvalue may be an lvalue also.  I think you are saying a varobj
with children which aren't lvalues.  How would you create such a varobj?  Note:

int m[10];

(gdb) 
-var-create - * m
^done,name="var1",numchild="10",type="int [10]"
(gdb) 
-var-create - * (float)m
^done,name="var2",numchild="0",type="float"
(gdb) 
-var-create - * 2*m
&"Argument to arithmetic operation not a number or boolean.\n"
^error,msg="Argument to arithmetic operation not a number or boolean."


 > >  > > *************** varobj_value_is_changeable_p (struct var
 > >  > > *** 1819,1837 ****
 > >  > > --- 1822,1842 ----
 > >  > > 
 > >  > > type = get_value_type (var);
 > >  > > 
 > >  > > +
 > >  > > switch (TYPE_CODE (type))
 > >  > > {
 > >  > > case TYPE_CODE_STRUCT:
 > >  > > case TYPE_CODE_UNION:
 > >  > > case TYPE_CODE_ARRAY:
 > >  > > !     case TYPE_CODE_FUNC:
 > >  > > !     case TYPE_CODE_METHOD:
 > >  > > !       return 0;
 > >  > 
 > >  > In current gdb, assuming this declaration:
 > >  > 
 > >  >         void (*fp)();
 > >  > 
 > >  > I can create varobj for *fp:
 > >  > 
 > >  >         -var-create V * *fp
 > >  > 
 > >  > and V will be updated if fp changes. With your patch,
 > >  > I get this:
 > >  > 
 > >  >         -var-create V * *fp
 > >  >         ~"varobj.c:2180: internal-error: c_value_of_variable: Assertion `varobj_value_is_changeable_p (var)' failed.\n"
 > >  >         ~"A problem internal to GDB has been detected,\n"
 > >  >         ~"further debugging may prove unreliable.\n"
 > >  >         ~"Quit this debugging session? (y or n) "
 > > 
 > > OK.  I had just thought about fp being TYPE_CODE_PTR.
 > > 
 > >  > So, probably TYPE_CODE_FUNC should be handled in variable_editable_p.
 > >  > I'm not sure about TYPE_CODE_METHOD -- I don't know how to construct
 > >  > an object of that type using any possible expression.
 > > 
 > > That's where they came from.  OK, I'll investigate.  It occurs to me that you
 > > might be create problems with pointers to structs, unions and arrays too.
 > 
 > We should not have any problems, because we never try to get varobj->value
 > for object of struct, union or array type.

Actually now I've looked at it, I think the problem is in c_value_of_variable.
I think there should be an extra clause, something like:


  switch (TYPE_CODE (type))
    {
+    case TYPE_CODE_FUNCTION:
+      return xstrdup ("<function>");
+      /* break; */

and a similar one for TYPE_CODE_METHOD, either here or in
cplus_value_of_variable.  Functions and methods are surely not changeable.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [MI] lvalues and variable_editable

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 5636 bytes --]

On Wednesday 11 July 2007 05:26, Nick Roberts wrote:

>  > Well, ideally if I have a varobj for rvalue structure, I want the children
>  > of such varobj to be reported as non-editable. It does not seem to happen,
>  > IIUC.
> 
> Remember that an rvalue may be an lvalue also.  

I'm not sure what you mean. Technically, lvalue can be converted to
rvalue (per section 4.1 of the C++ standard). As a practical matter,
however, each expression is either assignable-to, or non-assignable


> I think you are saying a varobj 
> with children which aren't lvalues.  How would you create such a varobj?  Note:

Consider the attached program. Then consider this session:

	(gdb)
	b main
	........
	(gdb)
	r
	.........	
	(gdb)
	-var-create F * return_foo()
	^done,name="F",numchild="1",type="foo"
	(gdb)
	-var-list-children F
	^done,numchild="1",children=[child={name="F.public",exp="public",numchild="2"}]
	(gdb)
	-var-list-children F.public
	^done,numchild="2",children=[child={name="F.public.i",exp="i",numchild="0",type="int"},child={name="F.public.j",exp="j",numchild="0",type="int"}]
	(gdb)
	-var-assign F.public.i 10
	&"mi_cmd_var_assign: Could not assign expression to varible object\n"
	^error,msg="mi_cmd_var_assign: Could not assign expression to varible object"
	(gdb)

Clearly, gdb does not like assigning to F.public.i, and I'm pretty sure it's because VALUE_LVAL for
that varobj returns false. Naturally, it's reasonable to expect to have F.public.i marked as non-editable,
so that frontend won't even let the user to assign a value. I don't think your patch will do that.

It should be noted that update of F, above, is broken, but it's broken in an unrelated way.

>  > >  > > *************** varobj_value_is_changeable_p (struct var
>  > >  > > *** 1819,1837 ****
>  > >  > > --- 1822,1842 ----
>  > >  > > 
>  > >  > > type = get_value_type (var);
>  > >  > > 
>  > >  > > +
>  > >  > > switch (TYPE_CODE (type))
>  > >  > > {
>  > >  > > case TYPE_CODE_STRUCT:
>  > >  > > case TYPE_CODE_UNION:
>  > >  > > case TYPE_CODE_ARRAY:
>  > >  > > !     case TYPE_CODE_FUNC:
>  > >  > > !     case TYPE_CODE_METHOD:
>  > >  > > !       return 0;
>  > >  > 
>  > >  > In current gdb, assuming this declaration:
>  > >  > 
>  > >  >         void (*fp)();
>  > >  > 
>  > >  > I can create varobj for *fp:
>  > >  > 
>  > >  >         -var-create V * *fp
>  > >  > 
>  > >  > and V will be updated if fp changes. With your patch,
>  > >  > I get this:
>  > >  > 
>  > >  >         -var-create V * *fp
>  > >  >         ~"varobj.c:2180: internal-error: c_value_of_variable: Assertion `varobj_value_is_changeable_p (var)' failed.\n"
>  > >  >         ~"A problem internal to GDB has been detected,\n"
>  > >  >         ~"further debugging may prove unreliable.\n"
>  > >  >         ~"Quit this debugging session? (y or n) "
>  > > 
>  > > OK.  I had just thought about fp being TYPE_CODE_PTR.
>  > > 
>  > >  > So, probably TYPE_CODE_FUNC should be handled in variable_editable_p.
>  > >  > I'm not sure about TYPE_CODE_METHOD -- I don't know how to construct
>  > >  > an object of that type using any possible expression.
>  > > 
>  > > That's where they came from.  OK, I'll investigate.  It occurs to me that you
>  > > might be create problems with pointers to structs, unions and arrays too.
>  > 
>  > We should not have any problems, because we never try to get varobj->value
>  > for object of struct, union or array type.
> 
> Actually now I've looked at it, I think the problem is in c_value_of_variable.
> I think there should be an extra clause, something like:
> 
> 
>   switch (TYPE_CODE (type))
>     {
> +    case TYPE_CODE_FUNCTION:
> +      return xstrdup ("<function>");
> +      /* break; */
> 
> and a similar one for TYPE_CODE_METHOD, either here or in
> cplus_value_of_variable.  

Such a change will be regression from the current behaviour. Consider the attached program
and the following session:

	(gdb)
	b main
	.....
	(gdb)
	r
	....
	(gdb)
	n
	&"n\n"
	~"8\t    fp = bar;\n"
	^done
	(gdb)
	-var-create F * *fp
	^done,name="F",numchild="0",type="void (void)"
	(gdb)
	-var-evaluate-expression F
	^done,value="{void (void)} 0x80483c4 <foo()>"
	(gdb)
	next
	&"next\n"
	~"9\t    return 0;\n"
	^done
	(gdb)
	-var-update F
	^done,changelist=[{name="F",in_scope="true",type_changed="false"}]
	(gdb)
	-var-evaluate-expression F
	^done,value="{void (void)} 0x80483ca <bar()>"
	(gdb)

Note that the current gdb has no problem whatsoever with printing the value
of function, and it also notices when a value changes. If you change c_value_of_variable
as outlined above, you'll only see "<function>" as output.

> Functions and methods are surely not changeable. 

I think you misunderstand the meaning of varobj_value_is_changeable_p. It does not
indicate if the object itself may be changed, by the programming language or by
gdb user. It indicates if the value of varobj, as printed by -var-evaluate-expression,
may change. As shown above, in current gdb, the value of varobj having type 'function'
can change just fine, in a meaningful way.

If you make gdb print "<function>" for objects of function type, then clearly the
value cannot change, and varobj_value_is_changrable_p can return true for
functions. However, I don't see any reason why printing "<function>" for
object of function type would be better than the current situation. I further suspect
that "*f" above will be rvalue, so it will be reported as non-editable by your patch anyway.

- Volodya

[-- Attachment #2: rvalue.cpp --]
[-- Type: text/x-c++src, Size: 184 bytes --]


int i = 0;

struct foo { int i; int j; };

foo return_foo ()
{
    struct foo f = { i, i+1};
    return f;
}


int main()
{
    i = 1;
    i = 2;
    i = 3;
    i = 4;
    return 0;
}

[-- Attachment #3: fp.cpp --]
[-- Type: text/x-c++src, Size: 96 bytes --]


void foo() {}
void bar() {}

int main()
{
    void (*fp)() = foo;
    fp = bar;
    return 0;
}

Re: [MI] lvalues and variable_editable

[Vladimir Prus]

Vladimir Prus wrote:

> Clearly, gdb does not like assigning to F.public.i, and I'm pretty sure
> it's because VALUE_LVAL for that varobj returns false. Naturally, it's
> reasonable to expect to have F.public.i marked as non-editable, so that
> frontend won't even let the user to assign a value. I don't think your
> patch will do that.
> 
> It should be noted that update of F, above, is broken, but it's broken in
> an unrelated way.

To clarify, I was able to get gdb to crash, but it appears that
requires creating variable object before actually starting the
program, and cannot reproduce the crash at will. Regular -var-update during
stepping works fine.

- Volodya

Re: [MI] lvalues and variable_editable

[Nick Roberts]

 > 	(gdb)
 > 	-var-assign F.public.i 10
 > 	&"mi_cmd_var_assign: Could not assign expression to varible object\n"
 > 	^error,msg="mi_cmd_var_assign: Could not assign expression to varible object"
 > 	(gdb)
 > 
 > Clearly, gdb does not like assigning to F.public.i, and I'm pretty sure it's
 > because VALUE_LVAL for that varobj returns false. Naturally, it's reasonable
 > to expect to have F.public.i marked as non-editable, so that frontend won't
 > even let the user to assign a value. I don't think your patch will do that.

OK.  It would be strange to try to edit such variable objects, but I take
your point and will change it (back to it's original set of conditions).

 > 	-var-update F
 > 	^done,changelist=[{name="F",in_scope="true",type_changed="false"}]
 > 	(gdb)
 > 	-var-evaluate-expression F
 > 	^done,value="{void (void)} 0x80483ca <bar()>"
 > 	(gdb)
 > 
 > Note that the current gdb has no problem whatsoever with printing the value
 > of function, and it also notices when a value changes. If you change c_value_of_variable
 > as outlined above, you'll only see "<function>" as output.
 > 
 > > Functions and methods are surely not changeable. 
 > 
 > I think you misunderstand the meaning of varobj_value_is_changeable_p. It
 > does not indicate if the object itself may be changed, by the programming
 > language or by gdb user. It indicates if the value of varobj, as printed by
 > -var-evaluate-expression, may change. As shown above, in current gdb, the
 > value of varobj having type 'function' can change just fine, in a meaningful
 > way.

It shows I misunderstood the concept of the value of a function.  I'll
change that also, which means varobj_editable_p can't be easily derived
from varobj_changeable_p.


-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [MI] lvalues and variable_editable

[Vladimir Prus]

On Wednesday 11 July 2007 15:57, Nick Roberts wrote:

>  > I think you misunderstand the meaning of varobj_value_is_changeable_p. It
>  > does not indicate if the object itself may be changed, by the programming
>  > language or by gdb user. It indicates if the value of varobj, as printed by
>  > -var-evaluate-expression, may change. As shown above, in current gdb, the
>  > value of varobj having type 'function' can change just fine, in a meaningful
>  > way.
> 
> It shows I misunderstood the concept of the value of a function.  I'll
> change that also, which means varobj_editable_p can't be easily derived
> from varobj_changeable_p.

Well, current varobj_changeable_p answers the question "do we need to store
a non-lazy value inside varobj, so that after -var-update we can detect that
the value has changed". It follows that -var-evaluate-expression should print
some static string for such varobjs.

As for editability, two approaches are possible:

1. If static string is printed as value, it makes no sense to assign anything.
In that case, using varobj_changeable_p by varobj_editable_p is reasonable.

2. Why not allow assigning "{1,2}" to a varobj of expression type? That might
be good feature to user, but I'm not sure how hard it will be to implement.

So for now I think we can take (1), and then use varobj_changeable_p from
varobj_editable_p. Should that become problematic, we can always make 
the functions completely independent.

- Volodya