public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [commit/Ada 03/11] ada_val_print_1: Go through val_print instead of recursive call to self.
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
                   ` (2 preceding siblings ...)
  2014-01-07  4:22 ` [commit/Ada 02/11] ada_val_print_1: Add language parameter Joel Brobecker
@ 2014-01-07  4:22 ` Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 07/11] rewrite ada_val_print_ref to reduce if/else block nesting depth Joel Brobecker
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:22 UTC (permalink / raw)
  To: gdb-patches

This is to standardize a little bit how printing is done, and in
particular make sure that everyone goes through val_print when
printing sub-objects.  This helps making sure that standard features
handled by val_print get activated when expected.

gdb/ChangeLog:

        * ada-valprint.c (ada_val_print_1): Replace calls to
        ada_val_print_1 by calls to val_print.
---
 gdb/ChangeLog      |  5 +++++
 gdb/ada-valprint.c | 23 +++++++++--------------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2866cae..ee0df63 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-valprint.c (ada_val_print_1): Replace calls to
+	ada_val_print_1 by calls to val_print.
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-valprint.c (ada_val_print_1): Add parameter "language".
 	Update calls to self accordingly.  Replace calls to c_val_print
 	by calls to val_print.
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 66cda39..ff0fa66 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -811,11 +811,9 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
 	  fprintf_filtered (stream, "0x0");
 	}
       else
-	ada_val_print_1 (value_type (val),
-			 value_contents_for_printing (val),
-			 value_embedded_offset (val),
-			 value_address (val), stream, recurse,
-			 val, options, language);
+	val_print (value_type (val), value_contents_for_printing (val),
+		   value_embedded_offset (val), value_address (val),
+		   stream, recurse, val, options, language);
       value_free_to_mark (mark);
       return;
     }
@@ -873,17 +871,14 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
 		= value_from_contents_and_address (type, valaddr + offset, 0);
 	      struct value *v = value_cast (target_type, v1);
 
-	      ada_val_print_1 (target_type,
-			       value_contents_for_printing (v),
-			       value_embedded_offset (v), 0,
-			       stream, recurse + 1, v, options,
-			       language);
+	      val_print (target_type, value_contents_for_printing (v),
+			 value_embedded_offset (v), 0, stream,
+			 recurse + 1, v, options, language);
 	    }
 	  else
-	    ada_val_print_1 (TYPE_TARGET_TYPE (type),
-			     valaddr, offset,
-			     address, stream, recurse,
-			     original_value, options, language);
+	    val_print (TYPE_TARGET_TYPE (type), valaddr, offset,
+		       address, stream, recurse, original_value,
+		       options, language);
 	  return;
 	}
       else
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [commit/Ada 09/11] Extract string-printing out of ada_val_print_array
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
                   ` (7 preceding siblings ...)
  2014-01-07  4:22 ` [commit/Ada 04/11] Remove call to gdb_flush at end of ada_val_print_1 Joel Brobecker
@ 2014-01-07  4:22 ` Joel Brobecker
  2014-01-07  4:23 ` [commit/Ada 11/11] Ada: Fix missing call to pretty-printer for fields of records Joel Brobecker
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:22 UTC (permalink / raw)
  To: gdb-patches

This patch creates a new function called "ada_val_print_string"
whose code is directly extracted out of ada_val_print_array.
The extracted code is then replaced by a call to this new function,
followed by a "return". The return avoids the need for an "else"
branch, with the associated block nesting. The latter is not really
terrible in this case, but it seems more readable this way.

gdb/ChangeLog:

        * ada-valprint.c (ada_val_print_string): New function,
        extracted from ada_val_print_array.
        (ada_val_print_array): Replace extracted code by call
        to ada_val_print_string followed by a return.  Move
        "else" branch to the function's top block.
---
 gdb/ChangeLog      |   8 ++++
 gdb/ada-valprint.c | 113 ++++++++++++++++++++++++++++++-----------------------
 2 files changed, 72 insertions(+), 49 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 004709b..e668617 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-valprint.c (ada_val_print_string): New function,
+	extracted from ada_val_print_array.
+	(ada_val_print_array): Replace extracted code by call
+	to ada_val_print_string followed by a return.  Move
+	"else" branch to the function's top block.
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-valprint.c (ada_val_print_array): Move implementation
 	down.  Rename parameter "offset" and "val" into "offset_aligned"
 	and "original_value" respectively.  Add parameter "offset".
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 43e1490..d1c8553 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -685,6 +685,54 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
   return comma_needed;
 }
 
+/* Implement Ada val_print'ing for the case where TYPE is
+   a TYPE_CODE_ARRAY of characters.  */
+
+static void
+ada_val_print_string (struct type *type, const gdb_byte *valaddr,
+		      int offset, int offset_aligned, CORE_ADDR address,
+		      struct ui_file *stream, int recurse,
+		      const struct value *original_value,
+		      const struct value_print_options *options)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+  struct type *elttype = TYPE_TARGET_TYPE (type);
+  unsigned int eltlen;
+  unsigned int len;
+
+  /* We know that ELTTYPE cannot possibly be null, because we assume
+     that we're called only when TYPE is a string-like type.
+     Similarly, the size of ELTTYPE should also be non-null, since
+     it's a character-like type.  */
+  gdb_assert (elttype != NULL);
+  gdb_assert (TYPE_LENGTH (elttype) != 0);
+
+  eltlen = TYPE_LENGTH (elttype);
+  len = TYPE_LENGTH (type) / eltlen;
+
+  if (options->prettyformat_arrays)
+    print_spaces_filtered (2 + 2 * recurse, stream);
+
+  /* If requested, look for the first null char and only print
+     elements up to it.  */
+  if (options->stop_print_at_null)
+    {
+      int temp_len;
+
+      /* Look for a NULL char.  */
+      for (temp_len = 0;
+	   (temp_len < len
+	    && temp_len < options->print_max
+	    && char_at (valaddr + offset_aligned,
+			temp_len, eltlen, byte_order) != 0);
+	   temp_len += 1);
+      len = temp_len;
+    }
+
+  printstr (stream, elttype, valaddr + offset_aligned, len, 0,
+	    eltlen, options);
+}
+
 /* Implement Ada val_print-ing for GNAT arrays (Eg. fat pointers,
    thin pointers, etc).  */
 
@@ -943,60 +991,27 @@ ada_val_print_array (struct type *type, const gdb_byte *valaddr,
 		     const struct value *original_value,
 		     const struct value_print_options *options)
 {
-  /* For an array of chars, print with string syntax.  */
+  /* For an array of characters, print with string syntax.  */
   if (ada_is_string_type (type)
       && (options->format == 0 || options->format == 's'))
     {
-      enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
-      struct type *elttype = TYPE_TARGET_TYPE (type);
-      unsigned int eltlen;
-      unsigned int len;
-
-      /* We know that ELTTYPE cannot possibly be null, because we found
-	 that TYPE is a string-like type.  Similarly, the size of ELTTYPE
-	 should also be non-null, since it's a character-like type.  */
-      gdb_assert (elttype != NULL);
-      gdb_assert (TYPE_LENGTH (elttype) != 0);
-
-      eltlen = TYPE_LENGTH (elttype);
-      len = TYPE_LENGTH (type) / eltlen;
-
-      if (options->prettyformat_arrays)
-        print_spaces_filtered (2 + 2 * recurse, stream);
-
-      /* If requested, look for the first null char and only print
-         elements up to it.  */
-      if (options->stop_print_at_null)
-        {
-          int temp_len;
-
-          /* Look for a NULL char.  */
-          for (temp_len = 0;
-               (temp_len < len
-                && temp_len < options->print_max
-                && char_at (valaddr + offset_aligned,
-			    temp_len, eltlen, byte_order) != 0);
-               temp_len += 1);
-          len = temp_len;
-        }
-
-      printstr (stream, elttype, valaddr + offset_aligned, len, 0,
-		eltlen, options);
+      ada_val_print_string (type, valaddr, offset, offset_aligned,
+			    address, stream, recurse, original_value,
+			    options);
+      return;
     }
+
+  fprintf_filtered (stream, "(");
+  print_optional_low_bound (stream, type, options);
+  if (TYPE_FIELD_BITSIZE (type, 0) > 0)
+    val_print_packed_array_elements (type, valaddr, offset_aligned,
+				     0, stream, recurse,
+				     original_value, options);
   else
-    {
-      fprintf_filtered (stream, "(");
-      print_optional_low_bound (stream, type, options);
-      if (TYPE_FIELD_BITSIZE (type, 0) > 0)
-        val_print_packed_array_elements (type, valaddr, offset_aligned,
-					 0, stream, recurse,
-					 original_value, options);
-      else
-        val_print_array_elements (type, valaddr, offset_aligned, address,
-				  stream, recurse, original_value,
-				  options, 0);
-      fprintf_filtered (stream, ")");
-    }
+    val_print_array_elements (type, valaddr, offset_aligned, address,
+			      stream, recurse, original_value,
+			      options, 0);
+  fprintf_filtered (stream, ")");
 }
 
 /* Implement Ada val_print'ing for the case where TYPE is
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [commit/Ada 01/11] ada-valprint.c: Reorder functions to reduce advance declarations.
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
@ 2014-01-07  4:22 ` Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 08/11] move ada_val_print_array down within other ada_val_print* functions Joel Brobecker
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:22 UTC (permalink / raw)
  To: gdb-patches

Advance function declarations add to the maintenance cost, since
any update to the function prototype needs to be made twice.
For static functions, this is not necessary, and this patch
reorders the function so as to reduce the use of such advanche
declarations.

gdb/ChangeLog:

        * ada-valprint.c (print_record): Delete declaration.
        (adjust_type_signedness, ada_val_print_1): Likewise.
        (ada_val_print): Move function implementation down.
        (print_variant_part, print_field_values, print_record):
        Move function implementation up.
---
 gdb/ChangeLog      |   8 ++
 gdb/ada-valprint.c | 385 ++++++++++++++++++++++++++---------------------------
 2 files changed, 194 insertions(+), 199 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 13d2e16..64d1e6b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-valprint.c (print_record): Delete declaration.
+	(adjust_type_signedness, ada_val_print_1): Likewise.
+	(ada_val_print): Move function implementation down.
+	(print_variant_part, print_field_values, print_record):
+	Move function implementation up.
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* python/py-type.c (typy_get_name): New function.
 	(type_object_getset): Add entry for attribute "name".
 	* NEWS: Add entry mentioning this new attribute.
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index bef5ff9..5162737 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -34,25 +34,12 @@
 #include "exceptions.h"
 #include "objfiles.h"
 
-static void print_record (struct type *, const gdb_byte *, int,
-			  struct ui_file *,
-			  int,
-			  const struct value *,
-			  const struct value_print_options *);
-
 static int print_field_values (struct type *, const gdb_byte *,
 			       int,
 			       struct ui_file *, int,
 			       const struct value *,
 			       const struct value_print_options *,
 			       int, struct type *, int);
-
-static void adjust_type_signedness (struct type *);
-
-static void ada_val_print_1 (struct type *, const gdb_byte *, int, CORE_ADDR,
-			     struct ui_file *, int,
-			     const struct value *,
-			     const struct value_print_options *);
 \f
 
 /* Make TYPE unsigned if its range of values includes no negatives.  */
