[PATCH RFC] Fix ptype behavior for structs

[PATCH RFC] Fix ptype behavior for structs

[Kevin Buettner]

The problem: Given struct definitions like the following:

    struct crass { char * const ptr; } crass;
    struct crisp { char * const *ptr; } crisp;

GDB currently prints these as follows:

    (gdb) ptype crass
    type = struct crass {
	char * constptr;
    }
    (gdb) ptype crisp
    type = struct crisp {
	char * const*ptr;
    }

For ``ptype crass'', the output is wrong because gdb failed to print
a space in between ``const'' and ``ptr''.  For ``ptype crisp'', the
output isn't technically wrong, but it's not as pretty as it could
be.  Here is what I regard to be the correct behavior:

    (gdb) ptype crass
    type = struct crass {
	char * const ptr;
    }
    (gdb) ptype crisp
    type = struct crisp {
	char * const *ptr;
    }

These examples come from the new constvars.exp tests that I've just
submitted.  See:

  http://sources.redhat.com/ml/gdb-patches/2002-10/msg00221.html

With regard to removal of the extern declaration for
c_type_print_varspec_prefix() from c-lang.h, it turns out that the
only files which use this function are c-typeprint.c and symtab.c. 
However, symtab.c doesn't really call it.  The code in question has
been disabled via a ``#if 0'' since symtab.c was first added to the
Cygnus cvs repository on March 28, 1991.  Here's what it looks like in
the present day sources:

#if 0
      /* Tiemann says: "info methods was never implemented."  */
      char *demangled_name;
      c_type_print_base (TYPE_FN_FIELD_TYPE (t, block),
			 gdb_stdout, 0, 0);
      c_type_print_varspec_prefix (TYPE_FN_FIELD_TYPE (t, block),
				   gdb_stdout, 0);
      ...
#endif

(The "Tiemann says" comment was originally associated with a different
bit of disabled code and apparently became associated with the above
code in a rewrite by Peter Schaer in December of 1996.)

Anyway, it seems to me that if we want to someday enable the above
code, it ought to be rewritten so that it doesn't call either
c_type_print_base() or c_type_print_varspec_prefix() directly.

I'll wait a couple of days for comments before checking this in.

	* c-lang.h (c_type_print_varspec_prefix): Delete.
	* c-typeprint.c (c_type_print_varspec_prefix): Make static.  Add
	``need_post_space'' parameter.  Adjust all callers.

