This is the mail archive of the archer@sourceware.org mailing list for the Archer project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[python] refactor pretty-printing, again


This patch refactors pretty-printing again.

Now, formatting is done in C rather than in Python.  This made it
simpler to handle the recursion case.  Recursing is needed, rather
than string-izing, to make indentation work out nicely in the 'pretty'
case.

I got rid of the value_print hook.  Perhaps this was overly eager of
me -- now we print references more verbosely.  Paul, let me know what
you think.  This is easily restored if needed.

Now we respect "set print pretty off".  Also, no we print some commas
and braces where we did not before.

I think the results look ok with either setting of "print pretty", but
we can tweak it more if not.

I updated the test suite a little.

I turned out not to need a ui_file wrapper in Python, but I wrote it
first :(.  I didn't include it in this patch, but if anybody needs it,
I can check it in.  Or maybe I will later anyhow.

Tom

2008-11-11  Tom Tromey  <tromey@redhat.com>

	* varobj.c (varobj_get_display_hint): Use gdbpy_get_display_hint.
	* python/python.c (apply_pretty_printer): Remove.
	(gdbpy_get_display_hint): New function.
	(print_string_repr): New function.
	(print_children): Likewise.
	(apply_val_pretty_printer): Rewrite.  Change return type.
	(_initialize_python) <_format_children>: Remove.
	(pretty_print_one_value): Remove 'children' argument.
	(apply_varobj_pretty_printer): Update.
	* python/python.h (apply_pretty_printer): Remove.
	(apply_val_pretty_printer): Update.
	* utils.c (fputs_indented): Remove.
	* defs.h (fputs_indented): Remove.
	* valprint.c (value_print): Don't pretty-print.
	(val_print): Update.
	* python/python-internal.h (gdbpy_get_display_hint): Declare.

2008-11-11  Tom Tromey  <tromey@redhat.com>

	* gdb.python/python-prettyprint.exp (run_lang_tests): Ensure "set
	print pretty" is on.  Update for recent changes.

diff --git a/gdb/defs.h b/gdb/defs.h
index 21f1daa..23a6599 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -446,8 +446,6 @@ extern struct ui_file *gdb_stdtargin;
 
 extern void fputs_filtered (const char *, struct ui_file *);
 
-extern void fputs_indented (const char *, struct ui_file *);
-
 extern void fputs_unfiltered (const char *, struct ui_file *);
 
 extern int fputc_filtered (int c, struct ui_file *);
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 71c2297..8f756c8 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -130,6 +130,7 @@ char *apply_varobj_pretty_printer (PyObject *print_obj, struct value *value,
 				   struct value **replacement);
 PyObject *gdbpy_get_varobj_pretty_printer (struct type *type);
 PyObject *gdbpy_instantiate_printer (PyObject *cons, struct value *value);
+char *gdbpy_get_display_hint (PyObject *printer);
 
 extern PyObject *gdbpy_children_cst;
 extern PyObject *gdbpy_to_string_cst;
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 9f4448b..206b4aa 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -748,12 +748,9 @@ find_pretty_printer (struct type *type)
    the function returns a string, an xmalloc()d copy is returned.
    Otherwise, if the function returns a value, a *OUT_VALUE is set to
    the value, and NULL is returned.  On error, *OUT_VALUE is set to
-   NULL and NULL is returned.  If CHILDREN is true, we may also try to
-   call an object's "children" method and format the output
-   accordingly.  */
+   NULL and NULL is returned.  */
 static char *
-pretty_print_one_value (PyObject *printer, struct value **out_value,
-			int children)
+pretty_print_one_value (PyObject *printer, struct value **out_value)
 {
   char *output = NULL;
   volatile struct gdb_exception except;
@@ -762,20 +759,7 @@ pretty_print_one_value (PyObject *printer, struct value **out_value,
     {
       PyObject *result;
 
-      /* If CHILDREN is true, call _format_children.  Otherwise, just
-	 try to call the object's to_string method.  */
-      if (children
-	  && PyObject_HasAttrString (gdb_module, "_format_children"))
-	{
-	  PyObject *fmt = PyObject_GetAttrString (gdb_module,
-						  "_format_children");
-	  result = PyObject_CallFunctionObjArgs (fmt, printer, NULL);
-	  Py_DECREF (fmt);
-	}
-      else
-	result = PyObject_CallMethodObjArgs (printer, gdbpy_to_string_cst,
-					     NULL);
-
+      result = PyObject_CallMethodObjArgs (printer, gdbpy_to_string_cst, NULL);
       if (result)
 	{
 	  if (gdbpy_is_string (result))
@@ -821,44 +805,172 @@ gdbpy_instantiate_printer (PyObject *cons, struct value *value)
   return result;
 }
 
-/* Try to pretty-print VALUE.  Return an xmalloc()d string
-   representation of the value.  If the result is NULL, and *OUT_VALUE
-   is set, then *OUT_VALUE is a value which should be printed in place
-   of VALUE.  *OUT_VALUE is not passed back to the pretty-printer.
-   Returns NULL and sets *OUT_VALUE to NULL on error or if no
-   pretty-printer was available.  */
+/* Return the display hint for the object printer, PRINTER.  Return
+   NULL if there is no display_hint method, or if the method did not
+   return a string.  On error, print stack trace and return NULL.  On
+   success, return an xmalloc()d string.  */
 char *
-apply_pretty_printer (struct value *value, struct value **out_value)
+gdbpy_get_display_hint (PyObject *printer)
 {
-  PyObject *func, *printer;
-  char *output = NULL;
+  PyObject *hint;
+  char *result = NULL;
 
-  *out_value = NULL;
-
-  /* Find the constructor.  */
-  func = find_pretty_printer (value_type (value));
-  if (! func)
+  if (! PyObject_HasAttr (printer, gdbpy_display_hint_cst))
     return NULL;
 
-  /* Instantiate it to get a printer object.  */
-  printer = gdbpy_instantiate_printer (func, value);
-  if (printer)
+  hint = PyObject_CallMethodObjArgs (printer, gdbpy_display_hint_cst, NULL);
+  if (gdbpy_is_string (hint))
+    result = python_string_to_host_string (hint);
+  if (hint)
+    Py_DECREF (hint);
+  else
+    gdbpy_print_stack ();
+
+  return result;
+}
+
+/* Helper for apply_val_pretty_printer which calls to_string and
+   formats the result.  */
+static void
+print_string_repr (PyObject *printer, struct ui_file *stream, int format,
+		   int deref_ref, int recurse,
+		   enum val_prettyprint pretty,
+		   const struct language_defn *language)
+{
+  char *output;
+  struct value *replacement = NULL;
+
+  output = pretty_print_one_value (printer, &replacement);
+  if (output)
     {
-      /* If instantiation returns None, then don't try to print.  */
-      if (printer != Py_None)
-	output = pretty_print_one_value (printer, out_value, 1);
-      Py_DECREF (printer);
+      fputs_filtered (output, stream);
+      xfree (output);
     }
-  Py_DECREF (func);
+  else if (replacement)
+    common_val_print (replacement, stream, format, deref_ref,
+		      recurse, pretty, language);
+  else
+    gdbpy_print_stack ();
+}
 
-  return output;
+/* Helper for apply_val_pretty_printer that formats children of the
+   printer, if any exist.  */
+static void
+print_children (PyObject *printer, struct ui_file *stream, int format,
+		int deref_ref, int recurse,
+		enum val_prettyprint pretty,
+		const struct language_defn *language)
+{
+  char *hint;
+  int is_map = 0, i;
+  PyObject *children, *iter;
+  struct cleanup *cleanups;
+
+  if (! PyObject_HasAttr (printer, gdbpy_children_cst))
+    return;
+
+  /* If we are printing a map, we want some special formatting.  */
+  hint = gdbpy_get_display_hint (printer);
+  if (hint)
+    {
+      is_map = ! strcmp (hint, "map");
+      xfree (hint);
+    }
+
+  children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
+					 NULL);
+  if (! children)
+    {
+      gdbpy_print_stack ();
+      return;
+    }
+
+  cleanups = make_cleanup_py_decref (children);
+
+  iter = PyObject_GetIter (children);
+  if (!iter)
+    {
+      gdbpy_print_stack ();
+      goto done;
+    }
+  make_cleanup_py_decref (iter);
+
+  for (i = 0; ; ++i)
+    {
+      PyObject *py_v, *item = PyIter_Next (iter);
+      char *name;
+      struct cleanup *inner_cleanup;
+
+      if (! item)
+	break;
+      inner_cleanup = make_cleanup_py_decref (item);
+
+      if (! PyArg_ParseTuple (item, "sO", &name, &py_v))
+	{
+	  gdbpy_print_stack ();
+	  continue;
+	}
+
+      if (i == 0)
+	fputs_filtered (" = {", stream);
+      else if (! is_map || i % 2 == 0)
+	fputs_filtered (pretty ? "," : ", ", stream);
+
+      if (pretty && (! is_map || i % 2 == 0))
+	{
+	  fputs_filtered ("\n", stream);
+	  print_spaces_filtered (2 + 2 * recurse, stream);
+	}
+
+      if (is_map && i % 2 == 0)
+	fputs_filtered ("[", stream);
+      else if (! is_map)
+	{
+	  fputs_filtered (name, stream);
+	  fputs_filtered (" = ", stream);
+	}
+
+      if (gdbpy_is_string (py_v))
+	{
+	  char *text = python_string_to_host_string (py_v);
+	  if (! text)
+	    gdbpy_print_stack ();
+	  else
+	    {
+	      fputs_filtered (text, stream);
+	      xfree (text);
+	    }
+	}
+      else
+	{
+	  struct value *value = convert_value_from_python (py_v);
+	  common_val_print (value, stream, format, deref_ref,
+			    recurse + 1, pretty, language);
+	}
+
+      if (is_map && i % 2 == 0)
+	fputs_filtered ("] = ", stream);
+      else if (! pretty)
+	wrap_here (n_spaces (2 + 2 * recurse));
+
+      do_cleanups (inner_cleanup);
+    }
+
+  if (i)
+    {
+      if (pretty)
+	{
+	  fputs_filtered ("\n", stream);
+	  print_spaces_filtered (2 * recurse, stream);
+	}
+      fputs_filtered ("}", stream);
+    }
+
+ done:
+  do_cleanups (cleanups);
 }
 
-/* Like apply_pretty_printer, but called from the 'val' (and not
-   'value') printing code.  Arguments are as for val_print.  Returns
-   an xmalloc()d pretty-printed string if a pretty-printer was found
-   and was successful.  */
-char *
+int
 apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
 			  int embedded_offset, CORE_ADDR address,
 			  struct ui_file *stream, int format,
@@ -867,13 +979,15 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
 			  const struct language_defn *language)
 {
   PyObject *func, *printer;
-  struct value *value, *replacement = NULL;
-  char *output = NULL;
+  struct value *value;
+  char *hint;
+  struct cleanup *cleanups;
+  int result = 0;
 
   /* Find the constructor.  */
   func = find_pretty_printer (type);
   if (! func)
-    return NULL;
+    return 0;
 
   /* Instantiate the printer.  */
   value = value_from_contents_and_address (type, valaddr, embedded_offset,
@@ -882,25 +996,24 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
   Py_DECREF (func);
 
   if (!printer)
-    return NULL;
+    {
+      gdbpy_print_stack ();
+      return 0;
+    }
 
+  cleanups = make_cleanup_py_decref (printer);
   if (printer != Py_None)
-    output = pretty_print_one_value (printer, &replacement, 1);
-  Py_DECREF (printer);
-  if (output)
-    return output;
-
-  if (! replacement)
-    return NULL;
+    {
+      print_string_repr (printer, stream, format, deref_ref,
+			 recurse, pretty, language);
+      print_children (printer, stream, format, deref_ref,
+		      recurse, pretty, language);
 
-  language->la_val_print (value_type (replacement),
-			  value_contents_all (replacement),
-			  value_embedded_offset (replacement),
-			  value_address (replacement),
-			  stream, format, deref_ref, recurse,
-			  pretty);
+      result = 1;
+    }
 
-  return xstrdup ("");
+  do_cleanups (cleanups);
+  return result;
 }
 
 /* Apply a pretty-printer for the varobj code.  PRINTER_OBJ is the
@@ -916,7 +1029,7 @@ apply_varobj_pretty_printer (PyObject *printer_obj, struct value *value,
 			     struct value **replacement)
 {
   *replacement = NULL;
-  return pretty_print_one_value (printer_obj, replacement, 0);
+  return pretty_print_one_value (printer_obj, replacement);
 }
 
 /* Find a pretty-printer object for the varobj module.  Returns a new
@@ -995,14 +1108,7 @@ eval_python_from_control_command (struct command_line *cmd)
   error (_("Python scripting is not supported in this copy of GDB."));
 }
 
-char *
-apply_pretty_printer (struct value *ignore, struct value **out)
-{
-  *out = NULL;
-  return NULL;
-}
-
-char *
+int
 apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
 			  int embedded_offset, CORE_ADDR address,
 			  struct ui_ifle *stream, int format,
@@ -1010,7 +1116,7 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
 			  enum val_prettyprint pretty,
 			  const language_defn *language)
 {
-  return NULL;
+  return 0;
 }
 
 void
@@ -1155,35 +1261,6 @@ sys.stderr = GdbOutputFile()\n\
 sys.stdout = GdbOutputFile()\n\
 ");
 
-  PyRun_SimpleString ("\
-def _format_children(obj):\n\
-  result = []\n\
-  if hasattr(obj, 'to_string'):\n\
-    result.append(str(obj.to_string()))\n\
-  if hasattr(obj, 'display_hint'):\n\
-    is_map = 'map' == obj.display_hint()\n\
-  else:\n\
-    is_map = False\n\
-  max = gdb.get_parameter('print elements')\n\
-  i = 0\n\
-  previous = None\n\
-  if hasattr(obj, 'children'):\n\
-    for elt in obj.children():\n\
-      (name, val) = elt\n\
-      if is_map:\n\
-	if i % 2 == 0:\n\
-	  previous = val\n\
-	else:\n\
-	  result.append('[%s] = %s' % (str (previous), str (val)))\n\
-      else:\n\
-	result.append('%s = %s' % (name, str(val)))\n\
-      i = i + 1\n\
-      if max != None and i == max:\n\
-	break\n\
-  return '\\n'.join(result)\n\
-\n\
-gdb._format_children = _format_children\n\
-");
 #endif /* HAVE_PYTHON */
 }
 
diff --git a/gdb/python/python.h b/gdb/python/python.h
index ef0560c..a2aec3f 100644
--- a/gdb/python/python.h
+++ b/gdb/python/python.h
@@ -30,13 +30,11 @@ void source_python_script (FILE *stream, char *file);
 
 void run_python_script (int argc, char **argv);
 
-char *apply_pretty_printer (struct value *, struct value **);
-
-char *apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
-				int embedded_offset, CORE_ADDR address,
-				struct ui_file *stream, int format,
-				int deref_ref, int recurse,
-				enum val_prettyprint pretty,
-				const struct language_defn *language);
+int apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
+			      int embedded_offset, CORE_ADDR address,
+			      struct ui_file *stream, int format,
+			      int deref_ref, int recurse,
+			      enum val_prettyprint pretty,
+			      const struct language_defn *language);
 
 #endif /* GDB_PYTHON_H */
