Re: [PATCH] MI: lvalues and variable_editable

[Nick Roberts <nickrob@snap.net.nz>]

 > > +   if (!(var->root->is_valid && var->value && VALUE_LVAL(var->value)))
 > > +     return 0;
 > 
 > Space before parenthesis.
 > >     /* FIXME: define masks for attributes */
 > > !   if (!varobj_editable_p (var))
 > >       error (_("mi_cmd_var_assign: Variable object is not editable"));
 > > +   else
 > > +     {
 > > +       expression = xstrdup (argv[1]);
 > 
 > No need to move all this code into an else; error will never return.
 > 
 > The patch seems fine to me, with Vladimir's comments.

OK.  Below are the changes that I committed.

Thanks,

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


2007-11-21  Nick Roberts  <nickrob@snap.net.nz>

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

	* mi-cmd-var.c (mi_cmd_var_assign): Simplify.

	* varobj.h: Add extern for varobj_editable_p.


*** varobj.c.~1.97~	2007-11-12 19:44:40.000000000 +1300
--- varobj.c	2007-11-21 08:17:05.000000000 +1300
*************** static struct value *value_of_root (stru
*** 221,228 ****
  
  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,
--- 221,226 ----
*************** static struct value *c_value_of_child (s
*** 248,255 ****
  
  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 */
--- 246,251 ----
*************** static struct value *cplus_value_of_chil
*** 270,277 ****
  
  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 */
--- 266,271 ----
*************** static struct value *java_value_of_child
*** 290,297 ****
  
  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 */
--- 284,289 ----
*************** struct language_specific
*** 324,332 ****
    /* 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);
  };
--- 316,321 ----
*************** static struct language_specific language
*** 343,349 ****
     c_value_of_root,
     c_value_of_child,
     c_type_of_child,
-    c_variable_editable,
     c_value_of_variable}
    ,
    /* C */
--- 332,337 ----
*************** static struct language_specific language
*** 356,362 ****
     c_value_of_root,
     c_value_of_child,
     c_type_of_child,
-    c_variable_editable,
     c_value_of_variable}
    ,
    /* C++ */
--- 344,349 ----
*************** static struct language_specific language
*** 369,375 ****
     cplus_value_of_root,
     cplus_value_of_child,
     cplus_type_of_child,
-    cplus_variable_editable,
     cplus_value_of_variable}
    ,
    /* Java */
--- 356,361 ----
*************** static struct language_specific language
*** 382,388 ****
     java_value_of_root,
     java_value_of_child,
     java_type_of_child,
-    java_variable_editable,
     java_value_of_variable}
  };
  
--- 368,373 ----
*************** varobj_create (char *objname,
*** 523,529 ****
  	  select_frame (fi);
  	}
  
!       /* We definitively need to catch errors here.
           If evaluate_expression succeeds we got the value we wanted.
           But if it fails, we still go on with a call to evaluate_type()  */
        if (!gdb_evaluate_expression (var->root->exp, &value))
--- 508,514 ----
  	  select_frame (fi);
  	}
  
!       /* We definitely need to catch errors here.
           If evaluate_expression succeeds we got the value we wanted.
           But if it fails, we still go on with a call to evaluate_type()  */
        if (!gdb_evaluate_expression (var->root->exp, &value))
