This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/9] Define and export Guile classes for all GDB object types
- From: Doug Evans <xdje42 at gmail dot com>
- To: Andy Wingo <wingo at igalia dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Sat, 12 Apr 2014 11:32:20 -0700
- Subject: Re: [PATCH 2/9] Define and export Guile classes for all GDB object types
- Authentication-results: sourceware.org; auth=none
- References: <1397060028-18158-1-git-send-email-wingo at igalia dot com> <1397060028-18158-3-git-send-email-wingo at igalia dot com>
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!