Re: MI: -var-list-children --simple-values

Re: MI: -var-list-children --simple-values

[Nick Roberts]

 >        -var-list-children --simple-values
 > 
 > is broken on mainline. Printing children of any C++ structure results in 
 > segfault. The problem is that C++ structures have "public" fake child
 > that has no type, so mi_print_value_p tries to access properties of  a NULL 
 > type.
 > 
 > This is easy to fix, but given that printing C++ structures is a mainstream 
 > use-case, and no IDE developer ever complained, this suggests that this 
 > feature is not used. Rather than carry it along, how about just removing it?

ISTR Daniel J wanted --simple-values for consistency with -stack-list-locals.
As it's easy to fix, I suggest doing that.  I also note a mistake in the error
message.  Both are fixed below.

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


*** mi-cmd-var.c	24 Dec 2005 07:57:46 +1300	1.23
--- mi-cmd-var.c	16 Nov 2006 00:09:09 +1300	
***************
*** 269,275 ****
    else
      error (_("Unknown value for PRINT_VALUES\n\
  Must be: 0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
! 	   mi_no_values, mi_simple_values, mi_all_values);
  }
  
  /* Return 1 if given the argument PRINT_VALUES we should display
--- 269,275 ----
    else
      error (_("Unknown value for PRINT_VALUES\n\
  Must be: 0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
! 	   mi_no_values, mi_all_values, mi_simple_values);
  }
  
  /* Return 1 if given the argument PRINT_VALUES we should display
***************
*** 278,286 ****
  static int
  mi_print_value_p (struct type *type, enum print_values print_values)
  {
-   if (type != NULL)
-     type = check_typedef (type);
- 
    if (print_values == PRINT_NO_VALUES)
      return 0;
  
--- 278,283 ----
***************
*** 290,295 ****
--- 287,298 ----
    /* For PRINT_SIMPLE_VALUES, only print the value if it has a type
       and that type is not a compound type.  */
  
+   if (type != NULL)
+     type = check_typedef (type);
+   else
+     /* For the "fake" variables. */
+     return 0;
+ 
    return (TYPE_CODE (type) != TYPE_CODE_ARRAY
  	  && TYPE_CODE (type) != TYPE_CODE_STRUCT
  	  && TYPE_CODE (type) != TYPE_CODE_UNION);

Re: MI: -var-list-children --simple-values

[Vladimir Prus]

On Wednesday 15 November 2006 14:17, Nick Roberts wrote:
>  >        -var-list-children --simple-values
>  >
>  > is broken on mainline. Printing children of any C++ structure results in
>  > segfault. The problem is that C++ structures have "public" fake child
>  > that has no type, so mi_print_value_p tries to access properties of  a
>  > NULL type.
>  >
>  > This is easy to fix, but given that printing C++ structures is a
>  > mainstream use-case, and no IDE developer ever complained, this suggests
>  > that this feature is not used. Rather than carry it along, how about
>  > just removing it?
>
> ISTR Daniel J wanted --simple-values for consistency with
> -stack-list-locals. As it's easy to fix, I suggest doing that.  

There are many things that are easy to fix, but it does not mean we need to 
keep unused functionality.

Perhaps Dan can comment why he really needed --simple-values.

> I also note 
> a mistake in the error message.  Both are fixed below.

I don't see any regression test for this crash. Are you going to provide one?

- Volodya

Re: MI: -var-list-children --simple-values

[Daniel Jacobowitz]

On Wed, Nov 15, 2006 at 02:50:19PM +0300, Vladimir Prus wrote:
> On Wednesday 15 November 2006 14:17, Nick Roberts wrote:
> > ISTR Daniel J wanted --simple-values for consistency with
> > -stack-list-locals. As it's easy to fix, I suggest doing that.  
> 
> There are many things that are easy to fix, but it does not mean we need to 
> keep unused functionality.
> 
> Perhaps Dan can comment why he really needed --simple-values.

I felt that it was useful; I still do.  Takeup of new MI features by
IDEs seems to be a very slow process, since many support the system's
installed GDB (which may be several revisions behind); reports of MI
features that don't work also seems to be a bit spotty.  So, honestly,
I wouldn't read too much into it that no one noticed.  It was new in
GDB 6.4.

> > I also note 
> > a mistake in the error message.  Both are fixed below.
> 
> I don't see any regression test for this crash. Are you going to
> provide one?

I'd appreciate it if one of you could do that, yes.  Shame on me, I
didn't write one at the time.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: -var-list-children --simple-values

[Vladimir Prus]

