From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1707 invoked by alias); 12 Apr 2014 18:32:31 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 1587 invoked by uid 89); 12 Apr 2014 18:32:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f44.google.com Received: from mail-pa0-f44.google.com (HELO mail-pa0-f44.google.com) (209.85.220.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Sat, 12 Apr 2014 18:32:28 +0000 Received: by mail-pa0-f44.google.com with SMTP id bj1so6717922pad.3 for ; Sat, 12 Apr 2014 11:32:26 -0700 (PDT) X-Received: by 10.66.185.39 with SMTP id ez7mr34716688pac.134.1397327546094; Sat, 12 Apr 2014 11:32:26 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-53-sfba.hfc.comcastbusiness.net. [173.13.178.53]) by mx.google.com with ESMTPSA id pb7sm54413498pac.10.2014.04.12.11.32.25 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 12 Apr 2014 11:32:25 -0700 (PDT) From: Doug Evans To: Andy Wingo Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/9] Define and export Guile classes for all GDB object types References: <1397060028-18158-1-git-send-email-wingo@igalia.com> <1397060028-18158-3-git-send-email-wingo@igalia.com> Date: Sat, 12 Apr 2014 18:32:00 -0000 In-Reply-To: <1397060028-18158-3-git-send-email-wingo@igalia.com> (Andy Wingo's message of "Wed, 9 Apr 2014 18:13:41 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2014-04/txt/msg00238.txt.bz2 Andy Wingo 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 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 > > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > ;; 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 ) (y )) (value-add x y))" Thanks!