diff mbox

Add printf format specifier for printing enumerator

Message ID 1454681876-17628-1-git-send-email-simon.marchi@polymtl.ca
State Changes Requested, archived
Headers

Commit Message

Simon Marchi Feb. 5, 2016, 2:17 p.m. UTC 
  This patch adds a format specifier to gdb's printf command, to print the
enumerator (the text label) of an enum value.  It's currently possible
to display that value using the print command, but not as part of a
formatted string.

For example, assuming we have the following enum definition:

  enum NodeType {
    ...
    NODE_INTEGER,
    ...
  };

  enum NodeType node = NODE_INTEGER;

we can display it this way:

  (gdb) printf "Visiting node of type %q\n", node
  Visiting node of type NODE_INTEGER

All the letters of "enum" are already used as format specifiers.
Actually, %m is used in GNU's extensions to printf to print errno's text
representation.  It is not used in GDB's printf, but I'd prefer to leave
the door open if we want to add it one day.  I just chose %q at random,
if you think another letter would be a better choice, I have no
objection.

In the tests, I added some enum values to enum some_volatile_enum, so
that it's not treated as an enum flag.

What do you think?

gdb/ChangeLog:

	* common/format.c (parse_format_string): Handle 'q' format specifier.
	* common/format.h (enum argclass): Add enum_arg.
	* printcmd.c (printf_enum): New function.
	(ui_printf): Handle enum_arg argclass.
	* valprint.c (val_print_enum_label): New function, factored out
	from...
	(generic_val_print_enum): ... this.
	* valprint.h (val_print_enum_label): New function declaration.
	* NEWS: Mention the new format specifier.

gdb/testsuite/ChangeLog:

	* gdb.base/printcmds.exp (test_printf): Add enum printf tests.

gdb/doc/ChangeLog:

	* gdb.texinfo (Commands for Controlled Output): Mention q
	conversion letter.
---
 gdb/NEWS                             |  6 +++
 gdb/common/format.c                  |  4 ++
 gdb/common/format.h                  |  2 +-
 gdb/doc/gdb.texinfo                  |  3 ++
 gdb/printcmd.c                       | 20 +++++++++
 gdb/testsuite/gdb.base/printcmds.c   |  2 +-
 gdb/testsuite/gdb.base/printcmds.exp | 10 +++++
 gdb/valprint.c                       | 87 +++++++++++++++++++-----------------
 gdb/valprint.h                       |  3 ++
 9 files changed, 95 insertions(+), 42 deletions(-)

Comments

Pedro Alves Feb. 5, 2016, 2:51 p.m. UTC | #1 
On 02/05/2016 02:17 PM, Simon Marchi wrote:

>   (gdb) printf "Visiting node of type %q\n", node
>   Visiting node of type NODE_INTEGER

> What do you think?

Did you consider a convenience function to convert a value
to string, instead?  Then you'd write:

 (gdb) printf "Visiting node of type %s\n", $_as_string(node)

And maybe do other things with it.

Thanks,
Pedro Alves
Eli Zaretskii Feb. 5, 2016, 2:52 p.m. UTC | #2 
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Cc: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Fri,  5 Feb 2016 09:17:56 -0500
> 
> This patch adds a format specifier to gdb's printf command, to print the
> enumerator (the text label) of an enum value.  It's currently possible
> to display that value using the print command, but not as part of a
> formatted string.
> 
> For example, assuming we have the following enum definition:
> 
>   enum NodeType {
>     ...
>     NODE_INTEGER,
>     ...
>   };
> 
>   enum NodeType node = NODE_INTEGER;
> 
> we can display it this way:
> 
>   (gdb) printf "Visiting node of type %q\n", node
>   Visiting node of type NODE_INTEGER
> 
> All the letters of "enum" are already used as format specifiers.

What about upper-case letters?

> +@code{printf} supports the @samp{q} conversion letter to print the textual
> +label (enumerator) of a C enumeration value.

This sentence will look awkward because it starts with a lower-case
letter.  How about starting with "Also, "?

I think an example here will be good, as the text does not make it
crystal clear what will be printed.