On Wednesday 15 November 2006 17:43, Daniel Jacobowitz wrote:
> On Wed, Nov 15, 2006 at 02:50:19PM +0300, Vladimir Prus wrote:
> > On Wednesday 15 November 2006 14:17, Nick Roberts wrote:
> > > ISTR Daniel J wanted --simple-values for consistency with
> > > -stack-list-locals. As it's easy to fix, I suggest doing that.
> >
> > There are many things that are easy to fix, but it does not mean we need
> > to keep unused functionality.
> >
> > Perhaps Dan can comment why he really needed --simple-values.
>
> I felt that it was useful; I still do.  

Why? Especially, why it's better than --all-values. Note that --simple-values 
only don't print values of arrays, structures and unions, and if you check 
c_value_of_variable, you'll see that for structures just a literal string is 
returned, while for array very trivial formatting operation is used.

In other words, --all-values do not involve any complex processing, reading 
data from the target, or anything. What are we trying to save by 
adding --simple-values?

> Takeup of new MI features by 
> IDEs seems to be a very slow process, since many support the system's
> installed GDB (which may be several revisions behind); reports of MI
> features that don't work also seems to be a bit spotty.  So, honestly,
> I wouldn't read too much into it that no one noticed.  It was new in
> GDB 6.4.
>
> > > I also note
> > > a mistake in the error message.  Both are fixed below.
> >
> > I don't see any regression test for this crash. Are you going to
> > provide one?
>
> I'd appreciate it if one of you could do that, yes.  Shame on me, I
> didn't write one at the time.

Heh ;-)

- Volodya

Re: MI: -var-list-children --simple-values

[Daniel Jacobowitz]

On Wed, Nov 15, 2006 at 05:55:02PM +0300, Vladimir Prus wrote:
> Why? Especially, why it's better than --all-values. Note that --simple-values 
> only don't print values of arrays, structures and unions, and if you check 
> c_value_of_variable, you'll see that for structures just a literal string is 
> returned, while for array very trivial formatting operation is used.
> 
> In other words, --all-values do not involve any complex processing, reading 
> data from the target, or anything. What are we trying to save by 
> adding --simple-values?

But the result of --all-values where it differs from --simple-values is
probably not used.  I've never used an IDE that printed out the
collapsed value of a struct; instead it wants an expandable item
for each struct member.  --simple-values is for things without
children.

I didn't invent --simple-values though, I just added it to -var-update.
Nick added it to -stack-list-locals on 20-Jan-2004.  Ah here:
  http://sourceware.org/ml/gdb/2003-11/msg00055.html

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: -var-list-children --simple-values

[Vladimir Prus]

On Wednesday 15 November 2006 18:09, Daniel Jacobowitz wrote:
> On Wed, Nov 15, 2006 at 05:55:02PM +0300, Vladimir Prus wrote:
> > Why? Especially, why it's better than --all-values. Note that
> > --simple-values only don't print values of arrays, structures and unions,
> > and if you check c_value_of_variable, you'll see that for structures just
> > a literal string is returned, while for array very trivial formatting
> > operation is used.
> >
> > In other words, --all-values do not involve any complex processing,
> > reading data from the target, or anything. What are we trying to save by
> > adding --simple-values?
>
> But the result of --all-values where it differs from --simple-values is
> probably not used.  I've never used an IDE that printed out the
> collapsed value of a struct; 

Eclipse? KDevelop? I can send you screenshots, but I hope you'll believe me 
saying that both use "{...}" as value of a structure.

> I didn't invent --simple-values though, I just added it to -var-update.
> Nick added it to -stack-list-locals on 20-Jan-2004.  Ah here:
>   http://sourceware.org/ml/gdb/2003-11/msg00055.html