Index: c-lang.h
===================================================================
RCS file: /cvs/src/src/gdb/c-lang.h,v
retrieving revision 1.5
diff -u -p -r1.5 c-lang.h
--- c-lang.h	17 May 2002 17:57:48 -0000	1.5
+++ c-lang.h	10 Oct 2002 20:43:12 -0000
@@ -65,9 +65,6 @@ extern struct type **const (c_builtin_ty
 
 extern void c_type_print_base (struct type *, struct ui_file *, int, int);
 
-extern void c_type_print_varspec_prefix (struct type *, struct ui_file *,
-					 int, int);
-
 /* These are in cp-valprint.c */
 
 extern int vtblprint;		/* Controls printing of vtbl's */
Index: c-typeprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-typeprint.c,v
retrieving revision 1.22
diff -u -p -r1.22 c-typeprint.c
--- c-typeprint.c	29 Jul 2002 22:55:26 -0000	1.22
+++ c-typeprint.c	10 Oct 2002 20:43:13 -0000
@@ -49,8 +49,8 @@ static void c_type_print_args (struct ty
 
 static void cp_type_print_derivation_info (struct ui_file *, struct type *);
 
-void c_type_print_varspec_prefix (struct type *, struct ui_file *, int,
-				  int);
+static void c_type_print_varspec_prefix (struct type *, struct ui_file *, int,
+				         int, int);
 
 /* Print "const", "volatile", or address space modifiers. */
 static void c_type_print_modifier (struct type *, struct ui_file *,
@@ -67,6 +67,7 @@ c_print_type (struct type *type, char *v
 {
   register enum type_code code;
   int demangled_args;
+  int need_post_space;
 
   if (show > 0)
     CHECK_TYPEDEF (type);
@@ -85,7 +86,8 @@ c_print_type (struct type *type, char *v
 	|| code == TYPE_CODE_MEMBER
 	|| code == TYPE_CODE_REF)))
     fputs_filtered (" ", stream);
-  c_type_print_varspec_prefix (type, stream, show, 0);
+  need_post_space = (varstring != NULL && strcmp (varstring, "") != 0);
+  c_type_print_varspec_prefix (type, stream, show, 0, need_post_space);
 
   if (varstring != NULL)
     {
@@ -192,11 +194,15 @@ cp_type_print_method_args (struct type *
    On outermost call, pass 0 for PASSED_A_PTR.
    On outermost call, SHOW > 0 means should ignore
    any typename for TYPE and show its details.
-   SHOW is always zero on recursive calls.  */
+   SHOW is always zero on recursive calls.
+   
+   NEED_POST_SPACE is non-zero when a space will be be needed
+   between a trailing qualifier and a field, variable, or function
+   name.  */
 
 void
 c_type_print_varspec_prefix (struct type *type, struct ui_file *stream,
-			     int show, int passed_a_ptr)
+			     int show, int passed_a_ptr, int need_post_space)
 {
   char *name;
   if (type == 0)
@@ -210,15 +216,15 @@ c_type_print_varspec_prefix (struct type
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_PTR:
-      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 1);
+      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 1, 1);
       fprintf_filtered (stream, "*");
-      c_type_print_modifier (type, stream, 1, 0);
+      c_type_print_modifier (type, stream, 1, need_post_space);
       break;
 
     case TYPE_CODE_MEMBER:
       if (passed_a_ptr)
 	fprintf_filtered (stream, "(");
-      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0);
+      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0);
       fprintf_filtered (stream, " ");
       name = type_name_no_tag (TYPE_DOMAIN_TYPE (type));
       if (name)
@@ -231,7 +237,7 @@ c_type_print_varspec_prefix (struct type
     case TYPE_CODE_METHOD:
       if (passed_a_ptr)
 	fprintf_filtered (stream, "(");
-      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0);
+      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0);
       if (passed_a_ptr)
 	{
 	  fprintf_filtered (stream, " ");
@@ -241,19 +247,19 @@ c_type_print_varspec_prefix (struct type
       break;
 
     case TYPE_CODE_REF:
-      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 1);
+      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 1, 0);
       fprintf_filtered (stream, "&");
-      c_type_print_modifier (type, stream, 1, 0);
+      c_type_print_modifier (type, stream, 1, need_post_space);
       break;
 
     case TYPE_CODE_FUNC:
-      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0);
+      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0);
       if (passed_a_ptr)
 	fprintf_filtered (stream, "(");
       break;
 
     case TYPE_CODE_ARRAY:
-      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0);
+      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0);
       if (passed_a_ptr)
 	fprintf_filtered (stream, "(");
       break;

Re: [PATCH RFC] Fix ptype behavior for structs

[Jim Blandy]

Looks good to me.  I agree with you that the #if 0'd code in
print_symbol_info should be done a different way.

Kevin Buettner <kevinb@redhat.com> writes:

