Re: [PATCH 4/7] Move struct varobj to varobj.h.

[Doug Evans <dje@google.com>]

Joel Brobecker writes:
 > > How about this?
 > 
 > It does look a lot better to me, FWIW.  The only possibly contentious
 > question left would be making struct varobj public, when I personally
 > tend to prefer opaque structures. But I'm fine with this step, as it
 > helps achieve the goal of moving the language-specific stuff out of
 > varobj.c. I think Doug also pretty much agreed to that change. I would
 > give the patch, say, until the end of the week, JIC others want to
 > comment in.

Yeah, except for the nits you found, looks good to me.

[I can image more structs will get moved into headers in a c++ world,
thus this doesn't bother me.]

 > How does this new patch affect the rest of the patch series? No effect?
 > If not, we can continue reviewing the remainder.  Otherwise, can you
 > post an update?  Sorry it's taking so long. I just don't have much time.
 > But as I said, I like the direction this is taking.
 > 
 > Thanks!
 > 
 > > 2013-10-06  Yao Qi  <yao@codesourcery.com>
 > > 
 > > 	* varobj.c (struct varobj): Move most of the fields to
 > > 	varobj.h.
 > > 	(struct varobj_dynamic): New struct.
 > > 	(varobj_get_display_hint) [HAVE_PYTHON]: Adjust.
 > > 	(varobj_has_more): Likewise.
 > > 	(dynamic_varobj_has_child_method): Likewise.
 > > 	(update_dynamic_varobj_children): Likewise.
 > > 	(varobj_get_num_children): Likewise.
 > > 	(varobj_list_children, varobj_pretty_printed_p): Likewise.
 > > 	(install_new_value_visualizer): Likewise.
 > > 	(install_new_value_visualizer, install_new_value): Likewise.
 > > 	(varobj_update, new_variable, free_variable): Likewise.
 > > 	(my_value_of_variable, value_get_print_value): Likewise.
 > > 	(install_visualizer): Change the type of parameter 'var' to
 > > 	'struct varobjd_dynamic *'.  Callers update.
 > > 	* varobj.h (struct varobj): Moved from varobj.c.
 > > 	(struct varobj) <dynamic>: New field.
 > 
 > > @@ -2924,7 +2861,7 @@ value_get_print_value (struct value *value, enum varobj_display_formats format,
 > >  #if HAVE_PYTHON
 > >    if (gdb_python_initialized)
 > >      {
 > > -      PyObject *value_formatter = var->pretty_printer;
 > > +      PyObject *value_formatter=  var->dynamic->pretty_printer;
 > 
 > You accidently removed a space before '='.
 > 
 > > +/* Every variable in the system has a structure of this type defined
 > > +   for it.  This structure holds all information necessary to manipulate
 > > +   a particular object variable.  Members which must be freed are noted.  */
 > > +struct varobj
 > > +{
 > 
 > Not sure if there is a rule for it, or not. But I tend to prefer an
 > empty line between documentation and structure as well (same as with
 > subprograms). Add it if you agree, or else feel free to ignore. This
 > is just an arbitrary preference, AFAIK, and it really does not matter
 > much to me.

The more lines of code that follow, the more I like the blank line.

For something like:

/* blah blah blah ... */
int foo = 42;

I think the blank line is unnecessary.
[even if the comment itself is several lines]

But for a struct definition where more lines of code are involved, I like
the blank line.
[There is such a rule for function comments.  Whether it was originally
created because functions tend to involve several lines ... I'm not sure,
but I do like it, and am glad we're now enforcing it!]