Re: Remove type prefix for -var-evaluate-expression/functions

Re: Remove type prefix for -var-evaluate-expression/functions

[Nick Roberts]

> Hi,
> the following patch removes 'type prefix' from output of
> -var-evaluate-expression applied to objects of function type (not pointers
> to functions, but functions).
> 
> It causes no regression in the testsuite for me.

But I guess a new test would help prevent a regression (in the code) in the
future.

> Changelog:
> 
> 2006-03-15 Vladimir Prus <ghost@cs.msu.su>
>         
>         * c-valprint.c 
>          (c_val_print): Don't print type prefix for functions.
>          (c_value_print): Print type prefix for functions here.


> Patch attached.

... 
> +  if (TYPE_CODE (type) == TYPE_CODE_FUNC)

This condition must always be true here (case TYPE_CODE_FUNC:).

> +    {
> +      /* FIXME, we should consider, at least for ANSI C language, eliminating
> +         the distinction made between FUNCs and POINTERs to FUNCs.  */
> +      fprintf_filtered (stream, "{");
> +      type_print (type, "", stream, -1);
> +      fprintf_filtered (stream, "} ");
> +    }
> +
> +
>    if (objectprint && (TYPE_CODE (type) == TYPE_CODE_CLASS))
>      {
>        /* Attempt to determine real type of object */


Where's the patch for c_value_print?


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


PS As Jason Molenda suggested to me, if you put "diff -p" in your it will
add the name of the function at the top of each hunk which makes it easier
for others to read.

Re: Remove type prefix for -var-evaluate-expression/functions

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

Nick Roberts wrote:

>> Hi,
>> the following patch removes 'type prefix' from output of
>> -var-evaluate-expression applied to objects of function type (not
>> pointers to functions, but functions).
>> 
>> It causes no regression in the testsuite for me.
> 
> But I guess a new test would help prevent a regression (in the code) in
> the future.

Yes, can you suggest which file should I add this new testcase too?


>> Changelog:
>> 
>> 2006-03-15 Vladimir Prus <ghost@cs.msu.su>
>>         
>>         * c-valprint.c
>>          (c_val_print): Don't print type prefix for functions.
>>          (c_value_print): Print type prefix for functions here.
> 
> 
>> Patch attached.
> 
> ...
>> +  if (TYPE_CODE (type) == TYPE_CODE_FUNC)
> 
> This condition must always be true here (case TYPE_CODE_FUNC:).

This chunk is in 'c_value_print', where there's no switch.      

> 
> 
> Where's the patch for c_value_print?

I guess I'd better send the patch with "-p". It's attached, hopefully it's
more clear.

And thanks for the hint about -p!

- Volodya

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: type_prefix.diff --]
[-- Type: text/x-diff; name="type_prefix.diff", Size: 1384 bytes --]

Index: c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.39
diff -u -p -r1.39 c-valprint.c
--- c-valprint.c	18 Jan 2006 21:24:19 -0000	1.39
+++ c-valprint.c	17 Mar 2006 08:38:41 -0000
@@ -356,11 +356,6 @@ c_val_print (struct type *type, const gd
 	  print_scalar_formatted (valaddr + embedded_offset, type, format, 0, stream);
 	  break;
 	}
-      /* FIXME, we should consider, at least for ANSI C language, eliminating
-         the distinction made between FUNCs and POINTERs to FUNCs.  */
-      fprintf_filtered (stream, "{");
-      type_print (type, "", stream, -1);
-      fprintf_filtered (stream, "} ");
       /* Try to print what function it points to, and its address.  */
       print_address_demangle (address, stream, demangle);
       break;
@@ -570,6 +565,16 @@ c_value_print (struct value *val, struct
 	}
     }
 
