From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.bob131.so (server2.bob131.so [128.199.153.143]) by sourceware.org (Postfix) with ESMTPS id BC0463833035 for ; Thu, 3 Jun 2021 19:04:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BC0463833035 Received: from internal.mail.bob131.so (localhost [127.0.0.1]) by mail.bob131.so (Postfix) with ESMTP id 0E0033FA66; Thu, 3 Jun 2021 19:04:17 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 mail.bob131.so 0E0033FA66 Date: Fri, 04 Jun 2021 05:04:14 +1000 From: George Barrett To: Andrew Burgess Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] guile: fix smob exports Message-ID: References: <20210603092254.GM2672@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20210603092254.GM2672@embecosm.com> X-Spam-Status: No, score=-6.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Jun 2021 19:04:21 -0000 On Thu, Jun 03, 2021 at 10:22:54AM +0100, Andrew Burgess wrote: > > + SCM smob_type = scm_smob_type_class (result); > > I don't know if we have a defined earliest supported version of guile, > but I tried building again guile 2.0.14 with this patch applied and > the build failed at the above line as scm_smob_type_class was not > known. Before this patch 2.0.14 would build fine. Ouch, I forgot to even check! Sorry about that. As for an earliest supported version, trying to build against 1.8 fails but Fedora still links against 2.0. So 2.0 sounds about right. This is a bit of a problem, though. Digging though the code/commit log, options for accessing smob class values from outside libguile seems to break down as follows: - <2.1.0: Export from (oop goops) - =2.1.0: N/A (!!!) - >2.1.0: scm_smob_type_class IIUC Guile's odd minor versions are development/preview releases so the lack of availability in the 2.1.0 case shouldn't have much impact, but it'll make for a nice ifdef ladder. > > + SCM smob_type_name = scm_class_name (smob_type); > > + scm_define (smob_type_name, smob_type); > > There's also scm_c_define which looks like it would do the same thing > without us having to lookup the type name? We could just pass name > through directly. The class name isn't the same as what gets passed to `scm_make_smob_type'; rather, the supplied name is implicitly enclosed in '<>'. Should I add a comment saying as much? > > +# Check that smob classes are exported properly > > Missing '.' at the end here. Ack. > > +with_test_prefix "test exports" { > > + # For is-a? and > > Could you change this to 'Import (oop goops) for is-a? and .', > this comment really puzzled me for a while. Sure. Reading it back, I'm not sure anyone could blame you for being confused ;) Thanks