Re: [MI] lvalues and variable_editable

[Nick Roberts <nickrob at snap dot net dot nz>]

 > > + 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