@@ -551,26 +538,6 @@ ada_printstr (struct ui_file *stream, struct type *type,
 }
 
 
-/* See val_print for a description of the various parameters of this
-   function; they are identical.  */
-
-void
-ada_val_print (struct type *type, const gdb_byte *valaddr,
-	       int embedded_offset, CORE_ADDR address,
-	       struct ui_file *stream, int recurse,
-	       const struct value *val,
-	       const struct value_print_options *options)
-{
-  volatile struct gdb_exception except;
-
-  /* XXX: this catches QUIT/ctrl-c as well.  Isn't that busted?  */
-  TRY_CATCH (except, RETURN_MASK_ALL)
-    {
-      ada_val_print_1 (type, valaddr, embedded_offset, address,
-		       stream, recurse, val, options);
-    }
-}
-
 /* Assuming TYPE is a simple array, print the value of this array located
    at VALADDR + OFFSET.  See ada_val_print for a description of the various
    parameters of this function; they are identical.  */
@@ -635,6 +602,176 @@ ada_val_print_array (struct type *type, const gdb_byte *valaddr,
     }
 }
 
+static int
+print_variant_part (struct type *type, int field_num,
+		    const gdb_byte *valaddr, int offset,
+		    struct ui_file *stream, int recurse,
+		    const struct value *val,
+		    const struct value_print_options *options,
+		    int comma_needed,
+		    struct type *outer_type, int outer_offset)
+{
+  struct type *var_type = TYPE_FIELD_TYPE (type, field_num);
+  int which = ada_which_variant_applies (var_type, outer_type,
+					 valaddr + outer_offset);
+
+  if (which < 0)
+    return 0;
+  else
+    return print_field_values
+      (TYPE_FIELD_TYPE (var_type, which),
+       valaddr,
+       offset + TYPE_FIELD_BITPOS (type, field_num) / HOST_CHAR_BIT
+       + TYPE_FIELD_BITPOS (var_type, which) / HOST_CHAR_BIT,
+       stream, recurse, val, options,
+       comma_needed, outer_type, outer_offset);
+}
+
+/* Print out fields of value at VALADDR + OFFSET having structure type TYPE.
+
+   TYPE, VALADDR, OFFSET, STREAM, RECURSE, and OPTIONS have the same
+   meanings as in ada_print_value and ada_val_print.
+
+   OUTER_TYPE and OUTER_OFFSET give type and address of enclosing
+   record (used to get discriminant values when printing variant
+   parts).
+
+   COMMA_NEEDED is 1 if fields have been printed at the current recursion
+   level, so that a comma is needed before any field printed by this
+   call.
+
+   Returns 1 if COMMA_NEEDED or any fields were printed.  */
+
+static int
+print_field_values (struct type *type, const gdb_byte *valaddr,
+		    int offset, struct ui_file *stream, int recurse,
+		    const struct value *val,
+		    const struct value_print_options *options,
+		    int comma_needed,
+		    struct type *outer_type, int outer_offset)
+{
+  int i, len;
+
+  len = TYPE_NFIELDS (type);
+
+  for (i = 0; i < len; i += 1)
+    {
+      if (ada_is_ignored_field (type, i))
+	continue;
+
+      if (ada_is_wrapper_field (type, i))
+	{
+	  comma_needed =
+	    print_field_values (TYPE_FIELD_TYPE (type, i),
+				valaddr,
+				(offset
+				 + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
+				stream, recurse, val, options,
+				comma_needed, type, offset);
+	  continue;
+	}
+      else if (ada_is_variant_part (type, i))
+	{
+	  comma_needed =
+	    print_variant_part (type, i, valaddr,
+				offset, stream, recurse, val,
+				options, comma_needed,
+				outer_type, outer_offset);
+	  continue;
+	}
+
+      if (comma_needed)
+	fprintf_filtered (stream, ", ");
+      comma_needed = 1;
+
+      if (options->prettyformat)
+	{
+	  fprintf_filtered (stream, "\n");
+	  print_spaces_filtered (2 + 2 * recurse, stream);
+	}
+      else
+	{
+	  wrap_here (n_spaces (2 + 2 * recurse));
+	}
+
+      annotate_field_begin (TYPE_FIELD_TYPE (type, i));
+      fprintf_filtered (stream, "%.*s",
+			ada_name_prefix_len (TYPE_FIELD_NAME (type, i)),
+			TYPE_FIELD_NAME (type, i));
+      annotate_field_name_end ();
+      fputs_filtered (" => ", stream);
+      annotate_field_value ();
+
+      if (TYPE_FIELD_PACKED (type, i))
+	{
+	  struct value *v;
+
+	  /* Bitfields require special handling, especially due to byte
+	     order problems.  */
+	  if (HAVE_CPLUS_STRUCT (type) && TYPE_FIELD_IGNORE (type, i))
+	    {
+	      fputs_filtered (_("<optimized out or zero length>"), stream);
+	    }
+	  else
+	    {
+	      int bit_pos = TYPE_FIELD_BITPOS (type, i);
+	      int bit_size = TYPE_FIELD_BITSIZE (type, i);
+	      struct value_print_options opts;
+
+	      adjust_type_signedness (TYPE_FIELD_TYPE (type, i));
+	      v = ada_value_primitive_packed_val
+		    (NULL, valaddr,
+		     offset + bit_pos / HOST_CHAR_BIT,
+		     bit_pos % HOST_CHAR_BIT,
+		     bit_size, TYPE_FIELD_TYPE (type, i));
+	      opts = *options;
+	      opts.deref_ref = 0;
+	      val_print (TYPE_FIELD_TYPE (type, i),
+			 value_contents_for_printing (v),
+			 value_embedded_offset (v), 0,
+			 stream, recurse + 1, v,
+			 &opts, current_language);
+	    }
+	}
+      else
+	{
+	  struct value_print_options opts = *options;
+
+	  opts.deref_ref = 0;
+	  ada_val_print (TYPE_FIELD_TYPE (type, i),
+			 valaddr,
+			 (offset
+			  + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
+			 0, stream, recurse + 1, val, &opts);
+	}
+      annotate_field_end ();
+    }
+
+  return comma_needed;
+}
+
+static void
+print_record (struct type *type, const gdb_byte *valaddr,
+	      int offset,
+	      struct ui_file *stream, int recurse,
+	      const struct value *val,
+	      const struct value_print_options *options)
+{
+  type = ada_check_typedef (type);
+
+  fprintf_filtered (stream, "(");
+
+  if (print_field_values (type, valaddr, offset,
+			  stream, recurse, val, options,
+			  0, type, offset) != 0 && options->prettyformat)
+    {
+      fprintf_filtered (stream, "\n");
+      print_spaces_filtered (2 * recurse, stream);
+    }
+
+  fprintf_filtered (stream, ")");
+}
+
 /* See the comment on ada_val_print.  This function differs in that it
    does not catch evaluation errors (leaving that to ada_val_print).  */
 
@@ -913,29 +1050,24 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
   gdb_flush (stream);
 }
 
-static int
-print_variant_part (struct type *type, int field_num,
-		    const gdb_byte *valaddr, int offset,
-		    struct ui_file *stream, int recurse,
-		    const struct value *val,
-		    const struct value_print_options *options,
-		    int comma_needed,
-		    struct type *outer_type, int outer_offset)
+/* See val_print for a description of the various parameters of this
+   function; they are identical.  */
+
+void
+ada_val_print (struct type *type, const gdb_byte *valaddr,
+	       int embedded_offset, CORE_ADDR address,
+	       struct ui_file *stream, int recurse,
+	       const struct value *val,
+	       const struct value_print_options *options)
 {
-  struct type *var_type = TYPE_FIELD_TYPE (type, field_num);
-  int which = ada_which_variant_applies (var_type, outer_type,
-					 valaddr + outer_offset);
+  volatile struct gdb_exception except;
 
-  if (which < 0)
-    return 0;
-  else
-    return print_field_values
-      (TYPE_FIELD_TYPE (var_type, which),
-       valaddr,
-       offset + TYPE_FIELD_BITPOS (type, field_num) / HOST_CHAR_BIT
-       + TYPE_FIELD_BITPOS (var_type, which) / HOST_CHAR_BIT,
-       stream, recurse, val, options,
-       comma_needed, outer_type, outer_offset);
+  /* XXX: this catches QUIT/ctrl-c as well.  Isn't that busted?  */
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      ada_val_print_1 (type, valaddr, embedded_offset, address,
+		       stream, recurse, val, options);
+    }
 }
 
 void
@@ -987,148 +1119,3 @@ ada_value_print (struct value *val0, struct ui_file *stream,
 	     value_embedded_offset (val), address,
 	     stream, 0, val, &opts, current_language);
 }
