[2/9] Define and export Guile classes for all GDB object types

Message ID 1397060028-18158-3-git-send-email-wingo@igalia.com
State Changes Requested, archived
Headers

Commit Message

Andy Wingo April 9, 2014, 4:13 p.m. UTC
  * gdb/guile/scm-gsmob.c (gdbscm_make_smob_type): Define a binding for a
  GOOPS class corresponding to the SMOB type.  In Guile 2.0, this
  binding is also exported by (oop goops), but this is no longer the
  case in Guile 2.2, so we take care of doing that here.
  (gdbscm_initialize_smobs): Load GOOPS, so that we can ensure the
  classes actually get created.

* gdb/guile/lib/gdb.scm: Export the GOOPS classes.

* gdb/testsuite/gdb.guile/scm-generics.exp: Import (gdb) in the test so
  that we have access to the <gdb:value> type in Guile 2.2.
---
 gdb/guile/lib/gdb.scm                    | 18 ++++++++++++++++++
 gdb/guile/scm-gsmob.c                    | 14 +++++++++++++-
 gdb/testsuite/gdb.guile/scm-generics.exp |  2 +-
 3 files changed, 32 insertions(+), 2 deletions(-)
  

Comments

Doug Evans April 12, 2014, 6:32 p.m. UTC | #1
Andy Wingo <wingo@igalia.com> writes:

> * gdb/guile/scm-gsmob.c (gdbscm_make_smob_type): Define a binding for a
>   GOOPS class corresponding to the SMOB type.  In Guile 2.0, this
>   binding is also exported by (oop goops), but this is no longer the
>   case in Guile 2.2, so we take care of doing that here.
>   (gdbscm_initialize_smobs): Load GOOPS, so that we can ensure the
>   classes actually get created.
>
> * gdb/guile/lib/gdb.scm: Export the GOOPS classes.
>
> * gdb/testsuite/gdb.guile/scm-generics.exp: Import (gdb) in the test so
>   that we have access to the <gdb:value> type in Guile 2.2.
> ---
>  gdb/guile/lib/gdb.scm                    | 18 ++++++++++++++++++
>  gdb/guile/scm-gsmob.c                    | 14 +++++++++++++-
>  gdb/testsuite/gdb.guile/scm-generics.exp |  2 +-
>  3 files changed, 32 insertions(+), 2 deletions(-)

Hi.
ChangeLog format issues covered in 1/9 so I won't repeat them here
(or for 3-9), except to say comments explaining "why things are the
way they are" go in the code, not the ChangeLog.
The ChangeLog entry is just a brief description of what was changed.
But feel free to add whatever extensive elaboration
you desire in the git commit log entry.