This is in fact completely different change. Here's 
what "-stack-list-locals --all-values" produces:

      ^done,locals=[{name="the_struct",value="{a_fr = 120, b = 333, inner_fr =
           {i = 0, j = 5, k = 5}, c = 3, pad = {0 <repeats 100 times>}, d =
            10, pad2 = {0 <repeats 100 times>}, e = 10}"}]

So, the difference between --all-values and --simple-values is dramatic. In 
case of --var-list-children you don't get full structure printed. You won't 
even have the structure *read* (with my laziness patch, that is).

So, while --simple-values might be reasonable for -stack-list-locals, it's not 
all that needed for -var-list-children.

- Volodya

Re: MI: -var-list-children --simple-values

[Nick Roberts]

 > > I didn't invent --simple-values though, I just added it to -var-update.
 > > Nick added it to -stack-list-locals on 20-Jan-2004.  Ah here:
 > >   http://sourceware.org/ml/gdb/2003-11/msg00055.html

Actually I think you added it to -var-update (where I think it makes
no difference as all values here are simple) and -var-list-children.  I think
this all started because when I suggested "--all-values" for -var-update
and -var-list-children Eli found the names unfortunate and suggested others.
I think the final solution was an effort to keep everyone happy.

 > This is in fact completely different change. Here's 
 > what "-stack-list-locals --all-values" produces:
 > 
 >       ^done,locals=[{name="the_struct",value="{a_fr = 120, b = 333, inner_fr =
 >            {i = 0, j = 5, k = 5}, c = 3, pad = {0 <repeats 100 times>}, d =
 >             10, pad2 = {0 <repeats 100 times>}, e = 10}"}]
 > 
 > So, the difference between --all-values and --simple-values is dramatic. In 
 > case of --var-list-children you don't get full structure printed. You won't 
 > even have the structure *read* (with my laziness patch, that is).

Yes, I describe the purpose in the manual:

  ...; and if it is 2 or `--simple-values', print the name, type and value for
  simple data types and the name and type for arrays, structures and
  unions.  In this last case, a frontend can immediately display the
  value of simple data types and create variable objects for other data
  types when the the user wishes to explore their values in more detail.

ISTR this is what Totalview does (not that it matters but it might help some
understand its purpose).

 > So, while --simple-values might be reasonable for -stack-list-locals, it's
 > not all that needed for -var-list-children.

I can't see where it's needed, but its more trouble to remove than to fix.

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

Re: MI: -var-list-children --simple-values

[Vladimir Prus]

On Wednesday 15 November 2006 23:25, you wrote:

>   ...; and if it is 2 or `--simple-values', print the name, type and value
> for simple data types and the name and type for arrays, structures and
> unions.  In this last case, a frontend can immediately display the value of
> simple data types and create variable objects for other data types when the
> the user wishes to explore their values in more detail.
>
> ISTR this is what Totalview does (not that it matters but it might help
> some understand its purpose).
>
>  > So, while --simple-values might be reasonable for -stack-list-locals,
>  > it's not all that needed for -var-list-children.
>
> I can't see where it's needed, but its more trouble to remove than to fix.

Why? Removing is really quick. Fixing requires:

	- Fix itself (you've posted it)
	- Documentation (specific to -var-list-children).
	- Testcase

I did not see the latter two, so I suppose it's still some work ;-)

- Volodya

Re: MI: -var-list-children --simple-values

[Nick Roberts]

 > >  > So, while --simple-values might be reasonable for -stack-list-locals,
 > >  > it's not all that needed for -var-list-children.
 > >
 > > I can't see where it's needed, but its more trouble to remove than to fix.
 > 
 > Why? Removing is really quick. Fixing requires:

I don't really want to remove it as I didn't want to add it in the first place.
I don't mind it being there: I just don't use it.  I guess if you want remove
it then it's clearly less work (for me!) and that's fine with me too but you
need to remove the test and the description in the manual too.  More
imporatantly I think Daniel J arranged so that -var-list-children,
-stack-list-locals and -var-update shared code so you'll have to break that up.

 > 	- Fix itself (you've posted it)
 > 	- Documentation (specific to -var-list-children).
 > 	- Testcase
 >
 > I did not see the latter two, so I suppose it's still some work ;-)

If we fix it no changes are needed there unless you want to add a test for the
fix.


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

Re: MI: -var-list-children --simple-values

[Daniel Jacobowitz]

On Wed, Nov 15, 2006 at 07:29:28PM +0300, Vladimir Prus wrote:
> This is in fact completely different change. Here's 
> what "-stack-list-locals --all-values" produces:
> 
>       ^done,locals=[{name="the_struct",value="{a_fr = 120, b = 333, inner_fr =
>            {i = 0, j = 5, k = 5}, c = 3, pad = {0 <repeats 100 times>}, d =
>             10, pad2 = {0 <repeats 100 times>}, e = 10}"}]
> 
> So, the difference between --all-values and --simple-values is dramatic. In 
> case of --var-list-children you don't get full structure printed. You won't 
> even have the structure *read* (with my laziness patch, that is).
> 
> So, while --simple-values might be reasonable for -stack-list-locals, it's not 
> all that needed for -var-list-children.

Hmm.  I didn't realize that one use of --all-values printed out
structure contents and the other only "{...}" - I'm quite surprised.
I guess --simple-values and --all-values aren't so different then
for the varobj commands :-(

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: -var-list-children --simple-values

[Nick Roberts]

 > > I don't see any regression test for this crash. Are you going to
 > > provide one?
 > 
 > I'd appreciate it if one of you could do that, yes.  Shame on me, I
 > didn't write one at the time.

But we did create a test for -var-list-children --simple-values:

2005-07-27  Nick Roberts  <nickrob@snap.net.nz>

	* gdb.mi/mi-var-child.exp: Adapt tests for extra elements and use
	them for tests for "-var-update --no-values" and
	"-var-update --all-values".  
	Add test for "-var-list-children --simple-values".

Just not one for a C++ structure.

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