-
-static void
-print_record (struct type *type, const gdb_byte *valaddr,
-	      int offset,
-	      struct ui_file *stream, int recurse,
-	      const struct value *val,
-	      const struct value_print_options *options)
-{
-  type = ada_check_typedef (type);
-
-  fprintf_filtered (stream, "(");
-
-  if (print_field_values (type, valaddr, offset,
-			  stream, recurse, val, options,
-			  0, type, offset) != 0 && options->prettyformat)
-    {
-      fprintf_filtered (stream, "\n");
-      print_spaces_filtered (2 * recurse, stream);
-    }
-
-  fprintf_filtered (stream, ")");
-}
-
-/* Print out fields of value at VALADDR + OFFSET having structure type TYPE.
-
-   TYPE, VALADDR, OFFSET, STREAM, RECURSE, and OPTIONS have the same
-   meanings as in ada_print_value and ada_val_print.
-
-   OUTER_TYPE and OUTER_OFFSET give type and address of enclosing
-   record (used to get discriminant values when printing variant
-   parts).
-
-   COMMA_NEEDED is 1 if fields have been printed at the current recursion
-   level, so that a comma is needed before any field printed by this
-   call.
-
-   Returns 1 if COMMA_NEEDED or any fields were printed.  */
-
-static int
-print_field_values (struct type *type, const gdb_byte *valaddr,
-		    int offset, struct ui_file *stream, int recurse,
-		    const struct value *val,
-		    const struct value_print_options *options,
-		    int comma_needed,
-		    struct type *outer_type, int outer_offset)
-{
-  int i, len;
-
-  len = TYPE_NFIELDS (type);
-
-  for (i = 0; i < len; i += 1)
-    {
-      if (ada_is_ignored_field (type, i))
-	continue;
-
-      if (ada_is_wrapper_field (type, i))
-	{
-	  comma_needed =
-	    print_field_values (TYPE_FIELD_TYPE (type, i),
-				valaddr,
-				(offset
-				 + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
-				stream, recurse, val, options,
-				comma_needed, type, offset);
-	  continue;
-	}
-      else if (ada_is_variant_part (type, i))
-	{
-	  comma_needed =
-	    print_variant_part (type, i, valaddr,
-				offset, stream, recurse, val,
-				options, comma_needed,
-				outer_type, outer_offset);
-	  continue;
-	}
-
-      if (comma_needed)
-	fprintf_filtered (stream, ", ");
-      comma_needed = 1;
-
-      if (options->prettyformat)
-	{
-	  fprintf_filtered (stream, "\n");
-	  print_spaces_filtered (2 + 2 * recurse, stream);
-	}
-      else
-	{
-	  wrap_here (n_spaces (2 + 2 * recurse));
-	}
-
-      annotate_field_begin (TYPE_FIELD_TYPE (type, i));
-      fprintf_filtered (stream, "%.*s",
-			ada_name_prefix_len (TYPE_FIELD_NAME (type, i)),
-			TYPE_FIELD_NAME (type, i));
-      annotate_field_name_end ();
-      fputs_filtered (" => ", stream);
-      annotate_field_value ();
-
-      if (TYPE_FIELD_PACKED (type, i))
-	{
-	  struct value *v;
-
-	  /* Bitfields require special handling, especially due to byte
-	     order problems.  */
-	  if (HAVE_CPLUS_STRUCT (type) && TYPE_FIELD_IGNORE (type, i))
-	    {
-	      fputs_filtered (_("<optimized out or zero length>"), stream);
-	    }
-	  else
-	    {
-	      int bit_pos = TYPE_FIELD_BITPOS (type, i);
-	      int bit_size = TYPE_FIELD_BITSIZE (type, i);
-	      struct value_print_options opts;
-
-	      adjust_type_signedness (TYPE_FIELD_TYPE (type, i));
-	      v = ada_value_primitive_packed_val
-		    (NULL, valaddr,
-		     offset + bit_pos / HOST_CHAR_BIT,
-		     bit_pos % HOST_CHAR_BIT,
-		     bit_size, TYPE_FIELD_TYPE (type, i));
-	      opts = *options;
-	      opts.deref_ref = 0;
-	      val_print (TYPE_FIELD_TYPE (type, i),
-			 value_contents_for_printing (v),
-			 value_embedded_offset (v), 0,
-			 stream, recurse + 1, v,
-			 &opts, current_language);
-	    }
-	}
-      else
-	{
-	  struct value_print_options opts = *options;
-
-	  opts.deref_ref = 0;
-	  ada_val_print (TYPE_FIELD_TYPE (type, i),
-			 valaddr,
-			 (offset
-			  + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
-			 0, stream, recurse + 1, val, &opts);
-	}
-      annotate_field_end ();
-    }
-
-  return comma_needed;
-}
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [commit/Ada 05/11] Split ada_val_print_1 into smaller functions
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
                   ` (4 preceding siblings ...)
  2014-01-07  4:22 ` [commit/Ada 07/11] rewrite ada_val_print_ref to reduce if/else block nesting depth Joel Brobecker
@ 2014-01-07  4:22 ` Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 06/11] ada-valprint.c: Inline print_record inside ada_val_print_struct_union Joel Brobecker
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:22 UTC (permalink / raw)
  To: gdb-patches

The idea of this patch is that it's hard to have a global view of
ada_val_print_1 because its body spans over too many lines. Also,
each individual "case" block within the giant "switch" can be hard
to isolate if spanning over multiple pages as well.

gdb/ChangeLog:

        * ada-valprint.c (ada_val_print_gnat_array): New function,
        extracted from ada_val_print_1;
        (ada_val_print_ptr, ada_val_print_num, ada_val_print_enum)
        (ada_val_print_flt, ada_val_print_struct_union)
        (ada_val_print_ref): Likewise.
        (ada_val_print_1): Delete variables i and elttype.
        Replace extracted-out code by call to corresponding
        new functions.
---
 gdb/ChangeLog      |  11 ++
 gdb/ada-valprint.c | 539 +++++++++++++++++++++++++++++++----------------------
 2 files changed, 332 insertions(+), 218 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f8820a5..4be753d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,16 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-valprint.c (ada_val_print_gnat_array): New function,
+	extracted from ada_val_print_1;
+	(ada_val_print_ptr, ada_val_print_num, ada_val_print_enum)
+	(ada_val_print_flt, ada_val_print_struct_union)
+	(ada_val_print_ref): Likewise.
+	(ada_val_print_1): Delete variables i and elttype.
+	Replace extracted-out code by call to corresponding
+	new functions.
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-valprint.c (ada_val_print_1): Remove call to gdb_flush.
 
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 9c60a79..1111d90 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -772,6 +772,302 @@ print_record (struct type *type, const gdb_byte *valaddr,
   fprintf_filtered (stream, ")");
 }
 
+/* Implement Ada val_print-ing for GNAT arrays (Eg. fat pointers,
+   thin pointers, etc).  */
+
+static void
+ada_val_print_gnat_array (struct type *type, const gdb_byte *valaddr,
+			  int offset, CORE_ADDR address,
+			  struct ui_file *stream, int recurse,
+			  const struct value *original_value,
+			  const struct value_print_options *options,
+			  const struct language_defn *language)
+{
+  struct value *mark = value_mark ();
+  struct value *val;
+
+  val = value_from_contents_and_address (type, valaddr + offset, address);
+  /* If this is a reference, coerce it now.  This helps taking care
+     of the case where ADDRESS is meaningless because original_value
+     was not an lval.  */
+  val = coerce_ref (val);
+  if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF)  /* array access type.  */
+    val = ada_coerce_to_simple_array_ptr (val);
+  else
+    val = ada_coerce_to_simple_array (val);
+  if (val == NULL)
+    {
+      gdb_assert (TYPE_CODE (type) == TYPE_CODE_TYPEDEF);
+      fprintf_filtered (stream, "0x0");
+    }
+  else
+    val_print (value_type (val), value_contents_for_printing (val),
+	       value_embedded_offset (val), value_address (val),
+	       stream, recurse, val, options, language);
+  value_free_to_mark (mark);
+}
+
+/* Implement Ada val_print'ing for the case where TYPE is
+   a TYPE_CODE_PTR.  */
+
+static void
+ada_val_print_ptr (struct type *type, const gdb_byte *valaddr,
+		   int offset, int offset_aligned, CORE_ADDR address,
+		   struct ui_file *stream, int recurse,
+		   const struct value *original_value,
+		   const struct value_print_options *options,
+		   const struct language_defn *language)
+{
+  val_print (type, valaddr, offset, address, stream, recurse,
+	     original_value, options, language_def (language_c));
+
+  if (ada_is_tag_type (type))
+    {
+      struct value *val =
+	value_from_contents_and_address (type,
+					 valaddr + offset_aligned,
+					 address + offset_aligned);
+      const char *name = ada_tag_name (val);
+
+      if (name != NULL)
+	fprintf_filtered (stream, " (%s)", name);
+    }
+}
+
+/* Implement Ada val_print'ing for the case where TYPE is
+   a TYPE_CODE_INT or TYPE_CODE_RANGE.  */
+
+static void
+ada_val_print_num (struct type *type, const gdb_byte *valaddr,
+		   int offset, int offset_aligned, CORE_ADDR address,
+		   struct ui_file *stream, int recurse,
+		   const struct value *original_value,
+		   const struct value_print_options *options,
+		   const struct language_defn *language)
+{
+  if (ada_is_fixed_point_type (type))
+    {
+      LONGEST v = unpack_long (type, valaddr + offset_aligned);
+
+      fprintf_filtered (stream, TYPE_LENGTH (type) < 4 ? "%.11g" : "%.17g",
+			(double) ada_fixed_to_float (type, v));
+      return;
+    }
+  else if (TYPE_CODE (type) == TYPE_CODE_RANGE)
+    {
+      struct type *target_type = TYPE_TARGET_TYPE (type);
+
+      if (TYPE_LENGTH (type) != TYPE_LENGTH (target_type))
+	{
+	  /* Obscure case of range type that has different length from
+	     its base type.  Perform a conversion, or we will get a
+	     nonsense value.  Actually, we could use the same
+	     code regardless of lengths; I'm just avoiding a cast.  */
+	  struct value *v1
+	    = value_from_contents_and_address (type, valaddr + offset, 0);
+	  struct value *v = value_cast (target_type, v1);
+
+	  val_print (target_type, value_contents_for_printing (v),
+		     value_embedded_offset (v), 0, stream,
+		     recurse + 1, v, options, language);
+	}
+      else
+	val_print (TYPE_TARGET_TYPE (type), valaddr, offset,
+		   address, stream, recurse, original_value,
+		   options, language);
+      return;
+    }
+  else
+    {
+      int format = (options->format ? options->format
+		    : options->output_format);
+
+      if (format)
+	{
+	  struct value_print_options opts = *options;
+
+	  opts.format = format;
+	  val_print_scalar_formatted (type, valaddr, offset_aligned,
+				      original_value, &opts, 0, stream);
+	}
+      else if (ada_is_system_address_type (type))
+	{
+	  /* FIXME: We want to print System.Address variables using
+	     the same format as for any access type.  But for some
+	     reason GNAT encodes the System.Address type as an int,
+	     so we have to work-around this deficiency by handling
+	     System.Address values as a special case.  */
+
+	  struct gdbarch *gdbarch = get_type_arch (type);
+	  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+	  CORE_ADDR addr = extract_typed_address (valaddr + offset_aligned,
+						  ptr_type);
+
+	  fprintf_filtered (stream, "(");
+	  type_print (type, "", stream, -1);
+	  fprintf_filtered (stream, ") ");
+	  fputs_filtered (paddress (gdbarch, addr), stream);
+	}
+      else
+	{
+	  val_print_type_code_int (type, valaddr + offset_aligned, stream);
+	  if (ada_is_character_type (type))
+	    {
+	      LONGEST c;
+
+	      fputs_filtered (" ", stream);
+	      c = unpack_long (type, valaddr + offset_aligned);
+	      ada_printchar (c, type, stream);
+	    }
+	}
+      return;
+    }
+}
+
+/* Implement Ada val_print'ing for the case where TYPE is
+   a TYPE_CODE_ENUM.  */
+
+static void
+ada_val_print_enum (struct type *type, const gdb_byte *valaddr,
+		    int offset, int offset_aligned, CORE_ADDR address,
+		    struct ui_file *stream, int recurse,
+		    const struct value *original_value,
+		    const struct value_print_options *options,
+		    const struct language_defn *language)
+{
+  int i;
+  unsigned int len;
+  LONGEST val;
+
+  if (options->format)
+    {
+      val_print_scalar_formatted (type, valaddr, offset_aligned,
+				  original_value, options, 0, stream);
+      return;
+    }
+
+  len = TYPE_NFIELDS (type);
+  val = unpack_long (type, valaddr + offset_aligned);
+  for (i = 0; i < len; i++)
+    {
+      QUIT;
+      if (val == TYPE_FIELD_ENUMVAL (type, i))
+	break;
+    }
+
+  if (i < len)
+    {
+      const char *name = ada_enum_name (TYPE_FIELD_NAME (type, i));
+
+      if (name[0] == '\'')
+	fprintf_filtered (stream, "%ld %s", (long) val, name);
+      else
+	fputs_filtered (name, stream);
+    }
+  else
+    print_longest (stream, 'd', 0, val);
+}
+
+/* Implement Ada val_print'ing for the case where TYPE is
+   a TYPE_CODE_FLT.  */
+
+static void
+ada_val_print_flt (struct type *type, const gdb_byte *valaddr,
+		   int offset, int offset_aligned, CORE_ADDR address,
+		   struct ui_file *stream, int recurse,
+		   const struct value *original_value,
+		   const struct value_print_options *options,
+		   const struct language_defn *language)
+{
+  if (options->format)
+    {
+      val_print (type, valaddr, offset, address, stream, recurse,
+		 original_value, options, language_def (language_c));
+      return;
+    }
+
+  ada_print_floating (valaddr + offset, type, stream);
+}
+
+/* Implement Ada val_print'ing for the case where TYPE is
+   a TYPE_CODE_STRUCT or TYPE_CODE_UNION.  */
+
+static void
+ada_val_print_struct_union
+  (struct type *type, const gdb_byte *valaddr, int offset,
+   int offset_aligned, CORE_ADDR address, struct ui_file *stream,
+   int recurse, const struct value *original_value,
+   const struct value_print_options *options,
+   const struct language_defn *language)
+{
+  if (ada_is_bogus_array_descriptor (type))
+    {
+      fprintf_filtered (stream, "(...?)");
+      return;
+    }
+
+  print_record (type, valaddr, offset_aligned,
+		stream, recurse, original_value, options);
+}
+
+/* Implement Ada val_print'ing for the case where TYPE is
+   a TYPE_CODE_REF.  */
+
+static void
+ada_val_print_ref (struct type *type, const gdb_byte *valaddr,
+		   int offset, int offset_aligned, CORE_ADDR address,
+		   struct ui_file *stream, int recurse,
+		   const struct value *original_value,
+		   const struct value_print_options *options,
+		   const struct language_defn *language)
+{
+  /* For references, the debugger is expected to print the value as
+     an address if DEREF_REF is null.  But printing an address in place
+     of the object value would be confusing to an Ada programmer.
+     So, for Ada values, we print the actual dereferenced value
+     regardless.  */
+  struct type *elttype = check_typedef (TYPE_TARGET_TYPE (type));
+
+  if (TYPE_CODE (elttype) != TYPE_CODE_UNDEF)
+    {
+      CORE_ADDR deref_val_int;
+      struct value *deref_val;
+
+      deref_val = coerce_ref_if_computed (original_value);
+      if (deref_val)
+	{
+	  if (ada_is_tagged_type (value_type (deref_val), 1))
+	    deref_val = ada_tag_value_at_base_address (deref_val);
+
+	  common_val_print (deref_val, stream, recurse + 1, options,
+			    current_language);
+	  return;
+	}
+
+      deref_val_int = unpack_pointer (type, valaddr + offset_aligned);
+      if (deref_val_int != 0)
+	{
+	  deref_val =
+	    ada_value_ind (value_from_pointer
+			   (lookup_pointer_type (elttype),
+			    deref_val_int));
+
+	  if (ada_is_tagged_type (value_type (deref_val), 1))
+	    deref_val = ada_tag_value_at_base_address (deref_val);
+
+	  val_print (value_type (deref_val),
+		     value_contents_for_printing (deref_val),
+		     value_embedded_offset (deref_val),
+		     value_address (deref_val), stream, recurse + 1,
+		     deref_val, options, current_language);
+	}
+      else
+	fputs_filtered ("(null)", stream);
+    }
+  else
+    fputs_filtered ("???", stream);
+}
+
 /* See the comment on ada_val_print.  This function differs in that it
    does not catch evaluation errors (leaving that to ada_val_print).  */
 
@@ -783,8 +1079,6 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
 		 const struct value_print_options *options,
 		 const struct language_defn *language)
 {
-  int i;
-  struct type *elttype;
   int offset_aligned;
 
   type = ada_check_typedef (type);
@@ -793,28 +1087,9 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
       || (ada_is_constrained_packed_array_type (type)
 	  && TYPE_CODE (type) != TYPE_CODE_PTR))
     {
-      struct value *mark = value_mark ();
-      struct value *val;
-
-      val = value_from_contents_and_address (type, valaddr + offset, address);
-      /* If this is a reference, coerce it now.  This helps taking care
-	 of the case where ADDRESS is meaningless because original_value
-	 was not an lval.  */
-      val = coerce_ref (val);
-      if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF)  /* array access type.  */
-	val = ada_coerce_to_simple_array_ptr (val);
-      else
-	val = ada_coerce_to_simple_array (val);
-      if (val == NULL)
-	{
-	  gdb_assert (TYPE_CODE (type) == TYPE_CODE_TYPEDEF);
-	  fprintf_filtered (stream, "0x0");
-	}
-      else
-	val_print (value_type (val), value_contents_for_printing (val),
-		   value_embedded_offset (val), value_address (val),
-		   stream, recurse, val, options, language);
-      value_free_to_mark (mark);
+      ada_val_print_gnat_array (type, valaddr, offset, address,
+				stream, recurse, original_value,
+				options, language);
       return;
     }
 
@@ -829,165 +1104,36 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
       break;
 
     case TYPE_CODE_PTR:
-      {
-	val_print (type, valaddr, offset, address, stream, recurse,
-		   original_value, options, language_def (language_c));
-
-	if (ada_is_tag_type (type))
-	  {
-	    struct value *val =
-	      value_from_contents_and_address (type,
-					       valaddr + offset_aligned,
-					       address + offset_aligned);
-	    const char *name = ada_tag_name (val);
-
-	    if (name != NULL) 
-	      fprintf_filtered (stream, " (%s)", name);
-	  }
-	return;
-      }
+      ada_val_print_ptr (type, valaddr, offset, offset_aligned,
+			 address, stream, recurse, original_value,
+			 options, language);
+      break;
 
     case TYPE_CODE_INT:
     case TYPE_CODE_RANGE:
-      if (ada_is_fixed_point_type (type))
-	{
-	  LONGEST v = unpack_long (type, valaddr + offset_aligned);
-
-	  fprintf_filtered (stream, TYPE_LENGTH (type) < 4 ? "%.11g" : "%.17g",
-			    (double) ada_fixed_to_float (type, v));
-	  return;
-	}
-      else if (TYPE_CODE (type) == TYPE_CODE_RANGE)
-	{
-	  struct type *target_type = TYPE_TARGET_TYPE (type);
-
-	  if (TYPE_LENGTH (type) != TYPE_LENGTH (target_type))
-	    {
-	      /* Obscure case of range type that has different length from
-	         its base type.  Perform a conversion, or we will get a
-	         nonsense value.  Actually, we could use the same
-	         code regardless of lengths; I'm just avoiding a cast.  */
-	      struct value *v1
-		= value_from_contents_and_address (type, valaddr + offset, 0);
-	      struct value *v = value_cast (target_type, v1);
-
-	      val_print (target_type, value_contents_for_printing (v),
-			 value_embedded_offset (v), 0, stream,
-			 recurse + 1, v, options, language);
-	    }
-	  else
-	    val_print (TYPE_TARGET_TYPE (type), valaddr, offset,
-		       address, stream, recurse, original_value,
-		       options, language);
-	  return;
-	}
-      else
-	{
-	  int format = (options->format ? options->format
-			: options->output_format);
-
-	  if (format)
-	    {
-	      struct value_print_options opts = *options;
-
-	      opts.format = format;
-	      val_print_scalar_formatted (type, valaddr, offset_aligned,
-					  original_value, &opts, 0, stream);
-	    }
-          else if (ada_is_system_address_type (type))
-            {
-              /* FIXME: We want to print System.Address variables using
-                 the same format as for any access type.  But for some
-                 reason GNAT encodes the System.Address type as an int,
-                 so we have to work-around this deficiency by handling
-                 System.Address values as a special case.  */
-
-	      struct gdbarch *gdbarch = get_type_arch (type);
-	      struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
-	      CORE_ADDR addr = extract_typed_address (valaddr + offset_aligned,
-						      ptr_type);
-
-              fprintf_filtered (stream, "(");
-              type_print (type, "", stream, -1);
-              fprintf_filtered (stream, ") ");
-	      fputs_filtered (paddress (gdbarch, addr), stream);
-            }
-	  else
-	    {
-	      val_print_type_code_int (type, valaddr + offset_aligned, stream);
-	      if (ada_is_character_type (type))
-		{
-		  LONGEST c;
-
-		  fputs_filtered (" ", stream);
-		  c = unpack_long (type, valaddr + offset_aligned);
-		  ada_printchar (c, type, stream);
-		}
-	    }
-	  return;
-	}
+      ada_val_print_num (type, valaddr, offset, offset_aligned,
+			 address, stream, recurse, original_value,
+			 options, language);
+      break;
 
     case TYPE_CODE_ENUM:
-      {
-	unsigned int len;
-	LONGEST val;
-
-	if (options->format)
-	  {
-	    val_print_scalar_formatted (type, valaddr, offset_aligned,
-					original_value, options, 0, stream);
-	    break;
-	  }
-	len = TYPE_NFIELDS (type);
-	val = unpack_long (type, valaddr + offset_aligned);
-	for (i = 0; i < len; i++)
-	  {
-	    QUIT;
-	    if (val == TYPE_FIELD_ENUMVAL (type, i))
-	      {
-		break;
-	      }
-	  }
-	if (i < len)
-	  {
-	    const char *name = ada_enum_name (TYPE_FIELD_NAME (type, i));
-
-	    if (name[0] == '\'')
-	      fprintf_filtered (stream, "%ld %s", (long) val, name);
-	    else
-	      fputs_filtered (name, stream);
-	  }
-	else
-	  {
-	    print_longest (stream, 'd', 0, val);
-	  }
-	break;
-      }
+      ada_val_print_enum (type, valaddr, offset, offset_aligned,
+			  address, stream, recurse, original_value,
+			  options, language);
+      break;
 
     case TYPE_CODE_FLT:
-      if (options->format)
-	{
-	  val_print (type, valaddr, offset, address, stream, recurse,
-		     original_value, options, language_def (language_c));
-	  return;
-	}
-      else
-	ada_print_floating (valaddr + offset, type, stream);
+      ada_val_print_flt (type, valaddr, offset, offset_aligned,
+			 address, stream, recurse, original_value,
+			 options, language);
       break;
 
     case TYPE_CODE_UNION:
     case TYPE_CODE_STRUCT:
-      if (ada_is_bogus_array_descriptor (type))
-	{
-	  fprintf_filtered (stream, "(...?)");
-	  return;
-	}
-      else
-	{
-	  print_record (type, valaddr, offset_aligned,
-			stream, recurse, original_value, options);
-	  return;
-	}
+      ada_val_print_struct_union (type, valaddr, offset, offset_aligned,
+				  address, stream, recurse,
+				  original_value, options, language);
+      break;
 
     case TYPE_CODE_ARRAY:
       ada_val_print_array (type, valaddr, offset_aligned,
@@ -996,52 +1142,9 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
       return;
 
     case TYPE_CODE_REF:
-      /* For references, the debugger is expected to print the value as
-         an address if DEREF_REF is null.  But printing an address in place
-         of the object value would be confusing to an Ada programmer.
-         So, for Ada values, we print the actual dereferenced value
-         regardless.  */
-      elttype = check_typedef (TYPE_TARGET_TYPE (type));
-      
-      if (TYPE_CODE (elttype) != TYPE_CODE_UNDEF)
-        {
-          CORE_ADDR deref_val_int;
-	  struct value *deref_val;
-
-	  deref_val = coerce_ref_if_computed (original_value);
-	  if (deref_val)
-	    {
-	      if (ada_is_tagged_type (value_type (deref_val), 1))
-		deref_val = ada_tag_value_at_base_address (deref_val);
-
-	      common_val_print (deref_val, stream, recurse + 1, options,
-				current_language);
-	      break;
-	    }
-
-          deref_val_int = unpack_pointer (type, valaddr + offset_aligned);
-          if (deref_val_int != 0)
-            {
-              deref_val =
-                ada_value_ind (value_from_pointer
-                               (lookup_pointer_type (elttype),
-                                deref_val_int));
-
-	      if (ada_is_tagged_type (value_type (deref_val), 1))
-		deref_val = ada_tag_value_at_base_address (deref_val);
-
-              val_print (value_type (deref_val),
-                         value_contents_for_printing (deref_val),
-                         value_embedded_offset (deref_val),
-                         value_address (deref_val), stream, recurse + 1,
-			 deref_val, options, current_language);
-            }
-          else
-            fputs_filtered ("(null)", stream);
-        }
-      else
-        fputs_filtered ("???", stream);
-
+      ada_val_print_ref (type, valaddr, offset, offset_aligned,
+			 address, stream, recurse, original_value,
+			 options, language);
       break;
     }
 }
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [commit/Ada 08/11] move ada_val_print_array down within other ada_val_print* functions
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 01/11] ada-valprint.c: Reorder functions to reduce advance declarations Joel Brobecker
@ 2014-01-07  4:22 ` Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 02/11] ada_val_print_1: Add language parameter Joel Brobecker
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:22 UTC (permalink / raw)
  To: gdb-patches

This patch moves ada_val_print_array to group it with the other
ada_val_print_* function which are being called by ada_val_print_1.
Since this function is in the same situation, it is more logical
to move it within that group.

It also rationalizes the function's prototype to match the prototype
of the other ada_val_print_* routines.

gdb/ChangeLog:

        * ada-valprint.c (ada_val_print_array): Move implementation
        down.  Rename parameter "offset" and "val" into "offset_aligned"
        and "original_value" respectively.  Add parameter "offset".
---
 gdb/ChangeLog      |   6 +++
 gdb/ada-valprint.c | 133 +++++++++++++++++++++++++++--------------------------
 2 files changed, 73 insertions(+), 66 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ece68cc..004709b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-valprint.c (ada_val_print_array): Move implementation
+	down.  Rename parameter "offset" and "val" into "offset_aligned"
+	and "original_value" respectively.  Add parameter "offset".
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-valprint.c (ada_val_print_ref): Rewrite by mostly
 	re-organizing the code. Change the "???" message printed
 	when target type is a TYPE_CODE_UNDEF into
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 12c84da..43e1490 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -537,71 +537,6 @@ ada_printstr (struct ui_file *stream, struct type *type,
 	    options);
 }
 