Other than that, the documentation parts look good to me, thanks.
Simon Marchi Feb. 5, 2016, 3:02 p.m. UTC | #3 
On 2016-02-05 09:51, Pedro Alves wrote:
> Did you consider a convenience function to convert a value
> to string, instead?  Then you'd write:
> 
>  (gdb) printf "Visiting node of type %s\n", $_as_string(node)
> 
> And maybe do other things with it.
> 
> Thanks,
> Pedro Alves

No, but I think it's a good idea.

Do you see _as_string as applicable to any type, a bit like str() in 
Python, or just to some specific types that we chose and make sense 
(like enum)?
Pedro Alves Feb. 5, 2016, 3:08 p.m. UTC | #4 
On 02/05/2016 03:02 PM, Simon Marchi wrote:
> On 2016-02-05 09:51, Pedro Alves wrote:
>> Did you consider a convenience function to convert a value
>> to string, instead?  Then you'd write:
>>
>>  (gdb) printf "Visiting node of type %s\n", $_as_string(node)
>>
>> And maybe do other things with it.
> 
> No, but I think it's a good idea.
> 
> Do you see _as_string as applicable to any type, a bit like str() in 
> Python, or just to some specific types that we chose and make sense 
> (like enum)?

I was thinking the former.  Maybe even implement it in Python, in
gdb/python/lib/gdb/function/?

Thanks,
Pedro Alves
Simon Marchi Feb. 5, 2016, 7:58 p.m. UTC | #5 
On 2016-02-05 09:52, Eli Zaretskii wrote:
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> Cc: Simon Marchi <simon.marchi@polymtl.ca>
>> Date: Fri,  5 Feb 2016 09:17:56 -0500
>> 
>> This patch adds a format specifier to gdb's printf command, to print 
>> the
>> enumerator (the text label) of an enum value.  It's currently possible
>> to display that value using the print command, but not as part of a
>> formatted string.
>> 
>> For example, assuming we have the following enum definition:
>> 
>>   enum NodeType {
>>     ...
>>     NODE_INTEGER,
>>     ...
>>   };
>> 
>>   enum NodeType node = NODE_INTEGER;
>> 
>> we can display it this way:
>> 
>>   (gdb) printf "Visiting node of type %q\n", node
>>   Visiting node of type NODE_INTEGER
>> 
>> All the letters of "enum" are already used as format specifiers.
> 
> What about upper-case letters?
> 
>> +@code{printf} supports the @samp{q} conversion letter to print the 
>> textual
>> +label (enumerator) of a C enumeration value.
> 
> This sentence will look awkward because it starts with a lower-case
> letter.  How about starting with "Also, "?
> 
> I think an example here will be good, as the text does not make it
> crystal clear what will be printed.
> 
> Other than that, the documentation parts look good to me, thanks.

Thanks for the doc review.  The code will probably change a lot, so the 
doc will as well.  I'll keep your comments in mind when I have to re-do 
it.

Simon
diff mbox

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 962eae4..ce9f7b0 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,12 @@ 
 
 *** Changes since GDB 7.10
 
+* GDB's printf command now has a format specifier (%q) to display the
+  textual label (enumerator) of an enumeration value.  For example:
+
+    (gdb) printf "Visiting node of type %q\n", node
+    Visiting node of type NODE_INTEGER
+
 * GDB now supports debugging kernel-based threads on FreeBSD.
 
 * Per-inferior thread numbers
diff --git a/gdb/common/format.c b/gdb/common/format.c
index fbb82fd..f90893e 100644
--- a/gdb/common/format.c
+++ b/gdb/common/format.c
@@ -294,6 +294,10 @@  parse_format_string (const char **arg)
 	  case '\0':
 	    error (_("Incomplete format specifier at end of format string"));
 
+	  case 'q':
+	    this_argclass = enum_arg;
+	    break;
+
 	  default:
 	    error (_("Unrecognized format specifier '%c' in printf"), *f);
 	  }
diff --git a/gdb/common/format.h b/gdb/common/format.h
index cb56a2c..884403e 100644
--- a/gdb/common/format.h
+++ b/gdb/common/format.h
@@ -35,7 +35,7 @@  enum argclass
     literal_piece,
     int_arg, long_arg, long_long_arg, ptr_arg,
     string_arg, wide_string_arg, wide_char_arg,