*************** varobj_get_attributes (struct varobj *va
*** 856,862 ****
  {
    int attributes = 0;
  
!   if (var->root->is_valid && variable_editable (var))
      /* FIXME: define masks for attributes */
      attributes |= 0x00000001;	/* Editable */
  
--- 841,847 ----
  {
    int attributes = 0;
  
!   if (varobj_editable_p (var))
      /* FIXME: define masks for attributes */
      attributes |= 0x00000001;	/* Editable */
  
*************** varobj_set_value (struct varobj *var, ch
*** 886,940 ****
    struct expression *exp;
    struct value *value;
    int saved_input_radix = input_radix;
  
!   if (var->value != NULL && variable_editable (var))
!     {
!       char *s = expression;
!       int i;
  
!       input_radix = 10;		/* ALWAYS reset to decimal temporarily */
!       exp = parse_exp_1 (&s, 0, 0);
!       if (!gdb_evaluate_expression (exp, &value))
! 	{
! 	  /* We cannot proceed without a valid expression. */
! 	  xfree (exp);
! 	  return 0;
! 	}
! 
!       /* All types that are editable must also be changeable.  */
!       gdb_assert (varobj_value_is_changeable_p (var));
! 
!       /* The value of a changeable variable object must not be lazy.  */
!       gdb_assert (!value_lazy (var->value));
! 
!       /* Need to coerce the input.  We want to check if the
! 	 value of the variable object will be different
! 	 after assignment, and the first thing value_assign
! 	 does is coerce the input.
! 	 For example, if we are assigning an array to a pointer variable we
! 	 should compare the pointer with the the array's address, not with the
! 	 array's content.  */
!       value = coerce_array (value);
! 
!       /* The new value may be lazy.  gdb_value_assign, or 
! 	 rather value_contents, will take care of this.
! 	 If fetching of the new value will fail, gdb_value_assign
! 	 with catch the exception.  */
!       if (!gdb_value_assign (var->value, value, &val))
! 	return 0;
!      
!       /* If the value has changed, record it, so that next -var-update can
! 	 report this change.  If a variable had a value of '1', we've set it
! 	 to '333' and then set again to '1', when -var-update will report this
! 	 variable as changed -- because the first assignment has set the
! 	 'updated' flag.  There's no need to optimize that, because return value
! 	 of -var-update should be considered an approximation.  */
!       var->updated = install_new_value (var, val, 0 /* Compare values. */);
!       input_radix = saved_input_radix;
!       return 1;
      }
  
!   return 0;
  }
  
  /* Returns a malloc'ed list with all root variable objects */
--- 871,921 ----
    struct expression *exp;
    struct value *value;
    int saved_input_radix = input_radix;
+   char *s = expression;
+   int i;
  
!   gdb_assert (varobj_editable_p (var));
  
!   input_radix = 10;		/* ALWAYS reset to decimal temporarily */
!   exp = parse_exp_1 (&s, 0, 0);
!   if (!gdb_evaluate_expression (exp, &value))
!     {
!       /* We cannot proceed without a valid expression. */
!       xfree (exp);
!       return 0;
      }
  
!   /* All types that are editable must also be changeable.  */
!   gdb_assert (varobj_value_is_changeable_p (var));
! 
!   /* The value of a changeable variable object must not be lazy.  */
!   gdb_assert (!value_lazy (var->value));
! 
!   /* Need to coerce the input.  We want to check if the
!      value of the variable object will be different
!      after assignment, and the first thing value_assign
!      does is coerce the input.
!      For example, if we are assigning an array to a pointer variable we
!      should compare the pointer with the the array's address, not with the
!      array's content.  */
!   value = coerce_array (value);
! 
!   /* The new value may be lazy.  gdb_value_assign, or 
!      rather value_contents, will take care of this.
!      If fetching of the new value will fail, gdb_value_assign
!      with catch the exception.  */
!   if (!gdb_value_assign (var->value, value, &val))
!     return 0;
!      
!   /* If the value has changed, record it, so that next -var-update can
!      report this change.  If a variable had a value of '1', we've set it
!      to '333' and then set again to '1', when -var-update will report this
!      variable as changed -- because the first assignment has set the
!      'updated' flag.  There's no need to optimize that, because return value
!      of -var-update should be considered an approximation.  */
!   var->updated = install_new_value (var, val, 0 /* Compare values. */);
!   input_radix = saved_input_radix;
!   return 1;
  }
  
  /* Returns a malloc'ed list with all root variable objects */
*************** value_of_child (struct varobj *parent, i
*** 1803,1816 ****
    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)
--- 1784,1789 ----
*************** value_get_print_value (struct value *val
*** 1842,1847 ****
--- 1815,1847 ----
    return thevalue;
  }
  
+ int
+ varobj_editable_p (struct varobj *var)
+ {
+   struct type *type;
+   struct value *value;
+ 
+   if (!(var->root->is_valid && var->value && VALUE_LVAL (var->value)))
+     return 0;
+ 
+   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;
+     }
+ }
+ 
  /* 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
*************** c_type_of_child (struct varobj *parent, 
*** 2214,2238 ****
    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)
  {
--- 2214,2219 ----
*************** cplus_type_of_child (struct varobj *pare
*** 2604,2618 ****
    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)
  {
--- 2585,2590 ----
*************** java_type_of_child (struct varobj *paren
*** 2696,2707 ****
    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)
  {


*** mi-cmd-var.c.~1.40~	2007-11-03 22:17:04.000000000 +1300
--- mi-cmd-var.c	2007-11-21 08:19:11.000000000 +1300
*************** mi_cmd_var_assign (char *command, char *
*** 511,518 ****
    if (var == NULL)
      error (_("mi_cmd_var_assign: Variable object not found"));
  
!   /* FIXME: define masks for attributes */
!   if (!(varobj_get_attributes (var) & 0x00000001))
      error (_("mi_cmd_var_assign: Variable object is not editable"));
  
    expression = xstrdup (argv[1]);
--- 511,517 ----
    if (var == NULL)
      error (_("mi_cmd_var_assign: Variable object not found"));
  
!   if (!varobj_editable_p (var))
      error (_("mi_cmd_var_assign: Variable object is not editable"));
  
    expression = xstrdup (argv[1]);


*** varobj.h.~1.12.~	2007-09-01 22:35:10.000000000 +1200
--- varobj.h	2007-11-03 12:13:27.000000000 +1300
*************** extern int varobj_update (struct varobj 
*** 114,117 ****
--- 114,119 ----
  
  extern void varobj_invalidate (void);
  
+ extern int varobj_editable_p (struct varobj *var);
+ 
  #endif /* VAROBJ_H */