-
-/* Assuming TYPE is a simple array, print the value of this array located
-   at VALADDR + OFFSET.  See ada_val_print for a description of the various
-   parameters of this function; they are identical.  */
-
-static void
-ada_val_print_array (struct type *type, const gdb_byte *valaddr,
-		     int offset, CORE_ADDR address,
-		     struct ui_file *stream, int recurse,
-		     const struct value *val,
-		     const struct value_print_options *options)
-{
-  /* For an array of chars, print with string syntax.  */
-  if (ada_is_string_type (type)
-      && (options->format == 0 || options->format == 's'))
-    {
-      enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
-      struct type *elttype = TYPE_TARGET_TYPE (type);
-      unsigned int eltlen;
-      unsigned int len;
-
-      /* We know that ELTTYPE cannot possibly be null, because we found
-	 that TYPE is a string-like type.  Similarly, the size of ELTTYPE
-	 should also be non-null, since it's a character-like type.  */
-      gdb_assert (elttype != NULL);
-      gdb_assert (TYPE_LENGTH (elttype) != 0);
-
-      eltlen = TYPE_LENGTH (elttype);
-      len = TYPE_LENGTH (type) / eltlen;
-
-      if (options->prettyformat_arrays)
-        print_spaces_filtered (2 + 2 * recurse, stream);
-
-      /* If requested, look for the first null char and only print
-         elements up to it.  */
-      if (options->stop_print_at_null)
-        {
-          int temp_len;
-
-          /* Look for a NULL char.  */
-          for (temp_len = 0;
-               (temp_len < len
-                && temp_len < options->print_max
-                && char_at (valaddr + offset,
-			    temp_len, eltlen, byte_order) != 0);
-               temp_len += 1);
-          len = temp_len;
-        }
-
-      printstr (stream, elttype, valaddr + offset, len, 0, eltlen, options);
-    }
-  else
-    {
-      fprintf_filtered (stream, "(");
-      print_optional_low_bound (stream, type, options);
-      if (TYPE_FIELD_BITSIZE (type, 0) > 0)
-        val_print_packed_array_elements (type, valaddr, offset,
-					 0, stream, recurse, val, options);
-      else
-        val_print_array_elements (type, valaddr, offset, address,
-				  stream, recurse, val, options, 0);
-      fprintf_filtered (stream, ")");
-    }
-}
-
 static int
 print_variant_part (struct type *type, int field_num,
 		    const gdb_byte *valaddr, int offset,
@@ -999,6 +934,72 @@ ada_val_print_struct_union
 }
 
 /* Implement Ada val_print'ing for the case where TYPE is
+   a TYPE_CODE_ARRAY.  */
+
+static void
+ada_val_print_array (struct type *type, const gdb_byte *valaddr,
+		     int offset, int offset_aligned, CORE_ADDR address,
+		     struct ui_file *stream, int recurse,
+		     const struct value *original_value,
+		     const struct value_print_options *options)
+{
+  /* For an array of chars, print with string syntax.  */
+  if (ada_is_string_type (type)
+      && (options->format == 0 || options->format == 's'))
+    {
+      enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+      struct type *elttype = TYPE_TARGET_TYPE (type);
+      unsigned int eltlen;
+      unsigned int len;
+
+      /* We know that ELTTYPE cannot possibly be null, because we found
+	 that TYPE is a string-like type.  Similarly, the size of ELTTYPE
+	 should also be non-null, since it's a character-like type.  */
+      gdb_assert (elttype != NULL);
+      gdb_assert (TYPE_LENGTH (elttype) != 0);
+
+      eltlen = TYPE_LENGTH (elttype);
+      len = TYPE_LENGTH (type) / eltlen;
+
+      if (options->prettyformat_arrays)
+        print_spaces_filtered (2 + 2 * recurse, stream);
+
+      /* If requested, look for the first null char and only print
+         elements up to it.  */
+      if (options->stop_print_at_null)
+        {
+          int temp_len;
+
+          /* Look for a NULL char.  */
+          for (temp_len = 0;
+               (temp_len < len
+                && temp_len < options->print_max
+                && char_at (valaddr + offset_aligned,
+			    temp_len, eltlen, byte_order) != 0);
+               temp_len += 1);
+          len = temp_len;
+        }
+
+      printstr (stream, elttype, valaddr + offset_aligned, len, 0,
+		eltlen, options);
+    }
+  else
+    {
+      fprintf_filtered (stream, "(");
+      print_optional_low_bound (stream, type, options);
+      if (TYPE_FIELD_BITSIZE (type, 0) > 0)
+        val_print_packed_array_elements (type, valaddr, offset_aligned,
+					 0, stream, recurse,
+					 original_value, options);
+      else
+        val_print_array_elements (type, valaddr, offset_aligned, address,
+				  stream, recurse, original_value,
+				  options, 0);
+      fprintf_filtered (stream, ")");
+    }
+}
+
+/* Implement Ada val_print'ing for the case where TYPE is
    a TYPE_CODE_REF.  */
 
 static void