-    double_arg, long_double_arg, decfloat_arg
+    double_arg, long_double_arg, decfloat_arg, enum_arg
   };
 
 /* A format piece is a section of the format string that may include a
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2d09d13..183bd9e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -24343,6 +24343,9 @@  sequences, such as @code{\n}, @samp{\t}, @samp{\\}, @samp{\"},
 single character.  Octal and hexadecimal escape sequences are not
 supported.
 
+@code{printf} supports the @samp{q} conversion letter to print the textual
+label (enumerator) of a C enumeration value.
+
 Additionally, @code{printf} supports conversion specifications for DFP
 (@dfn{Decimal Floating Point}) types using the following length modifiers
 together with a floating point specifier.
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index f5c4211..e4637df 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2258,6 +2258,23 @@  printf_pointer (struct ui_file *stream, const char *format,
     }
 }
 
+static void
+printf_enum (struct ui_file *stream, struct value *value)
+{
+  struct type *type = check_typedef (value_type (value));
+
+  if (TYPE_CODE (type) == TYPE_CODE_ENUM)
+    {
+      val_print_enum_label (type, value_contents_for_printing (value),
+			    value_embedded_offset (value), stream);
+    }
+  else
+    {
+      error (_("\
+Value given for enum format specifier (%%q) is not of an enum type."));
+    }
+}
+
 /* printf "printf format string" ARG to STREAM.  */
 
 static void
@@ -2446,6 +2463,9 @@  ui_printf (const char *arg, struct ui_file *stream)
 	  case ptr_arg:
 	    printf_pointer (stream, current_substring, val_args[i]);
 	    break;
+	  case enum_arg:
+	    printf_enum (stream, val_args[i]);
+	    break;
 	  case literal_piece:
 	    /* Print a portion of the format string that has no
 	       directives.  Note that this will not include any
diff --git a/gdb/testsuite/gdb.base/printcmds.c b/gdb/testsuite/gdb.base/printcmds.c
index 57e04e6..bc7c981 100644
--- a/gdb/testsuite/gdb.base/printcmds.c
+++ b/gdb/testsuite/gdb.base/printcmds.c
@@ -90,7 +90,7 @@  struct some_arrays {
 
 struct some_arrays *parrays = &arrays;
 
-enum some_volatile_enum { enumvolval1, enumvolval2 };
+enum some_volatile_enum { enumvolval1, enumvolval2, enumvolval3, enumvolval4 };
 
 /* A volatile enum variable whose name is the same as the enumeration
    name.  See PR11827.  */
diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
index 3c78a53..4254581d 100644
--- a/gdb/testsuite/gdb.base/printcmds.exp
+++ b/gdb/testsuite/gdb.base/printcmds.exp
@@ -763,6 +763,16 @@  proc test_printf {} {
 	"" \
 	"create hibob command"
     gdb_test "hibob" "hi bob zzz.*y" "run hibob command"
+
+    # Test for enum printing (%q) in printf
+    gdb_test "printf \"%q\\n\", some_volatile_enum" "enumvolval1"
+    gdb_test "printf \"%q\\n\", enumvolval2" "enumvolval2"
+    gdb_test "printf \"%q\\n\", (enum some_volatile_enum) 1" "enumvolval2"
+    gdb_test "printf \"%q\\n\", (enum some_volatile_enum) 111" "111"
+    gdb_test "printf \"%q\\n\", three" "\\(ONE | TWO\\)"
+    gdb_test "printf \"%q\\n\", (enum flag_enum) 8" "\\(unknown: 8\\)"
+
+    gdb_test "printf \"%q\\n\", 2" "Value given for enum format specifier \\(%q\\) is not of an enum type."
 }
 
 #Test printing DFP values with printf
diff --git a/gdb/valprint.c b/gdb/valprint.c
index a9b03ec..fde48b4 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -526,35 +526,23 @@  generic_val_print_ref (struct type *type, const gdb_byte *valaddr,
     }
 }
 