+  if (TYPE_CODE (type) == TYPE_CODE_FUNC)
+    {
+      /* FIXME, we should consider, at least for ANSI C language, eliminating
+         the distinction made between FUNCs and POINTERs to FUNCs.  */
+      fprintf_filtered (stream, "{");
+      type_print (type, "", stream, -1);
+      fprintf_filtered (stream, "} ");
+    }
+
+
   if (objectprint && (TYPE_CODE (type) == TYPE_CODE_CLASS))
     {
       /* Attempt to determine real type of object */

Re: Remove type prefix for -var-evaluate-expression/functions

[Daniel Jacobowitz]

On Tue, Apr 04, 2006 at 11:18:02AM +0400, Vladimir Prus wrote:
> The new version of the patch, with testsuite change, is attached.
> 
> Changelog:
> 2006-03-15 Vladimir Prus <ghost@cs.msu.su>
>         
>         * c-valprint.c 
>          (c_val_print): Don't print type prefix for functions.
>          (c_value_print): Print type prefix for functions here.
>         * testsuite/gdb.mi/mi-var-cmd.exp: Test for new behaviour.

Hi Vladimir (do you prefer Volodya?),

This patch is fine.  Please adjust the ChangeLog entry for GNU
formatting - I realize everyone else puts their asterisks in a
different place than we do:

	* c-valprint.c  (c_val_print): Don't print type prefix for functions.
	(c_value_print): Print type prefix for functions here.

	* gdb.mi/mi-var-cmd.exp: Test for printing functions.

Otherwise, feel free to check it in.

[You don't usually want to refer to "new behavior" in the testsuite
changelog entry, because it's tricky to go back and see what commit in
the gdb ChangeLog the test corresponds to.  Yes, this is just CVS's
fault really.]


-- 
Daniel Jacobowitz
CodeSourcery

Re: Remove type prefix for -var-evaluate-expression/functions

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 1029 bytes --]

Daniel Jacobowitz wrote:

>> > Well I guess mi-var-cmd.exp.  Please remember that I'm not the
>> > maintainer
>> > but just an interested party.  See what Daniel says.  He might want
>> > something for mi2-var-cmd.exp too.
>> 
>> Ok, I'll wait for his comment.
> 
> mi-var-cmd.exp sounds good to me.  I don't think we need it in
> mi2-var-cmd.exp.  I have the same question I asked a moment ago about
> Nick's patch - is there any chance that someone relies on this
> information?
> 
> Here I think the chance is pretty slim; for pointers it's a more
> serious concern, but for functions this is a pretty rare case.
> So not versioning this change makes sense to me.

The new version of the patch, with testsuite change, is attached.

Changelog:
2006-03-15 Vladimir Prus <ghost@cs.msu.su>
        
        * c-valprint.c 
         (c_val_print): Don't print type prefix for functions.
         (c_value_print): Print type prefix for functions here.
        * testsuite/gdb.mi/mi-var-cmd.exp: Test for new behaviour.



- Volodya

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: type_prefix_2.diff --]
[-- Type: text/x-diff; name="type_prefix_2.diff", Size: 2168 bytes --]

Index: c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.39
diff -u -r1.39 c-valprint.c
--- c-valprint.c	18 Jan 2006 21:24:19 -0000	1.39
+++ c-valprint.c	4 Apr 2006 07:09:25 -0000
@@ -356,11 +356,6 @@
 	  print_scalar_formatted (valaddr + embedded_offset, type, format, 0, stream);
 	  break;
 	}
-      /* FIXME, we should consider, at least for ANSI C language, eliminating
-         the distinction made between FUNCs and POINTERs to FUNCs.  */
-      fprintf_filtered (stream, "{");
-      type_print (type, "", stream, -1);
-      fprintf_filtered (stream, "} ");
       /* Try to print what function it points to, and its address.  */
       print_address_demangle (address, stream, demangle);
       break;
@@ -570,6 +565,16 @@
 	}
     }
 