@@ -1123,7 +1124,7 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
       break;
 
     case TYPE_CODE_ARRAY:
-      ada_val_print_array (type, valaddr, offset_aligned,
+      ada_val_print_array (type, valaddr, offset, offset_aligned,
 			   address, stream, recurse, original_value,
 			   options);
       return;
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [commit/Ada 06/11] ada-valprint.c: Inline print_record inside ada_val_print_struct_union
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
                   ` (5 preceding siblings ...)
  2014-01-07  4:22 ` [commit/Ada 05/11] Split ada_val_print_1 into smaller functions Joel Brobecker
@ 2014-01-07  4:22 ` Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 04/11] Remove call to gdb_flush at end of ada_val_print_1 Joel Brobecker
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:22 UTC (permalink / raw)
  To: gdb-patches

The function print_record is a fairly small and straightforward
function which is only called from one location. So this patch
inlines the code at the point of call.

One small advantage is that the context of use of this patch has
now become such that we can assume that TYPE is not a typedef,
nor an enum. So thhe call to ada_check_typedef is unnecessary,
and this patch removes it.

gdb/ChangeLog:

        * ada-valprint.c (print_record): Delete, implementation inlined...
        (ada_val_print_struct_union): ... here.  Remove call to
        ada_check_typedef in inlined implementation.
---
 gdb/ChangeLog      |  6 ++++++
 gdb/ada-valprint.c | 36 ++++++++++++------------------------
 2 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4be753d..4c1978c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-valprint.c (print_record): Delete, implementation inlined...
+	(ada_val_print_struct_union): ... here.  Remove call to
+	ada_check_typedef in inlined implementation.
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-valprint.c (ada_val_print_gnat_array): New function,
 	extracted from ada_val_print_1;
 	(ada_val_print_ptr, ada_val_print_num, ada_val_print_enum)
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 1111d90..22ec9c0 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -750,28 +750,6 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
   return comma_needed;
 }
 
-static void
-print_record (struct type *type, const gdb_byte *valaddr,
-	      int offset,
-	      struct ui_file *stream, int recurse,
-	      const struct value *val,
-	      const struct value_print_options *options)
-{
-  type = ada_check_typedef (type);
-
-  fprintf_filtered (stream, "(");
-
-  if (print_field_values (type, valaddr, offset,
-			  stream, recurse, val, options,
-			  0, type, offset) != 0 && options->prettyformat)
-    {
-      fprintf_filtered (stream, "\n");
-      print_spaces_filtered (2 * recurse, stream);
-    }
-
-  fprintf_filtered (stream, ")");
-}
-
 /* Implement Ada val_print-ing for GNAT arrays (Eg. fat pointers,
    thin pointers, etc).  */
 
@@ -1006,8 +984,18 @@ ada_val_print_struct_union
       return;
     }
 
-  print_record (type, valaddr, offset_aligned,
-		stream, recurse, original_value, options);
+  fprintf_filtered (stream, "(");
+
+  if (print_field_values (type, valaddr, offset_aligned,
+			  stream, recurse, original_value, options,
+			  0, type, offset_aligned) != 0
+      && options->prettyformat)
+    {
+      fprintf_filtered (stream, "\n");
+      print_spaces_filtered (2 * recurse, stream);
+    }
+
+  fprintf_filtered (stream, ")");
 }
 
 /* Implement Ada val_print'ing for the case where TYPE is
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [commit/Ada 02/11] ada_val_print_1: Add language parameter
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 01/11] ada-valprint.c: Reorder functions to reduce advance declarations Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 08/11] move ada_val_print_array down within other ada_val_print* functions Joel Brobecker
@ 2014-01-07  4:22 ` Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 03/11] ada_val_print_1: Go through val_print instead of recursive call to self Joel Brobecker
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:22 UTC (permalink / raw)
  To: gdb-patches

