Re: [PATCH v2 2/9] Define and export Guile classes for all GDB object types

[Andy Wingo <wingo at igalia dot com>]

Hi Doug,

Thanks for looking at these patches!  I just looked through my stack and
there were only a couple that you had questions on and that were still
relevant, and this was one of them.  A refreshed patch follows.

On Tue 27 May 2014 09:35, Doug Evans <xdje42@gmail.com> writes:

> Doug Evans <xdje42@gmail.com> writes:
>
>> Andy Wingo <wingo@igalia.com> writes:
>>
>>> gdb/ChangeLog:
>>> 	* guile/scm-gsmob.c (gdbscm_make_smob_type): Define a binding
>>> 	for a GOOPS class corresponding to the SMOB type.
>>> 	(gdbscm_initialize_smobs): Load GOOPS.
>>>
>>> 	* guile/lib/gdb.scm: Export the GOOPS classes.
>>
>> No blank line between related changelog entries.

Fixed.

>> Question: Is it possible to define the goops classes at some
>> random future point [e.g., long after gdb has initialized]?
>> Some might prefer not bringing goops into the picture unless
>> explicitly requested.

We could defer GOOPS class creation, yes; what happens is that when
GOOPS is loaded, that corresponding classes are created for all known
SMOB types, and that new SMOB types will create classes as they are
defined.

But it seems a bit needless to put it off, to me -- the startup penalty
is very low (just measured it to be about 12ms on a warm cache, on 2.2
at least), and it gives a uniform way to work with gsmob values, and the
type predicate, accessors, and class get to be in the same place, etc.
Dunno, wdyt?  Let me know if you want me to rework it in some way.

Cheers,

Andy

>From 19c75afe18b1fb82e87111fe179f756fa2c5a308 Mon Sep 17 00:00:00 2001
From: Andy Wingo <wingo@igalia.com>
Date: Tue, 10 Feb 2015 15:18:52 +0100
Subject: [PATCH 1/2] Define and export Guile classes for all GDB object types

gdb/ChangeLog:
	* configure.ac: When Guile is available, check for the
	availability of 'scm_smob_type_class'.
	* guile/guile-internal.h (scm_smob_type_class)
	[!HAVE_SCM_SMOB_TYPE_CLASS]: New function.
	* guile/scm-gsmob.c (gdbscm_make_smob_type): Define a binding
	for a GOOPS class corresponding to the SMOB type.
	(gdbscm_initialize_smobs): Load GOOPS.
	* guile/lib/gdb.scm: Export the GOOPS classes.
---
 gdb/ChangeLog              | 11 +++++++++++
 gdb/configure.ac           |  5 +++--
 gdb/guile/guile-internal.h | 12 ++++++++++++
 gdb/guile/lib/gdb.scm      | 20 ++++++++++++++++++++
 gdb/guile/scm-gsmob.c      | 20 +++++++++++++++++++-
 5 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 305b4b2..3e26864 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2014-04-17  Andy Wingo  <wingo@igalia.com>
+
+	* configure.ac: When Guile is available, check for the
+	availability of 'scm_smob_type_class'.
+	* guile/guile-internal.h (scm_smob_type_class)
+	[!HAVE_SCM_SMOB_TYPE_CLASS]: New function.
+	* guile/scm-gsmob.c (gdbscm_make_smob_type): Define a binding
+	for a GOOPS class corresponding to the SMOB type.
+	(gdbscm_initialize_smobs): Load GOOPS.
+	* guile/lib/gdb.scm: Export the GOOPS classes.
+
 2015-02-09  Mark Wielaard  <mjw@redhat.com>
 
 	* dwarf2read.c (set_cu_language): Recognize DW_LANG_Fortran03 and
diff --git a/gdb/configure.ac b/gdb/configure.ac
index dfc6947..886e83b 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1195,12 +1195,13 @@ if test "${have_libguile}" != no; then
   CONFIG_INSTALL="$CONFIG_INSTALL install-guile"
   ENABLE_CFLAGS="$ENABLE_CFLAGS \$(SUBDIR_GUILE_CFLAGS)"
 
-  dnl The 'scm_new_smob' function appeared in Guile 2.0.6.
+  dnl The 'scm_new_smob' function appeared in Guile 2.0.6, and
+  dnl 'scm_smob_type_class' in 2.1.1.
   save_LIBS="$LIBS"
   save_CPPFLAGS="$CPPFLAGS"
   LIBS="$GUILE_LIBS"
   CPPFLAGS="$GUILE_CPPFLAGS"
-  AC_CHECK_FUNCS([scm_new_smob])
+  AC_CHECK_FUNCS([scm_new_smob scm_smob_type_class])
   LIBS="$save_LIBS"
   CPPFLAGS="$save_CPPFLAGS"
 else
diff --git a/gdb/guile/guile-internal.h b/gdb/guile/guile-internal.h
index 9a8ef68..7b7f592 100644
--- a/gdb/guile/guile-internal.h
+++ b/gdb/guile/guile-internal.h
@@ -109,6 +109,18 @@ scm_new_smob (scm_t_bits tc, scm_t_bits data)
 
 #endif
 