+  if (TYPE_CODE (type) == TYPE_CODE_FUNC)
+    {
+      /* FIXME, we should consider, at least for ANSI C language, eliminating
+         the distinction made between FUNCs and POINTERs to FUNCs.  */
+      fprintf_filtered (stream, "{");
+      type_print (type, "", stream, -1);
+      fprintf_filtered (stream, "} ");
+    }
+
+
   if (objectprint && (TYPE_CODE (type) == TYPE_CODE_CLASS))
     {
       /* Attempt to determine real type of object */
Index: testsuite/gdb.mi/mi-var-cmd.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-cmd.exp,v
retrieving revision 1.18
diff -u -r1.18 mi-var-cmd.exp
--- testsuite/gdb.mi/mi-var-cmd.exp	14 Jan 2005 18:17:19 -0000	1.18
+++ testsuite/gdb.mi/mi-var-cmd.exp	4 Apr 2006 07:09:26 -0000
@@ -560,6 +560,15 @@
 mi_gdb_test "-var-update selected_a" \
 	"\\^done,changelist=\\\[\{name=\"selected_a\",in_scope=\"true\",new_type=\"int\",new_num_children=\"0\"\}\\\]" \
 	"update selected_a in do_special_tests"
+	
+mi_gdb_test "-var-create function * *f	" \
+	"\\^done,name=\"function\",numchild=\"0\",type=\"void \\(\\)\"" \
+	"create varobj for function variable"
+
+mi_gdb_test "-var-evaluate-expression function" \
+	"\\^done,value=\"0x.*\"" \
+	"print value of function object"
+
 
 mi_gdb_exit
 return 0

Re: Remove type prefix for -var-evaluate-expression/functions

[Daniel Jacobowitz]

On Fri, Mar 17, 2006 at 01:19:54PM +0300, Vladimir Prus wrote:
> On Friday 17 March 2006 13:02, Nick Roberts wrote:
> > > > > the following patch removes 'type prefix' from output of
> > > > > -var-evaluate-expression applied to objects of function type (not
> > > > > pointers to functions, but functions).
> > > > >
> > > > > It causes no regression in the testsuite for me.
> > > >
> > > > But I guess a new test would help prevent a regression (in the code) in
> > > > the future.
> > >
> > > Yes, can you suggest which file should I add this new testcase too?
> >
> > Well I guess mi-var-cmd.exp.  Please remember that I'm not the maintainer
> > but just an interested party.  See what Daniel says.  He might want
> > something for mi2-var-cmd.exp too.
> 
> Ok, I'll wait for his comment.

mi-var-cmd.exp sounds good to me.  I don't think we need it in
mi2-var-cmd.exp.  I have the same question I asked a moment ago about
Nick's patch - is there any chance that someone relies on this
information?

Here I think the chance is pretty slim; for pointers it's a more
serious concern, but for functions this is a pretty rare case.
So not versioning this change makes sense to me.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Remove type prefix for -var-evaluate-expression/functions

[Vladimir Prus]

On Friday 17 March 2006 13:02, Nick Roberts wrote:
> > > > the following patch removes 'type prefix' from output of
> > > > -var-evaluate-expression applied to objects of function type (not
> > > > pointers to functions, but functions).
> > > >
> > > > It causes no regression in the testsuite for me.
> > >
> > > But I guess a new test would help prevent a regression (in the code) in
> > > the future.
> >
> > Yes, can you suggest which file should I add this new testcase too?
>
> Well I guess mi-var-cmd.exp.  Please remember that I'm not the maintainer
> but just an interested party.  See what Daniel says.  He might want
> something for mi2-var-cmd.exp too.

Ok, I'll wait for his comment.

>
> > > > +  if (TYPE_CODE (type) == TYPE_CODE_FUNC)
> > >
> > > This condition must always be true here (case TYPE_CODE_FUNC:).
> >
> > This chunk is in 'c_value_print', where there's no switch.
>
> Ah yes!  I misread the patch.
>
> > I guess I'd better send the patch with "-p". It's attached, hopefully
> > it's more clear.
> >
> > And thanks for the hint about -p!
>
> Hmm...yes, I guess I've proved my own point.

Indeed.

> PS I'm not subscribed to gdb-patches so please include me in your replies.
> I think its fairly standard to do this as its more inconvenient to get no
> reply, than get two and have to delete one.

Apologies. In fact, I regularly use NNTP interface to read/reply gdb-patches  
(gmane.comp.gdb.patches newsgroup at gmane.org), and NNTP reader does not 
allow to reply by email both to list and poster. I did not realize this can 
cause problems -- I'll try to reply via email client in future.

- Volodya

Re: Remove type prefix for -var-evaluate-expression/functions

[Nick Roberts]

> > > the following patch removes 'type prefix' from output of
> > > -var-evaluate-expression applied to objects of function type (not
> > > pointers to functions, but functions).
> > > 
> > > It causes no regression in the testsuite for me.
> > 
> > But I guess a new test would help prevent a regression (in the code) in
> > the future.
> 
> Yes, can you suggest which file should I add this new testcase too?

Well I guess mi-var-cmd.exp.  Please remember that I'm not the maintainer
but just an interested party.  See what Daniel says.  He might want something
for mi2-var-cmd.exp too.


> > > +  if (TYPE_CODE (type) == TYPE_CODE_FUNC)
> > 
> > This condition must always be true here (case TYPE_CODE_FUNC:).
> 
> This chunk is in 'c_value_print', where there's no switch.      

Ah yes!  I misread the patch.

> I guess I'd better send the patch with "-p". It's attached, hopefully it's
> more clear.

> And thanks for the hint about -p!

Hmm...yes, I guess I've proved my own point.

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


PS I'm not subscribed to gdb-patches so please include me in your replies.
I think its fairly standard to do this as its more inconvenient to get no
reply, than get two and have to delete one.

Remove type prefix for -var-evaluate-expression/functions

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]


