From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12081 invoked by alias); 29 Dec 2011 11:10:54 -0000 Received: (qmail 12072 invoked by uid 22791); 29 Dec 2011 11:10:53 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 29 Dec 2011 11:10:40 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 63F6F2BAFBF; Thu, 29 Dec 2011 06:10:39 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id ZmVmiaZG0rTB; Thu, 29 Dec 2011 06:10:39 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 6D71C2BAFB5; Thu, 29 Dec 2011 06:10:38 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id D7D79145615; Thu, 29 Dec 2011 03:10:24 -0800 (PST) Date: Thu, 29 Dec 2011 11:13:00 -0000 From: Joel Brobecker To: gdb-patches@sourceware.org, vladimir@codesourcery.com Cc: Tom Tromey Subject: Re: RFC: how to handle mutable types in varobj? Message-ID: <20111229111024.GT23376@adacore.com> References: <20111228155943.GD2632@adacore.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="n+lFg1Zro7sl44OB" Content-Disposition: inline In-Reply-To: <20111228155943.GD2632@adacore.com> User-Agent: Mutt/1.5.20 (2009-06-14) 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: 2011-12/txt/msg00887.txt.bz2 --n+lFg1Zro7sl44OB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 2972 > I'd like to discuss how mutable types should be handled in varobj. It turns out to be a little simpler than I thought, assuming my understanding of the GDB/MI varobj interface is right: If a varobj mutated, whether a root varobj or not, it should be listed in the change list as a "type_changed". However, all its children varobjs are simply destroyed without further ado, and they will not be listed in the change list. This means that we expect the front-end to understand that children varobjs of a varobj that has mutated are invalid, and thus automatically destroyed. With the attached prototype, this is what I get. (1) First, creating the varobj and listing its children: | -var-create v * v | ^done,name="v",numchild="4",value="{...}",type="pck.variant",has_more="0" | (gdb) | -var-list-children 1 v | ^done,numchild="4",children=[child={name="v.disc",exp="disc",numchild="0",value="true",type="boolean"},[snip]],has_more="0" | (gdb) (2) Second, doing an update after some changes in the value, but no mutation (only field "a" in our variant record changed): | -var-update v | ^done,changelist=[{name="v.a",in_scope="true",type_changed="false",has_more="0"}] | (gdb) (3) Lastly, doing an update after the value mutated. This time, we see that varobj simply reports that the value has mutated (type_changed="true"), and just provides the number number of children: | -var-update v | ^done,changelist=[{name="v",in_scope="true",type_changed="true",new_type="pck.variant",new_num_children="3",has_more="0"}] | (gdb) The prototype cannot be applied as is, as it needs to be applied on top of some other changes that are work-in-progress. But it should give a good idea of how I proceeded. The good thing is that this is fairly localized. I'm also wondering whether this might be done without a language-dependent hook (see the first FIXME of ada_varobj_value_has_mutated's description). Perhaps create_child_with_value could solve my practical problem? I haven't gone that far yet. I also tried testing the case where it's a child rather than the root that mutates, but for some reason the child is not properly described (in a sense similar to what c_describe_child does), and thus I miss the mutation. So that feature is untested, but it's just because of a bug I will chase ASAP. The unfortunate part, though, is that this is done somewhat in parallel of the work done to handle pretty-printers. I think it kind of makes sense that pretty-printers should take precedence over this treatment. So, I'm somewhat thinking that it's better that way. I haven't had time to verify that precedence is respected, however. I just wanted to send the patch for comment... The careful reader will also notice that I'm having trouble with figuring out some invariants regarding the `from' and `to' fields with respect to the `num_children' field... More stuff to be figured out later... Thanks, -- Joel --n+lFg1Zro7sl44OB Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="ada-type-mutation.diff" Content-length: 5406 commit d75d746f839bf5a6e022cc8b7bfe1725345036e5 Author: Joel Brobecker Date: Thu Dec 29 11:39:06 2011 +0400 Prototype of Ada value type-mutation. diff --git a/gdb/varobj.c b/gdb/varobj.c index 88f356c..b26e109 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -1759,6 +1759,87 @@ varobj_set_visualizer (struct varobj *var, const char *visualizer) #endif } +/* FIXME: Should be a language_specific attribute? For now, it's just + used as a prototype. It cannot be made language-independent, because + it needs to get the number of children and their names solely from + the new varobj *value*. Perhaps the problem could be solved by + creating a temporary anonymous varobj for the varobj's new value? + But then, there is the question whether mutation is a concept that + remains the same for all languages. It seems a shame to be going + through this for languages such as C, where there are clearly no + type mutations the way they occur in Ada. + + This function should be able to assume that: + - The varobj has a value; + - The varobj's number of children is set (not < 0); + - NEW_VALUE is not NULL. + */ + +static int +ada_varobj_value_has_mutated (struct varobj *var, struct value *new_val) +{ + int i, from, to; + + /* If the number of fields have changed, then for sure the type + has mutated. */ + if (ada_varobj_get_number_of_children (new_val) != var->num_children) + return 1; + + /* If the number of fields have remained the same, then we need + to check the name of each field. If they remain the same, + then chances are the type hasn't mutated. This is technically + an incomplete test, as the child's type might have changed + despite the fact that the name remains the same. But we'll + handle this situation by saying that the child has mutated, + not this value. + + If only part (or none!) of the children have been fetched, + then only check the ones we fetched. It does not matter + to the frontend whether a child that it has not fetched yet + has mutated or not. So just assume it hasn't. */ + + restrict_range (var->children, &from, &to); + for (i = from; i < to; i++) + if (strcmp (ada_varobj_get_name_of_child (var->value, var->name, i), + VEC_index (varobj_p, var->children, i)->name) != 0) + return 1; + + return 0; +} + +/* If NEW_VALUE is the new value of the given varobj (var), return + non-zero if var has mutated. In other words, if the type of + the new value is different from the type of the varobj's old + value. + + NEW_VALUE may be NULL, if the varobj is now out of scope. */ + +static int +varobj_value_has_mutated (struct varobj *var, struct value *new_value) +{ + /* If there was no previous value, it probably means that our varobj + was out of scope. This is a scoping-change issue, handled + elsewhere, which makes the detection of mutations unnecessary. + Same if NEW_VALUE is NULL. */ + if (var->value == NULL || new_value == NULL) + return 0; + + /* If we haven't previously computed the number of children in var, + it does not matter from the front-end's perspective whether + the type has mutated or not. For all intents and purposes, + it has not mutated. + + FIXME: Do we need to check var->from & var->to? I cannot seem + to convince myself that this is independent of num_children. */ + if (var->num_children < 0) + return 0; + + if (var->root->lang->language == vlang_ada) + return ada_varobj_value_has_mutated (var, new_value); + else + return 0; +} + /* Update the values for a variable and its children. This is a two-pronged attack. First, re-parse the value for the root's expression to see if it's changed. Then go all the way @@ -1853,9 +1934,24 @@ varobj_update (struct varobj **varp, int explicit) /* Update this variable, unless it's a root, which is already updated. */ if (!r.value_installed) - { + { + int has_mutated; + new = value_of_child (v->parent, v->index); - if (install_new_value (v, new, 0 /* type not changed */)) + has_mutated = varobj_value_has_mutated (v, new); + + if (has_mutated) + { + /* The children are no longer valid; delete them now. + Report the fact that its type changed as well. */ + varobj_delete (v, NULL, 1 /* only_children */); + v->num_children = -1; + v->to = -1; /* FIXME: Is that necessary? */ + v->from = -1; + r.type_changed = 1; + } + + if (install_new_value (v, new, r.type_changed)) { r.changed = 1; v->updated = 0; @@ -2543,7 +2639,23 @@ value_of_root (struct varobj **var_handle, int *type_changed) *type_changed = 0; } - return (*var->root->lang->value_of_root) (var_handle); + { + struct value *value; + + value = (*var->root->lang->value_of_root) (var_handle); + if (varobj_value_has_mutated (var, value)) + { + /* The type has mutated, so the children are no longer valid. + Just delete them, and tell our caller that the type has + changed. */ + varobj_delete (var, NULL, 1 /* only_children */); + var->num_children = -1; + var->to = -1; /* FIXME: Is that necessary? */ + var->from = -1; + *type_changed = 1; + } + return value; + } } /* What is the ``struct value *'' for the INDEX'th child of PARENT? */ --n+lFg1Zro7sl44OB--