[commit/Ada] Add handling of "ptype TYPE_NAME.FIELD0.[...].FIELDN" expressions

[commit/Ada] Add handling of "ptype TYPE_NAME.FIELD0.[...].FIELDN" expressions

[Joel Brobecker]

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

Hello,

The subject says it all. Before the patch, you could do:

   (gdb) ptype type_name

But if you wanted to get the type of one field inside that type,
you would hit a limitation:

   (gdb) ptype circle.pos
   Invalid attempt to select from type: "circle.pos".

The attached patch enhances the debugger to able to do the above.

2008-01-04  Joel Brobecker  <brobecker@adacore.com>

        * ada-exp.y (chop_separator): New function.
        (write_selectors): Rewrite to re-use chop_separator.
        (ada_nget_field_index, get_symbol_field_type): New functions.
        (write_var_or_type): Add support for "ptype TYPENAME.FIELD"
        expressions.

As usual when I can, I wrote a testcase:

2008-01-04  Joel Brobecker  <brobecker@adacore.com>

        * gdb.ada/ptype_field/pck.ads, gdb.ada/ptype_field/pck.adb,
        gdb.ada/ptype_field/foo.adb: New files.
        * gdb.ada/ptype_field.exp: New testcase.

All tested on x86-linux. Note that I used the multi_line function
again! Very useful :)

Checked in.

-- 
Joel

[-- Attachment #2: ptype.diff --]
[-- Type: text/plain, Size: 4628 bytes --]

Index: ada-exp.y
===================================================================
--- ada-exp.y	(revision 69)
+++ ada-exp.y	(revision 70)
@@ -1112,6 +1112,22 @@ chop_selector (char *name, int end)
   return -1;
 }
 
+/* If NAME is a string beginning with a separator (either '__', or
+   '.'), chop this separator and return the result; else, return
+   NAME.  */
+
+static char *
+chop_separator (char *name)
+{
+  if (*name == '.')
+   return name + 1;
+
+  if (name[0] == '_' && name[1] == '_')
+    return name + 2;
+
+  return name;
+}
+
 /* Given that SELS is a string of the form (<sep><identifier>)*, where
    <sep> is '__' or '.', write the indicated sequence of
    STRUCTOP_STRUCT expression operators. */
