From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28922 invoked by alias); 30 Oct 2007 04:39:44 -0000 Received: (qmail 28914 invoked by uid 22791); 30 Oct 2007 04:39:44 -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; Tue, 30 Oct 2007 04:39:40 +0000 Received: from kahikatea.snap.net.nz (164.61.255.123.dynamic.snap.net.nz [123.255.61.164]) by viper.snap.net.nz (Postfix) with ESMTP id 45B063DA02D; Tue, 30 Oct 2007 17:39:36 +1300 (NZDT) Received: by kahikatea.snap.net.nz (Postfix, from userid 1000) id F209C8FC6D; Tue, 30 Oct 2007 17:39:34 +1300 (NZDT) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <18214.46470.57856.84852@kahikatea.snap.net.nz> Date: Tue, 30 Oct 2007 08:55:00 -0000 To: Vladimir Prus Cc: gdb-patches@sources.redhat.com Subject: Re: [PATCH] MI: lvalues and variable_editable In-Reply-To: <200710271615.23579.ghost@cs.msu.su> References: <18210.27153.559569.601092@kahikatea.snap.net.nz> <18211.10636.539778.664602@kahikatea.snap.net.nz> <200710271615.23579.ghost@cs.msu.su> X-Mailer: VM 7.19 under Emacs 23.0.50.38 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-10/txt/msg00790.txt.bz2 > > > 1. In varobj_editable_p you call gdb_evaluate_expression, and I believe > > > this to be wrong. We call gdb_evaluate_expression when we create > > > varobj, and it either succeeds, eventually setting varobj->value to > > > something, or it does not. There's no point to call > > > gdb_evaluate_expression again. > > > > If gdb_evaluate_expression fails in varobj_create, a variable object is > > still created, but just with an undefined value. It needs to be called to > > get value for VALUE_LVAL. > > Why would a second call to gdb_evaluate_expression succeed? We already tried > to evaluate expression, we failed, varobj has no value whatsoever. So, > I'd say we should refuse to assign anything in this case. The second call succeeds if the first one did (when the varobj does have a value) - but we seem to be going backwards. > > > Further, > > > in varobj_create, gdb_evaluate_expression is called in specific frame, > > > and varobj_editable_p calls it in current frame. > > > > The current frame should be the frame in which the variable object is defined. > > Can you explain why that's a problem? > > Here's the code in question: > > /* When the frame is different from the current frame, > we must select the appropriate frame before parsing > the expression, otherwise the value will not be current. > Since select_frame is so benign, just call it for all cases. */ > if (fi != NULL) > { > var->root->frame = get_frame_id (fi); > old_fi = get_selected_frame (NULL); > 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)) > > So it seems to be varobj is not necessary defined in the current frame. When it's created it can be in a different frame according to the syntax. However, in Emacs I only create watch expressions in the selected frame (the manual suggests that "*" sets the value in the current frame but there are many anomalies like this). Likewise I think variable assignment should only be done in the selected frame. Perhaps GDB should only allow this scenario, I don't know. I have used this patch for several months now without a problem and I'd rather focus on real use cases than hypothetical pathological ones. >... > > > 4. I don't think your test actually tests that the 'editable' attribute > > > comes out as 'false'. > > > > I'm not sure what to say. It shows that if you try to assign a value to a > > cast GDB says "Variable object is not editable". > > > > The error message for a variable object of a cast used to be: > > > > &"mi_cmd_var_assign: Could not assign expression to variable object\n" > > ^error,msg="mi_cmd_var_assign: Could not assign expression to variable object" > > (gdb) > > Is this test failing with current GDB, and passing after your change? The "not editable" message is new for this case. > I think > GDB will refuse to assign value to rvalue anyway -- at least here's what I > get without your patch: > > (gdb) > -var-create V * (float)argc > ^done,name="V",numchild="0",value="0",type="float" > (gdb) > -var-assign V 10 > &"mi_cmd_var_assign: Could not assign expression to variable object\n" > ^error,msg="mi_cmd_var_assign: Could not assign expression to variable object" > (gdb) That's what I've just said. > > I could add another test for -var-show-attributes which will now give: > > > > ^done,attr="noneditable" > > > > for a cast but I never use this command. > > I think the biggest value of this patch is that we get this attribute, > *before* trying to assign the value, so yes, I think a test for this > specific behaviour would be great. There's no before or after. It would basically be the same test. -- Nick http://www.inet.net.nz/~nickrob