Patch for allowing calling pascal-methods (fpc)

Patch for allowing calling pascal-methods (fpc)

[Joost van der Sluis]

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

Hi all,

More digging convinced me that the actual problem is in
lookup_struct_elt_type. 

Attached is a patch which makes it possible for gdb to call methods and
print the result. A testsuite-run did not show any regressions. The
patch also contains a test with an example.

Regards,

Joost van der Sluis

[-- Attachment #2: callmethods.diff --]
[-- Type: text/x-patch, Size: 4225 bytes --]

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index da9bade..f98dbe1 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1355,6 +1355,23 @@ lookup_struct_elt_type (struct type *type, char *name, int noerr)
 	}
     }
 
+  /* Check for cplus-stuf (Or Pascal-methods) */
+
+  for (i = 0; i < TYPE_NFN_FIELDS (type); i++)
+    {
+      char *t_method_name = TYPE_FN_FIELDLIST_NAME (type, i);
+
+      if (t_method_name && (strcmp_iw (t_method_name, name) == 0))
+	{
+          if (TYPE_FN_FIELDLIST_LENGTH(type, i) == 1)
+            {
+              return TYPE_FN_FIELD_TYPE (TYPE_FN_FIELDLIST1 (type, i), 0);
+            }
+          else if (!(noerr))
+            error (_("Could not determine which %s has to be used."), name);
+	}
+    }
+
   /* OK, it's not in this class.  Recursively check the baseclasses.  */
   for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--)
     {
diff --git a/gdb/testsuite/gdb.pascal/methods.exp b/gdb/testsuite/gdb.pascal/methods.exp
new file mode 100644
index 0000000..1a36ce0
--- /dev/null
+++ b/gdb/testsuite/gdb.pascal/methods.exp
@@ -0,0 +1,60 @@
+# Copyright 2011 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 "pascal.exp"
+
+set testfile "methods"
+set srcfile ${testfile}.pas
+set binfile ${objdir}/${subdir}/${testfile}$EXEEXT
+
+if {[gdb_compile_pascal "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+  return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+set bp_location1 [gdb_get_line_number "set breakpoint 1 here"]
+
+if { [gdb_breakpoint ${srcfile}:${bp_location1}] } {
+    pass "setting breakpoint 1"
+}
+
+# Verify that "start" lands inside the right procedure.
+if { [gdb_start_cmd] < 0 } {
+    untested start
+    return -1
+}
+
+gdb_test "" ".* at .*${srcfile}.*" "start"
+gdb_test "continue" ""
+
+#gdb_test "print integer_array" { = \{50, 51, 52, 53, 54, 55, 56, 57\}}
+#gdb_test "print /s integer_array" " = '23456789'" 
+
+gdb_test "print AClass.GetInt()" " = 42535" 
+#gdb_test "print /d char_array" { = \{50, 51, 52, 53, 54, 55, 56, 57\}}
+#gdb_test "print /x char_array" { = \{0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39\}}
+# Use next two times to avoid GPC line numbering problem
+#gdb_test "next" ""
+#gdb_test "next" ""
+#gdb_test "print char_array" " = '2345X789'"
+gdb_exit
+
diff --git a/gdb/testsuite/gdb.pascal/methods.pas b/gdb/testsuite/gdb.pascal/methods.pas
new file mode 100644
index 0000000..1b32d5c
--- /dev/null
+++ b/gdb/testsuite/gdb.pascal/methods.pas
@@ -0,0 +1,37 @@
+{
+ Copyright 2011 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/>.
+}
+program methods;
+
+{$mode objfpc}
+
+type
+  TMyClass = class
+    function getInt: integer;
+  end;
+
+function TMyClass.getInt: integer;
+begin
+  result := 42535;
+end;
+
+var 
+  AClass: TMyClass;
+
+begin
+  AClass := TMyClass.Create;
+  writeln(AClass.GetInt); { set breakpoint 1 here }
+end.

Re: Patch for allowing calling pascal-methods (fpc)

[Tom Tromey]

>>>>> "Joost" == Joost van der Sluis <joost@cnoc.nl> writes:

Joost> More digging convinced me that the actual problem is in
Joost> lookup_struct_elt_type. 

Could you explain why?

I am not sure that applying this patch is ok.  It is generic code, so
this change may negatively affect other languages -- it is hard to be
sure without replicating your analysis.

Perhaps some other Pascal-specific fix would be more obviously safe.

Do you have copyright papers in place?  If not, contact me off-list to
get started.

Joost> Attached is a patch which makes it possible for gdb to call methods and
Joost> print the result. A testsuite-run did not show any regressions. The
Joost> patch also contains a test with an example.

Patches should go to gdb-patches and have a ChangeLog entry.
See gdb/CONTRIBUTE.

Joost> +      if (t_method_name && (strcmp_iw (t_method_name, name) == 0))
Joost> +	{
Joost> +          if (TYPE_FN_FIELDLIST_LENGTH(type, i) == 1)
Joost> +            {
Joost> +              return TYPE_FN_FIELD_TYPE (TYPE_FN_FIELDLIST1 (type, i), 0);
Joost> +            }
Joost> +          else if (!(noerr))
Joost> +            error (_("Could not determine which %s has to be used."), name);

This seems iffy for C++.

Tom

Re: Patch for allowing calling pascal-methods (fpc)

[Joost van der Sluis]

On Wed, 2011-10-05 at 12:14 -0600, Tom Tromey wrote:
> >>>>> "Joost" == Joost van der Sluis <joost@cnoc.nl> writes:
> 
> Joost> More digging convinced me that the actual problem is in
> Joost> lookup_struct_elt_type. 
> 
> Could you explain why?

Well, the comment says it should: "Given a type TYPE, lookup the type of
the component of type named NAME". Now it does do that for fields, but
not for the FN fields. With the result that the Pascal-parser can not
handle those FN fields. (They are called FN fields, but they are in fact
(virtual)-classes related fields, so they also apply to Pascal)

> I am not sure that applying this patch is ok.  It is generic code, so
> this change may negatively affect other languages -- it is hard to be
> sure without replicating your analysis.

Thing is that this function is only used by the Pascal parser, and
within the ada-lang.c file. So it could be useful if someone with more
ada-knowledge could have a look at it. (I did have gnat installed when
doing the test-runs)

> Perhaps some other Pascal-specific fix would be more obviously safe.

I could copy lookup_struct_elt_type under a different name to p-lang.c,
and change it there. 

I could also create a wrapper-function in p-lang.c, that calls
lookup_struct_elt_type and then does the additional search within the
FN-fields. But that's very difficult (impossible?), because of the
inheritance: A field has to be searched first within it's own class, and
only if it's not found in it's base-classes.

> Do you have copyright papers in place?  If not, contact me off-list to
> get started.

Yes, those are in place.

> Joost> Attached is a patch which makes it possible for gdb to call methods and
> Joost> print the result. A testsuite-run did not show any regressions. The
> Joost> patch also contains a test with an example.
> 
> Patches should go to gdb-patches and have a ChangeLog entry.
> See gdb/CONTRIBUTE.
> 
> Joost> +      if (t_method_name && (strcmp_iw (t_method_name, name) == 0))
> Joost> +	{
> Joost> +          if (TYPE_FN_FIELDLIST_LENGTH(type, i) == 1)
> Joost> +            {
> Joost> +              return TYPE_FN_FIELD_TYPE (TYPE_FN_FIELDLIST1 (type, i), 0);
> Joost> +            }
> Joost> +          else if (!(noerr))
> Joost> +            error (_("Could not determine which %s has to be used."), name);
> 
> This seems iffy for C++.

I agree, but there is more in lookup_struct_elt_type that's iffy for C
++. But it's ada and Pascal only.

Joost

Re: Patch for allowing calling pascal-methods (fpc)

[Tom Tromey]

>>>>> "Joost" == Joost van der Sluis <joost@cnoc.nl> writes:

Tom> I am not sure that applying this patch is ok.  It is generic code, so
Tom> this change may negatively affect other languages -- it is hard to be
Tom> sure without replicating your analysis.

Joost> Thing is that this function is only used by the Pascal parser, and
Joost> within the ada-lang.c file. So it could be useful if someone with more
Joost> ada-knowledge could have a look at it. (I did have gnat installed when
Joost> doing the test-runs)

No, there are calls to it from eval.c, e.g. for the STRUCTOP_STRUCT
case, which is emitted by the C parser.

If you can prove it does not negatively affect other languages then that
would be ok.

Joost> I could also create a wrapper-function in p-lang.c, that calls
Joost> lookup_struct_elt_type and then does the additional search within the
Joost> FN-fields. But that's very difficult (impossible?), because of the
Joost> inheritance: A field has to be searched first within it's own class, and
Joost> only if it's not found in it's base-classes.

I think a new function alongside lookup_struct_elt_type in gdbtypes.c
would be fine.

Tom