@@ -1121,10 +1137,8 @@ write_selectors (char *sels)
   while (*sels != '\0')
     {
       struct stoken field_name;
-      char *p;
-      while (*sels == '_' || *sels == '.')
-	sels += 1;
-      p = sels;
+      char *p = chop_separator (sels);
+      sels = p;
       while (*sels != '\0' && *sels != '.' 
 	     && (sels[0] != '_' || sels[1] != '_'))
 	sels += 1;
@@ -1154,6 +1168,70 @@ write_ambiguous_var (struct block *block
   write_exp_elt_opcode (OP_VAR_VALUE);
 }
 
+/* A convenient wrapper around ada_get_field_index that takes
+   a non NUL-terminated FIELD_NAME0 and a FIELD_NAME_LEN instead
+   of a NUL-terminated field name.  */
+
+static int
+ada_nget_field_index (const struct type *type, const char *field_name0,
+                      int field_name_len, int maybe_missing)
+{
+  char *field_name = alloca ((field_name_len + 1) * sizeof (char));
+
+  strncpy (field_name, field_name0, field_name_len);
+  field_name[field_name_len] = '\0';
+  return ada_get_field_index (type, field_name, maybe_missing);
+}
+
+/* If encoded_field_name is the name of a field inside symbol SYM,
+   then return the type of that field.  Otherwise, return NULL.
+
+   This function is actually recursive, so if ENCODED_FIELD_NAME
+   doesn't match one of the fields of our symbol, then try to see
+   if ENCODED_FIELD_NAME could not be a succession of field names
+   (in other words, the user entered an expression of the form
+   TYPE_NAME.FIELD1.FIELD2.FIELD3), in which case we evaluate
+   each field name sequentially to obtain the desired field type.
+   In case of failure, we return NULL.  */
+
+static struct type *
+get_symbol_field_type (struct symbol *sym, char *encoded_field_name)
+{
+  char *field_name = encoded_field_name;
+  char *subfield_name;
+  struct type *type = SYMBOL_TYPE (sym);
+  int fieldno;
+
+  if (type == NULL || field_name == NULL)
+    return NULL;
+
+  while (field_name[0] != '\0')
+    {
+      field_name = chop_separator (field_name);
+
+      fieldno = ada_get_field_index (type, field_name, 1);
+      if (fieldno >= 0)
+        return TYPE_FIELD_TYPE (type, fieldno);
+
+      subfield_name = field_name;
+      while (*subfield_name != '\0' && *subfield_name != '.' 
+	     && (subfield_name[0] != '_' || subfield_name[1] != '_'))
+	subfield_name += 1;
+
+      if (subfield_name[0] == '\0')
+        return NULL;
+
+      fieldno = ada_nget_field_index (type, field_name,
+                                      subfield_name - field_name, 1);
+      if (fieldno < 0)
+        return NULL;
+
+      type = TYPE_FIELD_TYPE (type, fieldno);
+      field_name = subfield_name;
+    }
+
+  return NULL;
+}
 
 /* Look up NAME0 (an unencoded identifier or dotted name) in BLOCK (or 
    expression_block_context if NULL).  If it denotes a type, return
@@ -1252,14 +1330,21 @@ write_var_or_type (struct block *block, 
 
 	  if (type_sym != NULL)
 	    {
-	      struct type *type = SYMBOL_TYPE (type_sym);
-
-	      if (TYPE_CODE (type) == TYPE_CODE_VOID)
-		error (_("`%s' matches only void type name(s)"), name0.ptr);
-	      else if (tail_index == name_len)
-		return type;
+              struct type *field_type;
+              
+              if (tail_index == name_len)
+                return SYMBOL_TYPE (type_sym);
+
+              /* We have some extraneous characters after the type name.
+                 If this is an expression "TYPE_NAME.FIELD0.[...].FIELDN",
+                 then try to get the type of FIELDN.  */
+              field_type
+                = get_symbol_field_type (type_sym, encoded_name + tail_index);
+              if (field_type != NULL)
+                return field_type;
 	      else 
-		error (_("Invalid attempt to select from type: \"%s\"."), name0.ptr);
+		error (_("Invalid attempt to select from type: \"%s\"."),
+                       name0.ptr);
 	    }
 	  else if (tail_index == name_len && nsyms == 0)
 	    {

[-- Attachment #3: ptype-tc.diff --]
[-- Type: text/plain, Size: 5831 bytes --]

Index: gdb.ada/ptype_field.exp
===================================================================
--- gdb.ada/ptype_field.exp	(revision 0)
+++ gdb.ada/ptype_field.exp	(revision 71)
@@ -0,0 +1,70 @@
+# Copyright 2008 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/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib "ada.exp"
+
+set testdir "ptype_field"
+set testfile "${testdir}/foo"
+set srcfile ${srcdir}/${subdir}/${testfile}.adb
+set binfile ${objdir}/${subdir}/${testfile}
+
+file mkdir ${objdir}/${subdir}/${testdir}
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug additional_flags=-gnat05 ]] != "" } {
+  return -1
+}
+
+# A convenience function that joins all the arguments together,
+# with a regexp that matches zero-or-more end of lines in between
+# each argument.  This function is ideal to write the expected output
+# of a GDB command that generates more than a couple of lines, as
+# this allows us to write each line as a separate string, which is
+# easier to read by a human being.
+
+proc multi_line { args } {
+    return [join $args "\[\r\n\]*"]
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
+runto "foo.adb:$bp_location"
+
+gdb_test "ptype circle" \
+         [multi_line "type = record" \
+                     "    pos: pck\\.position;" \
+                     "    radius: integer;" \
+                     "end record" ] \
+         "ptype circle"
+
+gdb_test "ptype circle.pos" \
+         [multi_line "type = record" \
+                     "    x: integer;" \
+                     "    y: integer;" \
+                     "end record" ] \
+         "ptype circle.pos"
+
+gdb_test "ptype circle.pos.x" \
+         "type = <\[0-9\]+-byte integer>" \
+         "ptype circle.pos.x"
+
+
+
Index: gdb.ada/ptype_field/pck.adb
===================================================================
--- gdb.ada/ptype_field/pck.adb	(revision 0)
+++ gdb.ada/ptype_field/pck.adb	(revision 71)
@@ -0,0 +1,23 @@
+--  Copyright 2008 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 (C : in out Circle) is
+   begin
+      null;
+   end Do_Nothing;
+
+end Pck;
Index: gdb.ada/ptype_field/pck.ads
===================================================================
--- gdb.ada/ptype_field/pck.ads	(revision 0)
+++ gdb.ada/ptype_field/pck.ads	(revision 71)
@@ -0,0 +1,30 @@
+--  Copyright 2008 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 Pck is
+
+   type Position is record
+      X : Integer;
+      Y : Integer;
+   end record;
+
+   type Circle is record
+      Pos : Position;
+      Radius : Integer;
+   end record;
+
+   procedure Do_Nothing (C : in out Circle);
+
+end Pck;
Index: gdb.ada/ptype_field/foo.adb
===================================================================
--- gdb.ada/ptype_field/foo.adb	(revision 0)
+++ gdb.ada/ptype_field/foo.adb	(revision 71)
@@ -0,0 +1,22 @@
+--  Copyright 2008 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
+   My_Circle : Circle := (Pos => (1, 2), Radius => 3);
+begin
+   Do_Nothing (My_Circle);  -- STOP
+end Foo;

Re: [commit/Ada] Add handling of "ptype TYPE_NAME.FIELD0.[...].FIELDN" expressions

[Eli Zaretskii]

> Date: Fri, 4 Jan 2008 07:13:40 -0800
> From: Joel Brobecker <brobecker@adacore.com>
> 
> The subject says it all. Before the patch, you could do:
> 
>    (gdb) ptype type_name
> 
> But if you wanted to get the type of one field inside that type,
> you would hit a limitation:
> 
>    (gdb) ptype circle.pos
>    Invalid attempt to select from type: "circle.pos".
> 
> The attached patch enhances the debugger to able to do the above.
> 
> 2008-01-04  Joel Brobecker  <brobecker@adacore.com>
> 
>         * ada-exp.y (chop_separator): New function.
>         (write_selectors): Rewrite to re-use chop_separator.
>         (ada_nget_field_index, get_symbol_field_type): New functions.
>         (write_var_or_type): Add support for "ptype TYPENAME.FIELD"
>         expressions.

Is this an Ada-only problem?  If not, why are we fixing it only for
Ada?

Re: [commit/Ada] Add handling of "ptype TYPE_NAME.FIELD0.[...].FIELDN" expressions

[Joel Brobecker]

> > 2008-01-04  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * ada-exp.y (chop_separator): New function.
> >         (write_selectors): Rewrite to re-use chop_separator.
> >         (ada_nget_field_index, get_symbol_field_type): New functions.
> >         (write_var_or_type): Add support for "ptype TYPENAME.FIELD"
> >         expressions.
> 
> Is this an Ada-only problem?  If not, why are we fixing it only for
> Ada?

I haven't tried with C, so I don't know (I am working with our testsuite
and trying to fix all the failures I get).  The fix is necessarily
Ada-specific because the parser is parsing an Ada expression and needs
to know about the underlying encoding.

-- 
Joel

Re: [commit/Ada] Add handling of "ptype TYPE_NAME.FIELD0.[...].FIELDN" expressions

[Daniel Jacobowitz]

On Fri, Jan 04, 2008 at 07:13:40AM -0800, Joel Brobecker wrote:
> Hello,
> 
> The subject says it all. Before the patch, you could do:
> 
>    (gdb) ptype type_name
> 
> But if you wanted to get the type of one field inside that type,
> you would hit a limitation:
> 
>    (gdb) ptype circle.pos
>    Invalid attempt to select from type: "circle.pos".
> 
> The attached patch enhances the debugger to able to do the above.

Is this actually valid in Ada?  You can't do this in C, which is why
GDB doesn't handle it for ptype.


-- 
Daniel Jacobowitz
CodeSourcery

Re: [commit/Ada] Add handling of "ptype TYPE_NAME.FIELD0.[...].FIELDN" expressions

[Joel Brobecker]

> >    (gdb) ptype circle.pos
> >    Invalid attempt to select from type: "circle.pos".
> > 
> > The attached patch enhances the debugger to able to do the above.
> 
> Is this actually valid in Ada?  You can't do this in C, which is why
> GDB doesn't handle it for ptype.

Actually, it isn't. But it seemed like a natural extension...

-- 
Joel

Re: [commit/Ada] Add handling of "ptype TYPE_NAME.FIELD0.[...].FIELDN" expressions

[Andreas Schwab]

Joel Brobecker <brobecker@adacore.com> writes:

>> >    (gdb) ptype circle.pos
>> >    Invalid attempt to select from type: "circle.pos".
>> > 
>> > The attached patch enhances the debugger to able to do the above.
>> 
>> Is this actually valid in Ada?  You can't do this in C, which is why
>> GDB doesn't handle it for ptype.
>
> Actually, it isn't. But it seemed like a natural extension...

For C you can use this:

(gdb) ptype ((circle)0).pos

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [commit/Ada] Add handling of "ptype TYPE_NAME.FIELD0.[...].FIELDN" expressions

[Joel Brobecker]

> > Actually, it isn't. But it seemed like a natural extension...
> 
> For C you can use this:
> 
> (gdb) ptype ((circle)0).pos

That's clever. Thinking more about it, I'm realizing that it might
not be as natural as I first thought it would be. The fact is that
I did use it without really thinking about it a couple of times,
but it's as easy to do: ptype circle, see what the type of pos is,
then do ptype of that type, etc.

IIRC, the extension was requested by a customer, so we (AdaCore)
cannot remove it from our sources.  But we can certainly remove it
from the FSF tree if we want to.  At this point, I think it's
harmless, so I'm inclined to leave it for now.  Comments welcome.

-- 
Joel