Hi,
the following patch removes 'type prefix' from output of
-var-evaluate-expression applied to objects of function type (not pointers
to functions, but functions).

It causes no regression in the testsuite for me.

Changelog:

2006-03-15 Vladimir Prus <ghost@cs.msu.su>
        
        * c-valprint.c 
         (c_val_print): Don't print type prefix for functions.
         (c_value_print): Print type prefix for functions here.


Patch attached.

- Volodya

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: type_prefix.diff --]
[-- Type: text/x-diff; name="type_prefix.diff", Size: 1299 bytes --]

Index: c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.39
diff -u -r1.39 c-valprint.c
--- c-valprint.c	18 Jan 2006 21:24:19 -0000	1.39
+++ c-valprint.c	15 Mar 2006 14:44:45 -0000
@@ -356,11 +356,6 @@
 	  print_scalar_formatted (valaddr + embedded_offset, type, format, 0, stream);
 	  break;
 	}
-      /* FIXME, we should consider, at least for ANSI C language, eliminating
-         the distinction made between FUNCs and POINTERs to FUNCs.  */
-      fprintf_filtered (stream, "{");
-      type_print (type, "", stream, -1);
-      fprintf_filtered (stream, "} ");
       /* Try to print what function it points to, and its address.  */
       print_address_demangle (address, stream, demangle);
       break;
@@ -570,6 +565,16 @@
 	}
     }
 
+  if (TYPE_CODE (type) == TYPE_CODE_FUNC)
+    {
+      /* FIXME, we should consider, at least for ANSI C language, eliminating
+         the distinction made between FUNCs and POINTERs to FUNCs.  */
+      fprintf_filtered (stream, "{");
+      type_print (type, "", stream, -1);
+      fprintf_filtered (stream, "} ");
+    }
+
+
   if (objectprint && (TYPE_CODE (type) == TYPE_CODE_CLASS))
     {
       /* Attempt to determine real type of object */