This is to help calling val_print.  We would like to be more systematic
in calling val_print when printing, because it allows us to make sure
we take advantage of the standard features such as pretty-printing
which are handled by val_print.

gdb/ChangeLog:

        * ada-valprint.c (ada_val_print_1): Add parameter "language".
        Update calls to self accordingly.  Replace calls to c_val_print
        by calls to val_print.
---
 gdb/ChangeLog      |  6 ++++++
 gdb/ada-valprint.c | 25 ++++++++++++++-----------
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 64d1e6b..2866cae 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-valprint.c (ada_val_print_1): Add parameter "language".
+	Update calls to self accordingly.  Replace calls to c_val_print
+	by calls to val_print.
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-valprint.c (print_record): Delete declaration.
 	(adjust_type_signedness, ada_val_print_1): Likewise.
 	(ada_val_print): Move function implementation down.
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 5162737..66cda39 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -780,7 +780,8 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
 		 int offset, CORE_ADDR address,
 		 struct ui_file *stream, int recurse,
 		 const struct value *original_value,
-		 const struct value_print_options *options)
+		 const struct value_print_options *options,
+		 const struct language_defn *language)
 {
   int i;
   struct type *elttype;
@@ -814,7 +815,7 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
 			 value_contents_for_printing (val),
 			 value_embedded_offset (val),
 			 value_address (val), stream, recurse,
-			 val, options);
+			 val, options, language);
       value_free_to_mark (mark);
       return;
     }
@@ -825,14 +826,14 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
   switch (TYPE_CODE (type))
     {
     default:
-      c_val_print (type, valaddr, offset, address, stream,
-		   recurse, original_value, options);
+      val_print (type, valaddr, offset, address, stream, recurse,
+		 original_value, options, language_def (language_c));
       break;
 
     case TYPE_CODE_PTR:
       {
-	c_val_print (type, valaddr, offset, address,
-		     stream, recurse, original_value, options);
+	val_print (type, valaddr, offset, address, stream, recurse,
+		   original_value, options, language_def (language_c));
 
 	if (ada_is_tag_type (type))
 	  {
@@ -875,13 +876,14 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
 	      ada_val_print_1 (target_type,
 			       value_contents_for_printing (v),
 			       value_embedded_offset (v), 0,
-			       stream, recurse + 1, v, options);
+			       stream, recurse + 1, v, options,
+			       language);
 	    }
 	  else
 	    ada_val_print_1 (TYPE_TARGET_TYPE (type),
 			     valaddr, offset,
 			     address, stream, recurse,
-			     original_value, options);
+			     original_value, options, language);
 	  return;
 	}
       else
@@ -970,8 +972,8 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
     case TYPE_CODE_FLT:
       if (options->format)
 	{
-	  c_val_print (type, valaddr, offset, address, stream,
-		       recurse, original_value, options);
+	  val_print (type, valaddr, offset, address, stream, recurse,
+		     original_value, options, language_def (language_c));
 	  return;
 	}
       else
@@ -1066,7 +1068,8 @@ ada_val_print (struct type *type, const gdb_byte *valaddr,
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
       ada_val_print_1 (type, valaddr, embedded_offset, address,
-		       stream, recurse, val, options);
+		       stream, recurse, val, options,
+		       current_language);
     }
 }
 
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [commit/Ada] General ada-valprint improvements
@ 2014-01-07  4:22 Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 01/11] ada-valprint.c: Reorder functions to reduce advance declarations Joel Brobecker
                   ` (11 more replies)
  0 siblings, 12 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:22 UTC (permalink / raw)
  To: gdb-patches

Hello,

FYI: I am pushing this patch series. The trigger was the last patch,
which is to fix a bug where the pretty-printer was not called for
components of record types. But the motivation was there even before
then.

Long term, I'd love to get rid of val_print, and only keep value_print.
This would help with partially-available values. But val_print and
value_print have subtle differences, so I'll leave that for later.

The patch series has been tested on x86_64-linux.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [commit/Ada 04/11] Remove call to gdb_flush at end of ada_val_print_1
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
                   ` (6 preceding siblings ...)
  2014-01-07  4:22 ` [commit/Ada 06/11] ada-valprint.c: Inline print_record inside ada_val_print_struct_union Joel Brobecker
@ 2014-01-07  4:22 ` Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 09/11] Extract string-printing out of ada_val_print_array Joel Brobecker
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:22 UTC (permalink / raw)
  To: gdb-patches

I am not sure why this function was called in the first place, but
it disrupts the printing flow when in GDB/MI mode, ending the current
console stream output, and starting a new one. It's not clear whether,
with the code as currently written, the problem is actually visible
or only latent. But, it becomes visible when we replace one of the
"return" statements in the "switch" block just above by a "break"
statement (this is something I'd like to do, and what made me realize
the problem). With the gdb_flush call (after having replaced the
"return" statement as explained above), we get:

        % gdb -q -i=mi ada_prg
        (gdb)
        print 1
        &"print 1\n"
  !! -> ~"$1 = 1"
  !! -> ~"\n"
        ^done

With the gdb_flush call removed, we now get the entire output into
a single stream.

        (gdb)
        print 1
        &"print 1\n"
        ~"$1 = 1"
        ~"\n"
        ^done

gdb/ChangeLog:

        * ada-valprint.c (ada_val_print_1): Remove call to gdb_flush.
---
 gdb/ChangeLog      | 4 ++++
 gdb/ada-valprint.c | 1 -
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ee0df63..f8820a5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-valprint.c (ada_val_print_1): Remove call to gdb_flush.
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-valprint.c (ada_val_print_1): Replace calls to
 	ada_val_print_1 by calls to val_print.
 
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index ff0fa66..9c60a79 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -1044,7 +1044,6 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
 
       break;
     }
-  gdb_flush (stream);
 }
 
 /* See val_print for a description of the various parameters of this
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [commit/Ada 07/11] rewrite ada_val_print_ref to reduce if/else block nesting depth
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
                   ` (3 preceding siblings ...)
  2014-01-07  4:22 ` [commit/Ada 03/11] ada_val_print_1: Go through val_print instead of recursive call to self Joel Brobecker
@ 2014-01-07  4:22 ` Joel Brobecker
  2014-01-07  4:22 ` [commit/Ada 05/11] Split ada_val_print_1 into smaller functions Joel Brobecker
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:22 UTC (permalink / raw)
  To: gdb-patches

The logic as currently implemented in this function was a little
difficult to follow, due to the nested of if/else conditions,
but most of the time, the "else" block was very simple. So this
patch re-organizes the code to use fewer levels of nesting by
using return statements, and writing the code as a sequence of
"if something simple, then handle it and return" blocks.

While touching this code, this patch changes the cryptic "???"
printed when trying to print a reference pointing to an undefined
type. This should only ever happen if the debugging information
was corrupted or improperly read. But in case that happens, we now
print "<ref to undefined type>" instead. This is more in line
with how we print other conditions such as optimized out pieces,
or synthetic pointers.

gdb/ChangeLog:

        * ada-valprint.c (ada_val_print_ref): Rewrite by mostly
        re-organizing the code. Change the "???" message printed
        when target type is a TYPE_CODE_UNDEF into
        "<ref to undefined type>".
---
 gdb/ChangeLog      |  7 ++++++
 gdb/ada-valprint.c | 65 +++++++++++++++++++++++++++---------------------------
 2 files changed, 39 insertions(+), 33 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4c1978c..ece68cc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-valprint.c (ada_val_print_ref): Rewrite by mostly
+	re-organizing the code. Change the "???" message printed
+	when target type is a TYPE_CODE_UNDEF into
+	"<ref to undefined type>".
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-valprint.c (print_record): Delete, implementation inlined...
 	(ada_val_print_struct_union): ... here.  Remove call to
 	ada_check_typedef in inlined implementation.
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 22ec9c0..12c84da 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -1015,45 +1015,44 @@ ada_val_print_ref (struct type *type, const gdb_byte *valaddr,
      So, for Ada values, we print the actual dereferenced value
      regardless.  */
   struct type *elttype = check_typedef (TYPE_TARGET_TYPE (type));
+  struct value *deref_val;
+  CORE_ADDR deref_val_int;
 
-  if (TYPE_CODE (elttype) != TYPE_CODE_UNDEF)
+  if (TYPE_CODE (elttype) == TYPE_CODE_UNDEF)
     {
-      CORE_ADDR deref_val_int;
-      struct value *deref_val;
+      fputs_filtered ("<ref to undefined type>", stream);
+      return;
+    }
 
-      deref_val = coerce_ref_if_computed (original_value);
-      if (deref_val)
-	{
-	  if (ada_is_tagged_type (value_type (deref_val), 1))
-	    deref_val = ada_tag_value_at_base_address (deref_val);
+  deref_val = coerce_ref_if_computed (original_value);
+  if (deref_val)
+    {
+      if (ada_is_tagged_type (value_type (deref_val), 1))
+	deref_val = ada_tag_value_at_base_address (deref_val);
 
-	  common_val_print (deref_val, stream, recurse + 1, options,
-			    current_language);
-	  return;
-	}
+      common_val_print (deref_val, stream, recurse + 1, options,
+			language);
+      return;
+    }
 
-      deref_val_int = unpack_pointer (type, valaddr + offset_aligned);
-      if (deref_val_int != 0)
-	{
-	  deref_val =
-	    ada_value_ind (value_from_pointer
-			   (lookup_pointer_type (elttype),
-			    deref_val_int));
-
-	  if (ada_is_tagged_type (value_type (deref_val), 1))
-	    deref_val = ada_tag_value_at_base_address (deref_val);
-
-	  val_print (value_type (deref_val),
-		     value_contents_for_printing (deref_val),
-		     value_embedded_offset (deref_val),
-		     value_address (deref_val), stream, recurse + 1,
-		     deref_val, options, current_language);
-	}
-      else
-	fputs_filtered ("(null)", stream);
+  deref_val_int = unpack_pointer (type, valaddr + offset_aligned);
+  if (deref_val_int == 0)
+    {
+      fputs_filtered ("(null)", stream);
+      return;
     }
