Changes to varobj.c

Changes to varobj.c

[Nick Roberts]

I apologise if I missed it, but I never saw a patch for the change below.

I can see a stale comment before varobj_update:

   NOTE: This function may delete the caller's varobj. If it
   returns TYPE_CHANGED,...

(varobj_update no longer returns TYPE_CHANGED)

2008-05-28  Vladimir Prus  <vladimir@codesourcery.com>

	Refactor varobj_update interface.
        * varobj.c (varobj_update): Report changes as vector.  Also
        return not just a list of varobj, but a list of special structures
        that tell what exactly has changed.
        * varobj.h (enum varobj_update_error): Rename to
        varobj_scope_status.
        (struct varobj_update_result_t): New.
        (varobj_update): Adjust prototype.
        * mi/mi-cmd-var.c: Adjust for changes.
                           ^^^^^^^^^^^^^^^^^^

It's usual to include the name of changed function in the ChangeLog.

If you want a second pair of eyes, I'm more likely to peer review patches than
changes that have already been comitted

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

Re: Changes to varobj.c

[Daniel Jacobowitz]

On Thu, May 29, 2008 at 11:06:23AM +1200, Nick Roberts wrote:
>         * mi/mi-cmd-var.c: Adjust for changes.
>                            ^^^^^^^^^^^^^^^^^^
> 
> It's usual to include the name of changed function in the ChangeLog.

Responding only to this part: when they are mechanical, it is also
typical not to include the names of changed functions.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Changes to varobj.c

[Vladimir Prus]

On Thursday 29 May 2008 03:06:23 Nick Roberts wrote:
> 
> I apologise if I missed it, but I never saw a patch for the change below.

I think you've missed it. The patch was posted here:

	http://article.gmane.org/gmane.comp.gdb.patches/40822

and the version I've checked in has no changes relative to the one
I've posted.

Would you like to CC-d on all MI patches?

> I can see a stale comment before varobj_update:
> 
>    NOTE: This function may delete the caller's varobj. If it
>    returns TYPE_CHANGED,...
> 
> (varobj_update no longer returns TYPE_CHANGED)

You are right. On the other hand, why do delete varobj when its
type changes, or in other words, why value_of_root uses varobj_create? I think
I'll just stop it making do so, together with the other planned change of
making -var-create of floating varobj not fail when the expression cannot be
parsed.

> If you want a second pair of eyes, I'm more likely to peer review patches than
> changes that have already been comitted

There are MI patches I'm fairly confident in, and there are patches where
a second pair of eye is appreciated. This one falls in the latter category,
and that's why I've posted it quite some time ago. I can CC you for all future
MI patches where comments are desired, or use [MI] prefix in emails, or something
else that will make sure you see them.

- Volodya

Re: Changes to varobj.c

[Nick Roberts]

 > I think you've missed it. The patch was posted here:
 > 
 > 	http://article.gmane.org/gmane.comp.gdb.patches/40822
 > 
 > and the version I've checked in has no changes relative to the one
 > I've posted.

It looks like it was attached in reply to a patch with the subject header
"[RFC][patch 0/9] Python support in GDB".  I don't read everything posted to
gdb-patches and delete messages based on the subject which is why I must have
missed this one.

 > Would you like to CC-d on all MI patches?

If you just give an indication in the subject header about the content, as you
have suggested, that should be enough - thanks.

 > > I can see a stale comment before varobj_update:
 > > 
 > >    NOTE: This function may delete the caller's varobj. If it
 > >    returns TYPE_CHANGED,...
 > > 
 > > (varobj_update no longer returns TYPE_CHANGED)
 > 
 > You are right. On the other hand, why do delete varobj when its type
 > changes, or in other words, why value_of_root uses varobj_create? I think
 > I'll just stop it making do so, 

If floating varobjs are used then var->root->exp changes when the frame changes
and using varobj_create is one way to get hold of this expression.  I think if
you don't use varobj_create then you will have to use a chunk of it.

 >                                together with the other planned change of
 > making -var-create of floating varobj not fail when the expression cannot be
 > parsed.

That's an independent matter though, right?

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


PS I've just noticed that:

  struct varobj **cv;
  struct varobj **templist = NULL;

are no longer used in varobj_update.

I've updated my linked list patch to your changes and it looks cleaner.  Using
a vector for result seems like a good idea, I just don't think it works for
varobj_list_children.