> The problem: Given struct definitions like the following:
> 
>     struct crass { char * const ptr; } crass;
>     struct crisp { char * const *ptr; } crisp;
> 
> GDB currently prints these as follows:
> 
>     (gdb) ptype crass
>     type = struct crass {
> 	char * constptr;
>     }
>     (gdb) ptype crisp
>     type = struct crisp {
> 	char * const*ptr;
>     }
> 
> For ``ptype crass'', the output is wrong because gdb failed to print
> a space in between ``const'' and ``ptr''.  For ``ptype crisp'', the
> output isn't technically wrong, but it's not as pretty as it could
> be.  Here is what I regard to be the correct behavior:
> 
>     (gdb) ptype crass
>     type = struct crass {
> 	char * const ptr;
>     }
>     (gdb) ptype crisp
>     type = struct crisp {
> 	char * const *ptr;
>     }
> 
> These examples come from the new constvars.exp tests that I've just
> submitted.  See:
> 
>   http://sources.redhat.com/ml/gdb-patches/2002-10/msg00221.html
> 
> With regard to removal of the extern declaration for
> c_type_print_varspec_prefix() from c-lang.h, it turns out that the
> only files which use this function are c-typeprint.c and symtab.c. 
> However, symtab.c doesn't really call it.  The code in question has
> been disabled via a ``#if 0'' since symtab.c was first added to the
> Cygnus cvs repository on March 28, 1991.  Here's what it looks like in
> the present day sources:
> 
> #if 0
>       /* Tiemann says: "info methods was never implemented."  */
>       char *demangled_name;
>       c_type_print_base (TYPE_FN_FIELD_TYPE (t, block),
> 			 gdb_stdout, 0, 0);
>       c_type_print_varspec_prefix (TYPE_FN_FIELD_TYPE (t, block),
> 				   gdb_stdout, 0);
>       ...
> #endif
> 
> (The "Tiemann says" comment was originally associated with a different
> bit of disabled code and apparently became associated with the above
> code in a rewrite by Peter Schaer in December of 1996.)
> 
> Anyway, it seems to me that if we want to someday enable the above
> code, it ought to be rewritten so that it doesn't call either
> c_type_print_base() or c_type_print_varspec_prefix() directly.
> 
> I'll wait a couple of days for comments before checking this in.
> 
> 	* c-lang.h (c_type_print_varspec_prefix): Delete.
> 	* c-typeprint.c (c_type_print_varspec_prefix): Make static.  Add
> 	``need_post_space'' parameter.  Adjust all callers.
> 
> Index: c-lang.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/c-lang.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 c-lang.h
> --- c-lang.h	17 May 2002 17:57:48 -0000	1.5
> +++ c-lang.h	10 Oct 2002 20:43:12 -0000
> @@ -65,9 +65,6 @@ extern struct type **const (c_builtin_ty
>  
>  extern void c_type_print_base (struct type *, struct ui_file *, int, int);
>  
> -extern void c_type_print_varspec_prefix (struct type *, struct ui_file *,
> -					 int, int);
> -
>  /* These are in cp-valprint.c */
>  
>  extern int vtblprint;		/* Controls printing of vtbl's */
> Index: c-typeprint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/c-typeprint.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 c-typeprint.c
> --- c-typeprint.c	29 Jul 2002 22:55:26 -0000	1.22
> +++ c-typeprint.c	10 Oct 2002 20:43:13 -0000
> @@ -49,8 +49,8 @@ static void c_type_print_args (struct ty
>  
>  static void cp_type_print_derivation_info (struct ui_file *, struct type *);
>  
> -void c_type_print_varspec_prefix (struct type *, struct ui_file *, int,
> -				  int);
> +static void c_type_print_varspec_prefix (struct type *, struct ui_file *, int,
> +				         int, int);
>  
>  /* Print "const", "volatile", or address space modifiers. */
>  static void c_type_print_modifier (struct type *, struct ui_file *,
> @@ -67,6 +67,7 @@ c_print_type (struct type *type, char *v
>  {
>    register enum type_code code;
>    int demangled_args;
> +  int need_post_space;
>  
>    if (show > 0)
>      CHECK_TYPEDEF (type);
> @@ -85,7 +86,8 @@ c_print_type (struct type *type, char *v
>  	|| code == TYPE_CODE_MEMBER
>  	|| code == TYPE_CODE_REF)))
>      fputs_filtered (" ", stream);
> -  c_type_print_varspec_prefix (type, stream, show, 0);
> +  need_post_space = (varstring != NULL && strcmp (varstring, "") != 0);
> +  c_type_print_varspec_prefix (type, stream, show, 0, need_post_space);
>  
>    if (varstring != NULL)
>      {
> @@ -192,11 +194,15 @@ cp_type_print_method_args (struct type *
>     On outermost call, pass 0 for PASSED_A_PTR.
>     On outermost call, SHOW > 0 means should ignore
>     any typename for TYPE and show its details.
> -   SHOW is always zero on recursive calls.  */
> +   SHOW is always zero on recursive calls.
> +   
> +   NEED_POST_SPACE is non-zero when a space will be be needed
> +   between a trailing qualifier and a field, variable, or function
> +   name.  */
>  
>  void
>  c_type_print_varspec_prefix (struct type *type, struct ui_file *stream,
> -			     int show, int passed_a_ptr)
> +			     int show, int passed_a_ptr, int need_post_space)
>  {
>    char *name;
>    if (type == 0)
> @@ -210,15 +216,15 @@ c_type_print_varspec_prefix (struct type
>    switch (TYPE_CODE (type))
>      {
>      case TYPE_CODE_PTR:
> -      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 1);
> +      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 1, 1);
>        fprintf_filtered (stream, "*");
> -      c_type_print_modifier (type, stream, 1, 0);
> +      c_type_print_modifier (type, stream, 1, need_post_space);
>        break;
>  
>      case TYPE_CODE_MEMBER:
>        if (passed_a_ptr)
>  	fprintf_filtered (stream, "(");
> -      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0);
> +      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0);
>        fprintf_filtered (stream, " ");
>        name = type_name_no_tag (TYPE_DOMAIN_TYPE (type));
>        if (name)
> @@ -231,7 +237,7 @@ c_type_print_varspec_prefix (struct type
>      case TYPE_CODE_METHOD:
>        if (passed_a_ptr)
>  	fprintf_filtered (stream, "(");
> -      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0);
> +      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0);
>        if (passed_a_ptr)
>  	{
>  	  fprintf_filtered (stream, " ");
> @@ -241,19 +247,19 @@ c_type_print_varspec_prefix (struct type
>        break;
>  
>      case TYPE_CODE_REF:
> -      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 1);
> +      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 1, 0);
>        fprintf_filtered (stream, "&");
> -      c_type_print_modifier (type, stream, 1, 0);
> +      c_type_print_modifier (type, stream, 1, need_post_space);
>        break;
>  
>      case TYPE_CODE_FUNC:
> -      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0);
> +      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0);
>        if (passed_a_ptr)
>  	fprintf_filtered (stream, "(");
>        break;
>  
>      case TYPE_CODE_ARRAY:
> -      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0);
> +      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0);
>        if (passed_a_ptr)
>  	fprintf_filtered (stream, "(");
>        break;

Re: [PATCH RFC] Fix ptype behavior for structs

[Kevin Buettner]

On Oct 10,  2:32pm, Kevin Buettner wrote:

> I'll wait a couple of days for comments before checking this in.
> 
> 	* c-lang.h (c_type_print_varspec_prefix): Delete.
> 	* c-typeprint.c (c_type_print_varspec_prefix): Make static.  Add
> 	``need_post_space'' parameter.  Adjust all callers.

Committed.

Re: [PATCH RFC] Fix ptype behavior for structs

[Andrew Cagney]

> Looks good to me.  I agree with you that the #if 0'd code in
> print_symbol_info should be done a different way.

So it gets deleted?

Andrew

>> #if 0
>>       /* Tiemann says: "info methods was never implemented."  */
>>       char *demangled_name;
>>       c_type_print_base (TYPE_FN_FIELD_TYPE (t, block),
>> 			 gdb_stdout, 0, 0);
>>       c_type_print_varspec_prefix (TYPE_FN_FIELD_TYPE (t, block),
>> 				   gdb_stdout, 0);
>>       ...
>> #endif