diff --git a/gdb/testsuite/gdb.python/python-prettyprint.exp b/gdb/testsuite/gdb.python/python-prettyprint.exp
index d07d195..89e3878 100644
--- a/gdb/testsuite/gdb.python/python-prettyprint.exp
+++ b/gdb/testsuite/gdb.python/python-prettyprint.exp
@@ -42,6 +42,8 @@ proc run_lang_tests {lang} {
 	return -1
     }
 
+    set nl "\[\r\n\]+"
+
     # Start with a fresh gdb.
     gdb_exit
     gdb_start
@@ -54,6 +56,8 @@ proc run_lang_tests {lang} {
 	return
     }
 
+    gdb_test "set print pretty on" ""
+
     gdb_test "b [gdb_get_line_number {break to inspect} ${testfile}.c ]" \
 	".*Breakpoint.*"
     gdb_test "continue" ".*Breakpoint.*"
@@ -66,19 +70,18 @@ proc run_lang_tests {lang} {
     
     if {$lang == "c++"} {
 	gdb_test "print cps" "=  a=<8> b=<$hex>"
-	gdb_test "print cpss" " = {zss = 9, s =  a=<10> b=<$hex>}"
-	gdb_test "print cpssa\[0\]" " = {zss = 11, s =  a=<12> b=<$hex>}"
-	gdb_test "print cpssa\[1\]" " = {zss = 13, s =  a=<14> b=<$hex>}"
-	gdb_test "print cpssa" " = {{zss = 11, s =  a=<12> b=<$hex>}, {zss = 13, s =  a=<14> b=<$hex>}}"
+	gdb_test "print cpss" " = {$nl *zss = 9, *$nl *s =  a=<10> b=<$hex>$nl}"
+	gdb_test "print cpssa\[0\]" " = {$nl *zss = 11, *$nl *s =  a=<12> b=<$hex>$nl}"
+	gdb_test "print cpssa\[1\]" " = {$nl *zss = 13, *$nl *s =  a=<14> b=<$hex>$nl}"
+	gdb_test "print cpssa" " = {{$nl *zss = 11, *$nl *s =  a=<12> b=<$hex>$nl *}, {$nl *zss = 13, *$nl *s =  a=<14> b=<$hex>$nl *}}"
 	gdb_test "print sss" "= a=<15> b=< a=<8> b=<$hex>>"
-	gdb_test "print ref" "= a=<15> b=< a=<8> b=<$hex>>"
+	gdb_test "print ref" "= .SSS &. a=<15> b=< a=<8> b=<$hex>>"
     }
 
     gdb_test "print x" " = $hex \"this is x\""
     gdb_test "print cstring" " = $hex \"const string\""
 
-    set nl "\[\r\n\]+"
-    gdb_test "print c" " = container $hex \"container\" with 2 elements$nl *.0. = 23$nl *.1. = 72"
+    gdb_test "print c" " = container $hex \"container\" with 2 elements = {$nl *.0. = 23,$nl *.1. = 72$nl}"
 
     gdb_test "continue" "Program exited normally\."
 }