[I can imagine the above is more of the latter, which is fine, except
that comments in the code are still required.  And a properly formed
ChangeLog entry is also still required, at least until community comes
to an agreement on what changes they want to make.  I realize Guile
does things differently, but until the gdb community agrees on changes
I'm obligated to ask for adherence to existing conventions.]

> diff --git a/gdb/guile/lib/gdb.scm b/gdb/guile/lib/gdb.scm
> index f12769e..37f0934 100644
> --- a/gdb/guile/lib/gdb.scm
> +++ b/gdb/guile/lib/gdb.scm
> @@ -278,6 +278,24 @@
>   gsmob-has-property?
>   gsmob-properties
>  
> + <gdb:value>
> + <gdb:block>
> + <gdb:iterator>
> + <gdb:pretty-printer-worker>
> + <gdb:pretty-printer>
> + <gdb:sal>
> + <gdb:symtab>
> + <gdb:frame>
> + <gdb:block-symbols-iterator>
> + <gdb:field>
> + <gdb:type>
> + <gdb:arch>
> + <gdb:exception>
> + <gdb:objfile>
> + <gdb:lazy-string>
> + <gdb:breakpoint>
> + <gdb:symbol>
> +
>   ;; scm-string.c
>  
>   string->argv

The export list is organized by the file the symbols come from.
I think it would be useful to preserve that.

> diff --git a/gdb/guile/scm-gsmob.c b/gdb/guile/scm-gsmob.c
> index b0f9e19..4c88ff9 100644
> --- a/gdb/guile/scm-gsmob.c
> +++ b/gdb/guile/scm-gsmob.c
> @@ -120,7 +120,17 @@ gdbscm_is_gsmob (SCM scm)
>  scm_t_bits
>  gdbscm_make_smob_type (const char *name, size_t size)
>  {
> -  scm_t_bits result = scm_make_smob_type (name, size);
> +  scm_t_bits result;
> +  SCM klass;
> +  char *class_name;
> +
> +  result = scm_make_smob_type (name, size);
> +
> +  klass = scm_smob_class[SCM_TC2SMOBNUM (result)];
> +  gdb_assert (SCM_UNPACK (klass) != 0);
> +  class_name = xstrprintf ("<%s>", name);
> +  scm_c_define (class_name, klass);
> +  xfree (class_name);

IWBN to document here why this code is needed for 2.2 and wasn't for 2.0.

>  
>    register_gsmob (result);
>    return result;
> @@ -475,6 +485,8 @@ Return an unsorted list of names of properties." },
>  void
>  gdbscm_initialize_smobs (void)
>  {
> +  scm_c_use_module ("oop goops");

Please add a comment documenting why the use_module is needed here.

> +
>    registered_gsmobs = htab_create_alloc (10,
>  					 hash_scm_t_bits, eq_scm_t_bits,
>  					 NULL, xcalloc, xfree);
> diff --git a/gdb/testsuite/gdb.guile/scm-generics.exp b/gdb/testsuite/gdb.guile/scm-generics.exp
> index 664affc..93ab0e5 100644
> --- a/gdb/testsuite/gdb.guile/scm-generics.exp
> +++ b/gdb/testsuite/gdb.guile/scm-generics.exp
> @@ -30,7 +30,7 @@ gdb_reinitialize_dir $srcdir/$subdir
>  gdb_install_guile_utils
>  gdb_install_guile_module
>  
> -gdb_test_no_output "guile (use-modules ((oop goops)))"
> +gdb_test_no_output "guile (use-modules (oop goops) (gdb))"

gdb_install_guile_module has already imported the gdb module.
Is there an ordering dependency? [seems unlikely, but I may be missing
something]

>  
>  gdb_test_no_output "guile (define-generic +)"
>  gdb_test_no_output "guile (define-method (+ (x <gdb:value>) (y <gdb:value>)) (value-add x y))"

Thanks!
  

Patch

diff --git a/gdb/guile/lib/gdb.scm b/gdb/guile/lib/gdb.scm
index f12769e..37f0934 100644
--- a/gdb/guile/lib/gdb.scm
+++ b/gdb/guile/lib/gdb.scm
@@ -278,6 +278,24 @@ 
  gsmob-has-property?
  gsmob-properties
 
+ <gdb:value>
+ <gdb:block>
+ <gdb:iterator>
+ <gdb:pretty-printer-worker>
+ <gdb:pretty-printer>
+ <gdb:sal>
+ <gdb:symtab>
+ <gdb:frame>
+ <gdb:block-symbols-iterator>
+ <gdb:field>
+ <gdb:type>
+ <gdb:arch>
+ <gdb:exception>
+ <gdb:objfile>
+ <gdb:lazy-string>
+ <gdb:breakpoint>
+ <gdb:symbol>
+
  ;; scm-string.c
 
  string->argv
diff --git a/gdb/guile/scm-gsmob.c b/gdb/guile/scm-gsmob.c
index b0f9e19..4c88ff9 100644
--- a/gdb/guile/scm-gsmob.c
+++ b/gdb/guile/scm-gsmob.c
@@ -120,7 +120,17 @@  gdbscm_is_gsmob (SCM scm)
 scm_t_bits
 gdbscm_make_smob_type (const char *name, size_t size)
 {
-  scm_t_bits result = scm_make_smob_type (name, size);
+  scm_t_bits result;
+  SCM klass;
+  char *class_name;
+
+  result = scm_make_smob_type (name, size);
+
+  klass = scm_smob_class[SCM_TC2SMOBNUM (result)];
+  gdb_assert (SCM_UNPACK (klass) != 0);
+  class_name = xstrprintf ("<%s>", name);
+  scm_c_define (class_name, klass);
+  xfree (class_name);
 
   register_gsmob (result);
   return result;
@@ -475,6 +485,8 @@  Return an unsorted list of names of properties." },
 void
 gdbscm_initialize_smobs (void)
 {
+  scm_c_use_module ("oop goops");
+
   registered_gsmobs = htab_create_alloc (10,
 					 hash_scm_t_bits, eq_scm_t_bits,
 					 NULL, xcalloc, xfree);
diff --git a/gdb/testsuite/gdb.guile/scm-generics.exp b/gdb/testsuite/gdb.guile/scm-generics.exp
index 664affc..93ab0e5 100644
--- a/gdb/testsuite/gdb.guile/scm-generics.exp
+++ b/gdb/testsuite/gdb.guile/scm-generics.exp
@@ -30,7 +30,7 @@  gdb_reinitialize_dir $srcdir/$subdir
 gdb_install_guile_utils
 gdb_install_guile_module
 
-gdb_test_no_output "guile (use-modules ((oop goops)))"
+gdb_test_no_output "guile (use-modules (oop goops) (gdb))"
 
 gdb_test_no_output "guile (define-generic +)"
 gdb_test_no_output "guile (define-method (+ (x <gdb:value>) (y <gdb:value>)) (value-add x y))"