-  else
-    fputs_filtered ("???", stream);
+
+  deref_val
+    = ada_value_ind (value_from_pointer (lookup_pointer_type (elttype),
+					 deref_val_int));
+  if (ada_is_tagged_type (value_type (deref_val), 1))
+    deref_val = ada_tag_value_at_base_address (deref_val);
+
+  val_print (value_type (deref_val),
+	     value_contents_for_printing (deref_val),
+	     value_embedded_offset (deref_val),
+	     value_address (deref_val), stream, recurse + 1,
+	     deref_val, options, language);
 }
 
 /* See the comment on ada_val_print.  This function differs in that it
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [commit/Ada 10/11] ada_print_floating: Remove use of statically sized buffer.
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
                   ` (9 preceding siblings ...)
  2014-01-07  4:23 ` [commit/Ada 11/11] Ada: Fix missing call to pretty-printer for fields of records Joel Brobecker
@ 2014-01-07  4:23 ` Joel Brobecker
  2014-01-07 17:29 ` [commit/Ada] General ada-valprint improvements Tom Tromey
  11 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:23 UTC (permalink / raw)
  To: gdb-patches

ada_print_floating declares a char buffer with a size that we're hoping
to always be large enough to hold any string representation of a float
value.  But that's not really necessary, and also forces us to create
a small wrapper (ui_memcpy) to perform the extraction from a temporary
stream into this buffer.  This patches fixes both issues by relying on
ui_file_xstrdup.  This forces us to make a few adjustments that are
minor in nature, as we now need to defer the cleanup to the end of
the function.

gdb/ChangeLog:

        * ada-valprint.c (ui_memcpy): Delete.
        (ada_print_floating): Update documentation.  Add empty line
        between between function documentation and implementation.
        Delete variable "buffer".  Use ui_file_xstrdup in place of
        ui_file_put.  Minor adjustments following this change.
---
 gdb/ChangeLog      |  8 ++++++++
 gdb/ada-valprint.c | 24 ++++++++----------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e668617..9fc947b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-valprint.c (ui_memcpy): Delete.
+	(ada_print_floating): Update documentation.  Add empty line
+	between between function documentation and implementation.
+	Delete variable "buffer".  Use ui_file_xstrdup in place of
+	ui_file_put.  Minor adjustments following this change.
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-valprint.c (ada_val_print_string): New function,
 	extracted from ada_val_print_array.
 	(ada_val_print_array): Replace extracted code by call
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index d1c8553..1ae2089 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -289,32 +289,22 @@ char_at (const gdb_byte *string, int i, int type_len,
                                            type_len, byte_order);
 }
 
-/* Wrapper around memcpy to make it legal argument to ui_file_put.  */
-static void
-ui_memcpy (void *dest, const char *buffer, long len)
-{
-  memcpy (dest, buffer, (size_t) len);
-  ((char *) dest)[len] = '\0';
-}
-
 /* Print a floating-point value of type TYPE, pointed to in GDB by
    VALADDR, on STREAM.  Use Ada formatting conventions: there must be
    a decimal point, and at least one digit before and after the
-   point.  We use GNAT format for NaNs and infinities.  */
+   point.  We use the GNAT format for NaNs and infinities.  */
+
 static void
 ada_print_floating (const gdb_byte *valaddr, struct type *type,
 		    struct ui_file *stream)
 {
-  char buffer[64];
   char *s, *result;
   struct ui_file *tmp_stream = mem_fileopen ();
   struct cleanup *cleanups = make_cleanup_ui_file_delete (tmp_stream);
 
   print_floating (valaddr, type, tmp_stream);
-  ui_file_put (tmp_stream, ui_memcpy, buffer);
-  do_cleanups (cleanups);
-
-  result = buffer;
+  result = ui_file_xstrdup (tmp_stream, NULL);
+  make_cleanup (xfree, result);
 
   /* Modify for Ada rules.  */
 
@@ -348,9 +338,11 @@ ada_print_floating (const gdb_byte *valaddr, struct type *type,
 	fprintf_filtered (stream, "%s.0", result);
       else
 	fprintf_filtered (stream, "%.*s.0%s", (int) (s-result), result, s);
-      return;
     }
-  fprintf_filtered (stream, "%s", result);
+  else
+    fprintf_filtered (stream, "%s", result);
+
+  do_cleanups (cleanups);
 }
 
 void
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [commit/Ada 11/11] Ada: Fix missing call to pretty-printer for fields of records.
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
                   ` (8 preceding siblings ...)
  2014-01-07  4:22 ` [commit/Ada 09/11] Extract string-printing out of ada_val_print_array Joel Brobecker
@ 2014-01-07  4:23 ` Joel Brobecker
  2014-01-07  4:23 ` [commit/Ada 10/11] ada_print_floating: Remove use of statically sized buffer Joel Brobecker
  2014-01-07 17:29 ` [commit/Ada] General ada-valprint improvements Tom Tromey
  11 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-01-07  4:23 UTC (permalink / raw)
  To: gdb-patches

Consider the following types:

   type Time_T is record
      Secs : Integer;
   end record;
   Before : Time_T := (Secs => 1384395743);

In this example, we assume that type Time_T is the number of seconds
since Epoch, and so added a Python pretty-printer, to print this
type in a more human-friendly way. For instance:

    (gdb) print before
    $1 = Thu Nov 14 02:22:23 2013 (1384395743)

However, we've noticed that things stop working when this type is
embedded inside another record, and we try to print that record.
For instance, with the following declarations:

   type Composite is record
      Id : Integer;
      T : Time_T;
   end record;
   Afternoon : Composite := (Id => 1, T => (Secs => 1384395865));

    (gdb) print afternoon
    $2 = (id => 1, t => (secs => 1384395865))

We expected instead:

    (gdb) print afternoon
    $2 = (id => 1, t => Thu Nov 14 02:24:25 2013 (1384395865))

This patch fixes the problem by making sure that we try to print
each field via a call to val_print, rather than calling ada_val_print
directly. We need to go through val_print, as the val_print
handles all language-independent features such as calling the
pretty-printer, knowing that ada_val_print will get called eventually
if actual Ada-specific printing is required (which should be the
most common scenario).

And because val_print takes the language as parameter, we enhanced
the print_field_values and print_variant_part to also take a language.
As a bonus, this allows us to remove a couple of references to
current_language.

gdb/ChangeLog:

        * ada-valprint.c (print_field_values): Add "language" parameter.
        Update calls to print_field_values and print_variant_part.
        Pass new parameter "language" in call to val_print instead
        of "current_language".  Replace call to ada_val_print by call
        to val_print.
        (print_variant_part): Add "language" parameter.
        (ada_val_print_struct_union): Update call to print_field_values.

gdb/testsuite/ChangeLog:

        * gdb.ada/pp-rec-component.exp, gdb.ada/pp-rec-component.py,
        gdb.ada/pp-rec-component/foo.adb, gdb.ada/pp-rec-component/pck.adb,
        gdb.ada/pp-rec-component/pck.ads: New files.
---
 gdb/ChangeLog                                  | 10 +++++++
 gdb/ada-valprint.c                             | 27 ++++++++---------
 gdb/testsuite/ChangeLog                        |  6 ++++
 gdb/testsuite/gdb.ada/pp-rec-component.exp     | 40 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.ada/pp-rec-component.py      | 35 ++++++++++++++++++++++
 gdb/testsuite/gdb.ada/pp-rec-component/foo.adb | 22 ++++++++++++++
 gdb/testsuite/gdb.ada/pp-rec-component/pck.adb | 21 ++++++++++++++
 gdb/testsuite/gdb.ada/pp-rec-component/pck.ads | 23 +++++++++++++++
 8 files changed, 171 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/pp-rec-component.exp
 create mode 100644 gdb/testsuite/gdb.ada/pp-rec-component.py
 create mode 100644 gdb/testsuite/gdb.ada/pp-rec-component/foo.adb
 create mode 100644 gdb/testsuite/gdb.ada/pp-rec-component/pck.adb
 create mode 100644 gdb/testsuite/gdb.ada/pp-rec-component/pck.ads

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9fc947b..11e5565 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-valprint.c (print_field_values): Add "language" parameter.
+	Update calls to print_field_values and print_variant_part.
+	Pass new parameter "language" in call to val_print instead
+	of "current_language".  Replace call to ada_val_print by call
+	to val_print.
+	(print_variant_part): Add "language" parameter.
+	(ada_val_print_struct_union): Update call to print_field_values.
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-valprint.c (ui_memcpy): Delete.
 	(ada_print_floating): Update documentation.  Add empty line
 	between between function documentation and implementation.
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 1ae2089..8f2219f 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -39,7 +39,8 @@ static int print_field_values (struct type *, const gdb_byte *,
 			       struct ui_file *, int,
 			       const struct value *,
 			       const struct value_print_options *,
-			       int, struct type *, int);
+			       int, struct type *, int,
+			       const struct language_defn *);
 \f
 
 /* Make TYPE unsigned if its range of values includes no negatives.  */
@@ -536,7 +537,8 @@ print_variant_part (struct type *type, int field_num,
 		    const struct value *val,
 		    const struct value_print_options *options,
 		    int comma_needed,
-		    struct type *outer_type, int outer_offset)
+		    struct type *outer_type, int outer_offset,
+		    const struct language_defn *language)
 {
   struct type *var_type = TYPE_FIELD_TYPE (type, field_num);
   int which = ada_which_variant_applies (var_type, outer_type,
@@ -551,7 +553,7 @@ print_variant_part (struct type *type, int field_num,
        offset + TYPE_FIELD_BITPOS (type, field_num) / HOST_CHAR_BIT
        + TYPE_FIELD_BITPOS (var_type, which) / HOST_CHAR_BIT,
        stream, recurse, val, options,
-       comma_needed, outer_type, outer_offset);
+       comma_needed, outer_type, outer_offset, language);
 }
 
 /* Print out fields of value at VALADDR + OFFSET having structure type TYPE.
@@ -575,7 +577,8 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
 		    const struct value *val,
 		    const struct value_print_options *options,
 		    int comma_needed,
-		    struct type *outer_type, int outer_offset)
+		    struct type *outer_type, int outer_offset,
+		    const struct language_defn *language)
 {
   int i, len;
 
@@ -594,7 +597,7 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
 				(offset
 				 + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
 				stream, recurse, val, options,
-				comma_needed, type, offset);
+				comma_needed, type, offset, language);
 	  continue;
 	}
       else if (ada_is_variant_part (type, i))
@@ -603,7 +606,7 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
 	    print_variant_part (type, i, valaddr,
 				offset, stream, recurse, val,
 				options, comma_needed,
-				outer_type, outer_offset);
+				outer_type, outer_offset, language);
 	  continue;
 	}
 
@@ -657,7 +660,7 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
 			 value_contents_for_printing (v),
 			 value_embedded_offset (v), 0,
 			 stream, recurse + 1, v,
-			 &opts, current_language);
+			 &opts, language);
 	    }
 	}
       else
@@ -665,11 +668,9 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
 	  struct value_print_options opts = *options;
 
 	  opts.deref_ref = 0;
-	  ada_val_print (TYPE_FIELD_TYPE (type, i),
-			 valaddr,
-			 (offset
-			  + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
-			 0, stream, recurse + 1, val, &opts);
+	  val_print (TYPE_FIELD_TYPE (type, i), valaddr,
+		     (offset + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
+		     0, stream, recurse + 1, val, &opts, language);
 	}
       annotate_field_end ();
     }
@@ -963,7 +964,7 @@ ada_val_print_struct_union
 
   if (print_field_values (type, valaddr, offset_aligned,
 			  stream, recurse, original_value, options,
-			  0, type, offset_aligned) != 0
+			  0, type, offset_aligned, language) != 0
       && options->prettyformat)
     {
       fprintf_filtered (stream, "\n");
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 67e5cc1..8b00435 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,11 @@
 2014-01-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* gdb.ada/pp-rec-component.exp, gdb.ada/pp-rec-component.py,
+	gdb.ada/pp-rec-component/foo.adb, gdb.ada/pp-rec-component/pck.adb,
+	gdb.ada/pp-rec-component/pck.ads: New files.
+
+2014-01-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* gdb.python/py-pp-integral.c: New file.
 	* gdb.python/py-pp-integral.py: New file.
 	* gdb.python/py-pp-integral.exp: New file.
diff --git a/gdb/testsuite/gdb.ada/pp-rec-component.exp b/gdb/testsuite/gdb.ada/pp-rec-component.exp
new file mode 100644
index 0000000..faa7a0f
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/pp-rec-component.exp
@@ -0,0 +1,40 @@
+# Copyright 2014 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "ada.exp"
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
+  return -1
+}
+
+clean_restart ${testfile}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+set remote_python_file \
+    [gdb_remote_download host ${srcdir}/${subdir}/${gdb_test_file_name}.py]
+gdb_test_no_output "source ${remote_python_file}"
+
+set bp_location [gdb_get_line_number "BREAK" ${testdir}/foo.adb]
+runto "foo.adb:$bp_location"
+
+gdb_test "print before" \
+         " = Thu Nov 14 02:22:23 2013 \\(1384395743\\)"
+
+gdb_test "print /r before" \
+         " = \\(secs => 1384395743\\)"
diff --git a/gdb/testsuite/gdb.ada/pp-rec-component.py b/gdb/testsuite/gdb.ada/pp-rec-component.py
new file mode 100644
index 0000000..98bfa82
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/pp-rec-component.py
@@ -0,0 +1,35 @@
+# Copyright (C) 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from time import asctime, gmtime
+import gdb  # silence pyflakes
+
+
+class TimeTPrinter:
+    def __init__(self, val):
+        self.val = val
+
+    def to_string(self):
+        secs = int(self.val['secs'])
+        return "%s (%d)" % (asctime(gmtime(secs)), secs)
+
+
+def time_sniffer(val):
+    if val.type.tag == "pck__time_t":
+        return TimeTPrinter(val)
+    return None
+
+
+gdb.pretty_printers.append(time_sniffer)
diff --git a/gdb/testsuite/gdb.ada/pp-rec-component/foo.adb b/gdb/testsuite/gdb.ada/pp-rec-component/foo.adb
new file mode 100644
index 0000000..3f8e73b
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/pp-rec-component/foo.adb
@@ -0,0 +1,22 @@
+--  Copyright 2014 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Pck; use Pck;
+
+procedure Foo is
+   Before : Time_T := (Secs => 1384395743);
+begin
+   Do_Nothing (Before'Address);  -- BREAK
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/pp-rec-component/pck.adb b/gdb/testsuite/gdb.ada/pp-rec-component/pck.adb
new file mode 100644
index 0000000..2b31332
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/pp-rec-component/pck.adb
@@ -0,0 +1,21 @@
+--  Copyright 2014 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package body Pck is
+   procedure Do_Nothing (A : System.Address) is
+   begin
+      null;
+   end Do_Nothing;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/pp-rec-component/pck.ads b/gdb/testsuite/gdb.ada/pp-rec-component/pck.ads
new file mode 100644
index 0000000..b1652aa
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/pp-rec-component/pck.ads
@@ -0,0 +1,23 @@
+--  Copyright 2014 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with System;
+
+package Pck is
+   type Time_T is record
+      Secs : Integer;
+   end record;
+   procedure Do_Nothing (A : System.Address);
+end Pck;
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [commit/Ada] General ada-valprint improvements
  2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
                   ` (10 preceding siblings ...)
  2014-01-07  4:23 ` [commit/Ada 10/11] ada_print_floating: Remove use of statically sized buffer Joel Brobecker
@ 2014-01-07 17:29 ` Tom Tromey
  2014-01-08 11:35   ` Joel Brobecker
  11 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2014-01-07 17:29 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> This would help with partially-available values. But val_print and
Joel> value_print have subtle differences, so I'll leave that for later.

I'm curious to know what differences you are thinking of.

Tom

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [commit/Ada] General ada-valprint improvements
  2014-01-07 17:29 ` [commit/Ada] General ada-valprint improvements Tom Tromey