diff --git a/gdb/utils.c b/gdb/utils.c
index cbe93ed..ce05909 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2050,37 +2050,6 @@ fputs_filtered (const char *linebuffer, struct ui_file *stream)
   fputs_maybe_filtered (linebuffer, stream, 1);
 }
 
-/* Print TEXT to STREAM, using filtered output.  If TEXT wraps, indent
-   subsequent lines to the current output column at the time of the call to
-   this function.  */
-void
-fputs_indented (const char *text, struct ui_file *stream)
-{
-  const char *p;
-  char *save = wrap_indent;
-  unsigned int level = chars_printed;
-  int last_was_newline = 0;
-
-  wrap_indent = "";
-
-  for (p = text; *p; ++p)
-    {
-      if (last_was_newline && *p != '\n')
-	{
-	  /* Indent properly.  Note that we can't use n_spaces here,
-	     as a caller might have used it when calling wrap_here --
-	     so a call here would clobber the saved wrap_indent.  */
-	  unsigned int i;
-	  for (i = 0; i < level; ++i)
-	    fputc_filtered (' ', stream);
-	}
-      fputc_filtered (*p, stream);
-      last_was_newline = *p == '\n';
-    }
-
-  wrap_indent = save;
-}
-
 int
 putchar_unfiltered (int c)
 {
diff --git a/gdb/valprint.c b/gdb/valprint.c
index f49e5ab..c580f39 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -236,18 +236,12 @@ val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 
   if (!raw_printing)
     {
-      char *text;
-      text = apply_val_pretty_printer (type, valaddr, embedded_offset,
-				       address, stream, format,
-				       deref_ref, recurse, real_pretty,
-				       language);
-      if (text)
-	{
-	  fputs_indented (text, stream);
-	  ret = strlen (text);
-	  xfree (text);
-	  return ret;
-	}
+      ret = apply_val_pretty_printer (type, valaddr, embedded_offset,
+				      address, stream, format,
+				      deref_ref, recurse, real_pretty,
+				      language);
+      if (ret)
+	return ret;
     }
 
   TRY_CATCH (except, RETURN_MASK_ERROR)
@@ -332,21 +326,6 @@ value_print (struct value *val, struct ui_file *stream, int format,
     return 0;
 
   raw_printing = raw;
-  if (!raw)
-    {
-      struct value *replace = NULL;
-      char *output = apply_pretty_printer (val, &replace);
-      if (output)
-	{
-	  int len = strlen (output);
-	  fputs_indented (output, stream);
-	  xfree (output);
-	  return len;
-	}
-      else if (replace)
-	val = replace;
-    }
-
   return LA_VALUE_PRINT (val, stream, format, pretty);
 }
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 30a2e7a..e8e627f 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -772,20 +772,8 @@ varobj_get_display_hint (struct varobj *var)
   char *result = NULL;
 
 #if HAVE_PYTHON
-  PyObject *printer = var->pretty_printer;
-
-  if (printer && PyObject_HasAttr (printer, gdbpy_display_hint_cst))
-    {
-      PyObject *hint = PyObject_CallMethodObjArgs (printer,
-						   gdbpy_display_hint_cst,
-						   NULL);
-      if (gdbpy_is_string (hint))
-	result = python_string_to_host_string (hint);
-      if (hint)
-	Py_DECREF (hint);
-      else
-	PyErr_Clear ();
-    }
+  if (var->pretty_printer)
+    result = gdbpy_get_display_hint (var->pretty_printer);
 #endif
 
   return result;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]