+#ifndef HAVE_SCM_SMOB_TYPE_CLASS
+
+/* Guile < 2.1.1 does not provide this function, so provide it here.  */
+
+static inline SCM
+scm_smob_type_class (scm_t_bits tc)
+{
+  return scm_smob_class[SCM_TC2SMOBNUM (tc)];
+}
+
+#endif
+
 /* Function name that is passed around in case an error needs to be reported.
    __func is in C99, but we provide a wrapper "just in case",
    and because FUNC_NAME is the canonical value used in guile sources.
diff --git a/gdb/guile/lib/gdb.scm b/gdb/guile/lib/gdb.scm
index e95914a..cc35bc1 100644
--- a/gdb/guile/lib/gdb.scm
+++ b/gdb/guile/lib/gdb.scm
@@ -50,6 +50,7 @@
 
  ;; scm-arch.c
 
+ <gdb:arch>
  arch?
  current-arch
  arch-name
@@ -85,6 +86,7 @@
 
  ;; scm-block.c
 
+ <gdb:block>
  block?
  block-valid?
  block-start
@@ -96,6 +98,7 @@
  block-global?
  block-static?
  block-symbols
+ <gdb:block-symbols-iterator>
  make-block-symbols-iterator
  block-symbols-progress?
  lookup-block
@@ -113,6 +116,7 @@
  WP_WRITE
  WP_ACCESS
 
+ <gdb:breakpoint>
  make-breakpoint
  register-breakpoint!
  delete-breakpoint!
@@ -144,6 +148,7 @@
 
  ;; scm-cmd.c
 
+ <gdb:command>
  make-command
  register-command!
  command?
@@ -176,6 +181,7 @@
 
  ;; scm-exception.c
 
+ <gdb:exception>
  make-exception
  exception?
  exception-key
@@ -200,6 +206,7 @@
  FRAME_UNWIND_NO_SAVED_PC
  FRAME_UNWIND_MEMORY_ERROR
 
+ <gdb:frame>
  frame?
  frame-valid?
  frame-name
@@ -220,6 +227,7 @@
 
  ;; scm-iterator.c
 
+ <gdb:iterator>
  make-iterator
  iterator?
  iterator-object
@@ -232,6 +240,7 @@
  ;; scm-lazy-string.c
  ;; FIXME: Where's the constructor?
 
+ <gdb:lazy-string>
  lazy-string?
  lazy-string-address
  lazy-string-length
@@ -268,6 +277,7 @@
 
  ;; scm-objfile.c
 
+ <gdb:objfile>
  objfile?
  objfile-valid?
  objfile-filename
@@ -290,6 +300,7 @@
  PARAM_FILENAME
  PARAM_ENUM
 
+ <gdb:parameter>
  make-parameter
  register-parameter!
  parameter?
@@ -313,10 +324,12 @@
 
  ;; scm-pretty-print.c
 
+ <gdb:pretty-printer>
  make-pretty-printer
  pretty-printer?
  pretty-printer-enabled?
  set-pretty-printer-enabled!
+ <gdb:pretty-printer-worker>
  make-pretty-printer-worker
  pretty-printer-worker?
  pretty-printers
@@ -324,6 +337,7 @@
 
  ;; scm-progspace.c
 
+ <gdb:progspace>
  progspace?
  progspace-valid?
  progspace-filename
@@ -367,6 +381,7 @@
  SYMBOL_FUNCTIONS_DOMAIN
  SYMBOL_TYPES_DOMAIN
 
+ <gdb:symbol>
  symbol?
  symbol-valid?
  symbol-type
@@ -387,6 +402,7 @@
 
  ;; scm-symtab.c
 
+ <gdb:symtab>
  symtab?
  symtab-valid?
  symtab-filename
@@ -394,6 +410,7 @@
  symtab-objfile
  symtab-global-block
  symtab-static-block
+ <gdb:sal>
  sal?
  sal-valid?
  sal-symtab
@@ -431,6 +448,7 @@
  TYPE_CODE_DECFLOAT
  TYPE_CODE_INTERNAL_FUNCTION
 
+ <gdb:type>
  type?
  lookup-type
  type-code
@@ -453,6 +471,7 @@
  make-field-iterator
  type-field
  type-has-field?
+ <gdb:field>
  field?
  field-name
  field-type
@@ -464,6 +483,7 @@
 
  ;; scm-value.c
 
+ <gdb:value>
  value?
  make-value
  value-optimized-out?
diff --git a/gdb/guile/scm-gsmob.c b/gdb/guile/scm-gsmob.c
index cff22af..5f8313c 100644
--- a/gdb/guile/scm-gsmob.c
+++ b/gdb/guile/scm-gsmob.c
@@ -101,7 +101,22 @@ 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;
+
+  /* In Guile 2.0, defining a new SMOB type causes the binding to also be
+     exported by (oop goops), but this is deprecated behavior and is no longer
+     the case in Guile 2.2.  Here we arrange to export the class names from
+     (gdb).  */
+
+  result = scm_make_smob_type (name, size);
+
+  klass = scm_smob_type_class (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;
@@ -285,6 +300,9 @@ Return the kind of the GDB object, e.g., <gdb:breakpoint>, as a symbol." },
 void
 gdbscm_initialize_smobs (void)
 {
+  /* Load GOOPS, so that scm_make_smob_type defines GOOPS classes eagerly.  */
+  scm_c_use_module ("oop goops");
+
   registered_gsmobs = htab_create_alloc (10,
 					 hash_scm_t_bits, eq_scm_t_bits,
 					 NULL, xcalloc, xfree);
-- 
2.1.4