From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14048 invoked by alias); 3 Nov 2007 09:23:24 -0000 Received: (qmail 14033 invoked by uid 22791); 3 Nov 2007 09:23:22 -0000 X-Spam-Check-By: sourceware.org Received: from viper.snap.net.nz (HELO viper.snap.net.nz) (202.37.101.8) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 03 Nov 2007 09:23:15 +0000 Received: from kahikatea.snap.net.nz (229.30.255.123.static.snap.net.nz [123.255.30.229]) by viper.snap.net.nz (Postfix) with ESMTP id 919A63DA0D6; Sat, 3 Nov 2007 22:23:05 +1300 (NZDT) Received: by kahikatea.snap.net.nz (Postfix, from userid 1000) id F38188FC6D; Sat, 3 Nov 2007 22:22:57 +1300 (NZDT) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <18220.15856.752225.9086@kahikatea.snap.net.nz> Date: Sat, 03 Nov 2007 09:23:00 -0000 To: Daniel Jacobowitz Cc: Vladimir Prus , gdb-patches@sources.redhat.com Subject: Re: [PATCH] MI: lvalues and variable_editable In-Reply-To: <20071102113610.GA16883@caradoc.them.org> References: <18210.27153.559569.601092@kahikatea.snap.net.nz> <200710311458.58112.ghost@cs.msu.su> <18217.23406.21406.920384@kahikatea.snap.net.nz> <200710312247.40840.ghost@cs.msu.su> <20071101153955.GA20676@caradoc.them.org> <18218.42464.899738.594153@kahikatea.snap.net.nz> <20071102113610.GA16883@caradoc.them.org> X-Mailer: VM 7.19 under Emacs 22.1.50.14 X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-11/txt/msg00026.txt.bz2 > > In that case for such a varobj, var->value == NULL must indicate that the > > memory was inaccessible at the _last_ update, or creation, and it may no > > longer be inaccessible. Describing it as "noneditable" may then be a bit > > misleading. > > The next time it's -var-update'd it may get a new value; presumably > the IDE does that periodically. I guess, in that respect it's no different to -var-evaluate-expression. Here's my latest patch for varobj.c. I've removed the test for var->value != NULL in varobj_set_value as it's already in varobj_editable_p. I've also moved the test for var->root->is_valid from varobj_get_attributes to varobj_editable_p. This patch would require two x two changes to the testsuite: FAIL: gdb.mi/mi2-var-child.exp: is weird.int_ptr_ptr.*int_ptr_ptr editable FAIL: gdb.mi/mi2-var-child.exp: is weird.int_ptr_ptr.*int_ptr_ptr.**int_ptr_ptr editable editable -> noneditable likewise for mi-var-child.exp There's also an independent change to mi_cmd_var_assign in mi-cmd-var.c that I think makes it simpler. -- Nick http://www.inet.net.nz/~nickrob 2007-11-03 Nick Roberts * 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.c 27 Oct 2007 09:51:19 +1300 1.96 --- varobj.c 03 Nov 2007 21:50:19 +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 *** 887,940 **** 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 */ --- 872,920 ---- struct value *value; int saved_input_radix = input_radix; ! 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; } /* Returns a malloc'ed list with all root variable objects */ *************** value_of_child (struct varobj *parent, i *** 1801,1814 **** 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) --- 1781,1786 ---- *************** value_get_print_value (struct value *val *** 1840,1845 **** --- 1812,1847 ---- return thevalue; } + int + varobj_editable_p (struct varobj *var) + { + struct type *type; + struct value *value; + + if (CPLUS_FAKE_CHILD (var)) + return 0; + + 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, *** 2212,2236 **** 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 *** 2602,2616 **** 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 *** 2694,2705 **** 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) { --- 2668,2673 ---- *** mi-cmd-var.c 03 Nov 2007 22:17:04 +1300 1.40 --- mi-cmd-var.c 03 Nov 2007 12:24:24 +1300 *************** mi_cmd_var_assign (char *command, char * *** 512,527 **** 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]); ! ! if (!varobj_set_value (var, expression)) ! error (_("mi_cmd_var_assign: Could not assign expression to variable object")); ! ui_out_field_string (uiout, "value", varobj_get_value (var)); ! return MI_CMD_DONE; } enum mi_cmd_result --- 512,529 ---- error (_("mi_cmd_var_assign: Variable object not found")); /* 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]); ! if (!varobj_set_value (var, expression)) ! error (_("mi_cmd_var_assign: Could not assign expression to variable object")); ! ui_out_field_string (uiout, "value", varobj_get_value (var)); ! return MI_CMD_DONE; ! } } enum mi_cmd_result