-/* generic_val_print helper for TYPE_CODE_ENUM.  */
-
-static void
-generic_val_print_enum (struct type *type, const gdb_byte *valaddr,
-			int embedded_offset, struct ui_file *stream,
-			const struct value *original_value,
-			const struct value_print_options *options)
+void
+val_print_enum_label (struct type *type, const gdb_byte *valaddr,
+		      int embedded_offset, struct ui_file *stream)
 {
-  unsigned int i;
-  unsigned int len;
-  LONGEST val;
+  int len = TYPE_NFIELDS (type);
+  int i;
   struct gdbarch *gdbarch = get_type_arch (type);
   int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+  LONGEST val = unpack_long (type, valaddr + embedded_offset * unit_size);
 
-  if (options->format)
-    {
-      val_print_scalar_formatted (type, valaddr, embedded_offset,
-				  original_value, options, 0, stream);
-      return;
-    }
-  len = TYPE_NFIELDS (type);
-  val = unpack_long (type, valaddr + embedded_offset * unit_size);
   for (i = 0; i < len; i++)
     {
       QUIT;
       if (val == TYPE_FIELD_ENUMVAL (type, i))
-	{
-	  break;
-	}
+      {
+	break;
+      }
     }
   if (i < len)
     {
@@ -565,31 +553,31 @@  generic_val_print_enum (struct type *type, const gdb_byte *valaddr,
       int first = 1;
 
       /* We have a "flag" enum, so we try to decompose it into
-	 pieces as appropriate.  A flag enum has disjoint
-	 constants by definition.  */
+       pieces as appropriate.  A flag enum has disjoint
+       constants by definition.  */
       fputs_filtered ("(", stream);
       for (i = 0; i < len; ++i)
-	{
-	  QUIT;
+      {
+	QUIT;
 
-	  if ((val & TYPE_FIELD_ENUMVAL (type, i)) != 0)
-	    {
-	      if (!first)
-		fputs_filtered (" | ", stream);
-	      first = 0;
+	if ((val & TYPE_FIELD_ENUMVAL (type, i)) != 0)
+	  {
+	    if (!first)
+	      fputs_filtered (" | ", stream);
+	    first = 0;
 
-	      val &= ~TYPE_FIELD_ENUMVAL (type, i);
-	      fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
-	    }
-	}
+	    val &= ~TYPE_FIELD_ENUMVAL (type, i);
+	    fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
+	  }
+      }
 
       if (first || val != 0)
-	{
-	  if (!first)
-	    fputs_filtered (" | ", stream);
-	  fputs_filtered ("unknown: ", stream);
-	  print_longest (stream, 'd', 0, val);
-	}
+      {
+	if (!first)
+	  fputs_filtered (" | ", stream);
+	fputs_filtered ("unknown: ", stream);
+	print_longest (stream, 'd', 0, val);
+      }
 
       fputs_filtered (")", stream);
     }
@@ -597,6 +585,25 @@  generic_val_print_enum (struct type *type, const gdb_byte *valaddr,
     print_longest (stream, 'd', 0, val);
 }
 
+/* generic_val_print helper for TYPE_CODE_ENUM.  */
+
+static void
+generic_val_print_enum (struct type *type, const gdb_byte *valaddr,
+			int embedded_offset, struct ui_file *stream,
+			const struct value *original_value,
+			const struct value_print_options *options)
+{
+  if (options->format)
+    {
+      val_print_scalar_formatted (type, valaddr, embedded_offset,
+				  original_value, options, 0, stream);
+    }
+  else
+    {
+      val_print_enum_label (type, valaddr, embedded_offset, stream);
+    }
+}
+
 /* generic_val_print helper for TYPE_CODE_FLAGS.  */
 
 static void
diff --git a/gdb/valprint.h b/gdb/valprint.h
index bb03024..97a22d5 100644
--- a/gdb/valprint.h
+++ b/gdb/valprint.h
@@ -170,6 +170,9 @@  extern void val_print_unavailable (struct ui_file *stream);
 
 extern void val_print_invalid_address (struct ui_file *stream);
 
+extern void val_print_enum_label (struct type *type, const gdb_byte *valaddr,
+				  int embedded_offset, struct ui_file *stream);
+
 /* An instance of this is passed to generic_val_print and describes
    some language-specific ways to print things.  */