@ 2014-01-08 11:35   ` Joel Brobecker
  2014-01-08 15:41     ` Tom Tromey
  0 siblings, 1 reply; 17+ messages in thread
From: Joel Brobecker @ 2014-01-08 11:35 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> I'm curious to know what differences you are thinking of.

Sure!

valprint.c:val_print does a few things that value_print does not.
For instance, it handles TYPE_STUB, options->summary, etc.

On the Ada side, it's ada_value_print that does a few things
that ada_val_print does not. For instance, it prints some information
about the entity's type before printing the entity's value.

For Ada, I think untangling the extra stuff in ada_value_print
could be done by simply splitting it in two. For val_print,
I haven't even really looked at it yet. Just one of those many
things at the back of my mind.

-- 
Joel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [commit/Ada] General ada-valprint improvements
  2014-01-08 11:35   ` Joel Brobecker
@ 2014-01-08 15:41     ` Tom Tromey
  2014-01-09  3:06       ` Joel Brobecker
  0 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2014-01-08 15:41 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> I'm curious to know what differences you are thinking of.

Joel> Sure!

Joel> valprint.c:val_print does a few things that value_print does not.
Joel> For instance, it handles TYPE_STUB, options->summary, etc.

Thanks.

Joel> For Ada, I think untangling the extra stuff in ada_value_print
Joel> could be done by simply splitting it in two. For val_print,
Joel> I haven't even really looked at it yet. Just one of those many
Joel> things at the back of my mind.

I have an ancient branch where I took a stab at removing val_print.
It's a reasonably large patch and still doesn't even compile:

 30 files changed, 262 insertions(+), 486 deletions(-)

It's quite a big job and due to the way that most languages wind up
deferring to the C printer, I didn't see a nice way to do it
incrementally.

Tom

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [commit/Ada] General ada-valprint improvements
  2014-01-08 15:41     ` Tom Tromey
@ 2014-01-09  3:06       ` Joel Brobecker
  2014-01-09 16:14         ` Tom Tromey
  0 siblings, 1 reply; 17+ messages in thread
From: Joel Brobecker @ 2014-01-09  3:06 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> I have an ancient branch where I took a stab at removing val_print.
> It's a reasonably large patch and still doesn't even compile:
> 
>  30 files changed, 262 insertions(+), 486 deletions(-)
> 
> It's quite a big job and due to the way that most languages wind up
> deferring to the C printer, I didn't see a nice way to do it
> incrementally.

I was afraid of that. For Ada, I have some hopes that I could
re-implement val_print on top of value_print if I split ada_value_print.
If that works out, and if we find we can transition all languages
that way, perhaps that'll simplify the transition.

While talking crazy-large cleanup projects, I have been having this idea
in the back of my mind that I'd like the value printing to be done
by a language-independent engine, with the language only providing
decorations and hints. This is still very vague, and will require
a lot of thinking before I can even start, but the triggering thought
is varobj: varobj is already a generic object printing engine where,
if you simplify a bit, the language only provides child access.
Ideally, couldn't we extend that API to do exactly what each
LANG-print.c does? For instance, the only difference between
C structs and Ada records could be sumarized as:
  - C uses curly braces, while Ada uses parentheses
  - Ada likes to see the names of the fields

So, if we could extract out the varobj engine, and then make varobj
one formatter, and CLI-valprint another formatter, it seems like
most of the valprint units such as ada-valprint could go away.

-- 
Joel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [commit/Ada] General ada-valprint improvements
  2014-01-09  3:06       ` Joel Brobecker
@ 2014-01-09 16:14         ` Tom Tromey
  0 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2014-01-09 16:14 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> I was afraid of that. For Ada, I have some hopes that I could
Joel> re-implement val_print on top of value_print if I split ada_value_print.
Joel> If that works out, and if we find we can transition all languages
Joel> that way, perhaps that'll simplify the transition.

Yeah, that might be a workable approach.

Joel> While talking crazy-large cleanup projects, I have been having this idea
Joel> in the back of my mind that I'd like the value printing to be done
Joel> by a language-independent engine, with the language only providing
Joel> decorations and hints.
[...]
Joel> So, if we could extract out the varobj engine, and then make varobj
Joel> one formatter, and CLI-valprint another formatter, it seems like
Joel> most of the valprint units such as ada-valprint could go away.

That is an interesting idea.

FWIW I did do a tiny bit of work in this direction, see struct
generic_val_print_decorations.  This let us unify a reasonable amount
of code here.  At the time I looked at a lot of the type-code cases in
the various language value printers, but I don't remember any more
why I didn't push this idea further.

Tom

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-01-09 16:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-07  4:22 [commit/Ada] General ada-valprint improvements Joel Brobecker
2014-01-07  4:22 ` [commit/Ada 01/11] ada-valprint.c: Reorder functions to reduce advance declarations Joel Brobecker
2014-01-07  4:22 ` [commit/Ada 08/11] move ada_val_print_array down within other ada_val_print* functions Joel Brobecker
2014-01-07  4:22 ` [commit/Ada 02/11] ada_val_print_1: Add language parameter Joel Brobecker
2014-01-07  4:22 ` [commit/Ada 03/11] ada_val_print_1: Go through val_print instead of recursive call to self Joel Brobecker
2014-01-07  4:22 ` [commit/Ada 07/11] rewrite ada_val_print_ref to reduce if/else block nesting depth Joel Brobecker
2014-01-07  4:22 ` [commit/Ada 05/11] Split ada_val_print_1 into smaller functions Joel Brobecker
2014-01-07  4:22 ` [commit/Ada 06/11] ada-valprint.c: Inline print_record inside ada_val_print_struct_union Joel Brobecker
2014-01-07  4:22 ` [commit/Ada 04/11] Remove call to gdb_flush at end of ada_val_print_1 Joel Brobecker
2014-01-07  4:22 ` [commit/Ada 09/11] Extract string-printing out of ada_val_print_array Joel Brobecker
2014-01-07  4:23 ` [commit/Ada 11/11] Ada: Fix missing call to pretty-printer for fields of records Joel Brobecker
2014-01-07  4:23 ` [commit/Ada 10/11] ada_print_floating: Remove use of statically sized buffer Joel Brobecker
2014-01-07 17:29 ` [commit/Ada] General ada-valprint improvements Tom Tromey
2014-01-08 11:35   ` Joel Brobecker
2014-01-08 15:41     ` Tom Tromey
2014-01-09  3:06       ` Joel Brobecker
2014-01-09 16:14         ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).