From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Andy Wingo Newsgroups: gmane.lisp.guile.devel Subject: Re: [Guile-commits] GNU Guile branch, goops-cleanup, created. release_1-9-4-72-gb1955b1 Date: Fri, 06 Nov 2009 00:35:13 +0100 Message-ID: References: <87eiocalhu.fsf@gnu.org> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: ger.gmane.org 1257464111 21141 80.91.229.12 (5 Nov 2009 23:35:11 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 5 Nov 2009 23:35:11 +0000 (UTC) Cc: guile-devel@gnu.org To: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Fri Nov 06 00:35:04 2009 Return-path: Envelope-to: guile-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1N6Br6-0003q4-3d for guile-devel@m.gmane.org; Fri, 06 Nov 2009 00:35:04 +0100 Original-Received: from localhost ([127.0.0.1]:49576 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1N6Br5-0007C5-CV for guile-devel@m.gmane.org; Thu, 05 Nov 2009 18:35:03 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1N6Bqx-00079e-VM for guile-devel@gnu.org; Thu, 05 Nov 2009 18:34:56 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1N6Bqt-000756-Ew for guile-devel@gnu.org; Thu, 05 Nov 2009 18:34:55 -0500 Original-Received: from [199.232.76.173] (port=40315 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1N6Bqt-00074t-4L for guile-devel@gnu.org; Thu, 05 Nov 2009 18:34:51 -0500 Original-Received: from a-pb-sasl-sd.pobox.com ([64.74.157.62]:38428 helo=sasl.smtp.pobox.com) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1N6Bqm-0002AQ-QW; Thu, 05 Nov 2009 18:34:44 -0500 Original-Received: from sasl.smtp.pobox.com (unknown [127.0.0.1]) by a-pb-sasl-sd.pobox.com (Postfix) with ESMTP id A572D935E7; Thu, 5 Nov 2009 18:34:41 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type:content-transfer-encoding; s=sasl; bh=Ur37jMXf/LHR AHgmoFDef5cw4Ng=; b=nQVyzZGQdoyWQMbEs349LS1H7WQy9JG4zNWVLysSTTtA hDOMf8Njf9P18VGDtYhkdhdAROVVozIMrbxiH9+iJQMzwmcYWB3DfphqIr8QiIZk 0HuYpwknJ8pAvY2o6csq9eaAO+2DFE1mxFyNfA4zgiX98UD1kQ8KxLI2hIo81Rk= DomainKey-Signature: a=rsa-sha1; c=nofws; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type:content-transfer-encoding; q=dns; s=sasl; b=w0TPAL Jv+UKmirUz3wfgiOx0r9DcGJgcFDVipi+0q75mVL/oiaOytjJcL2MiOdHu65tV0z RQzxCC4WmJ6Hnk7G2ZtwNA5xjjO6TNIjq1d+BIMUfe8VY9KjgUeyTlITzH+yhctc qNeBH4uw3wKGI6hm0m/kvPyNPsC/VnhqPx45o= Original-Received: from a-pb-sasl-sd.pobox.com (unknown [127.0.0.1]) by a-pb-sasl-sd.pobox.com (Postfix) with ESMTP id 955F7935E6; Thu, 5 Nov 2009 18:34:40 -0500 (EST) Original-Received: from unquote (unknown [83.32.70.88]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by a-pb-sasl-sd.pobox.com (Postfix) with ESMTPSA id 76EE1935E5; Thu, 5 Nov 2009 18:34:38 -0500 (EST) In-Reply-To: <87eiocalhu.fsf@gnu.org> ("Ludovic =?utf-8?Q?Court=C3=A8s=22'?= =?utf-8?Q?s?= message of "Thu, 05 Nov 2009 19:13:01 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.92 (gnu/linux) X-Pobox-Relay-ID: C9E82A74-CA63-11DE-9BC9-A67CBBB5EC2E-02397024!a-pb-sasl-sd.pobox.com X-detected-operating-system: by monty-python.gnu.org: Solaris 10 (beta) X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.lisp.guile.devel:9636 Archived-At: Hi, On Thu 05 Nov 2009 19:13, ludo@gnu.org (Ludovic Court=C3=A8s) writes: > Here=E2=80=99s a quick review of =E2=80=98goops-cleanup=E2=80=99. Thanks very much :-)) > "Andy Wingo" writes: > >> * libguile/deprecated.h (scm_vtable_index_vtable): Define as a synon= ym >> for scm_vtable_index_self. >> (scm_vtable_index_printer): Alias scm_vtable_index_instance_printe= r. >> (scm_struct_i_free): Alias scm_vtable_index_instance_finalize. >> (scm_struct_i_flags): Alias scm_vtable_index_flags. > > IIUC these are no longer negative indices, but why deprecate them? I think they are bad names. scm_vtable_index_vtable sounds nonsensical.=20 scm_vtable_index_printer prints instances, not the vtable itself. scm_struct_i_free is only valid on vtables, and is just a function that runs at finalization time, and doesn't actually free anything. scm_struct_i_flags is only valid on vtables. >> (SCM_STRUCTF_FLAGS): Be a -1 mask, we have a whole word now. >> (SCM_SET_VTABLE_DESTRUCTOR): Implement by hand. > > Likewise. It is now deprecated to access flags through a mask, because the mask is unnecessary. "Destructor" isn't mentioned anywhere else in Guile. >> Hidden slots. >>=20=20=20=20=20 >> * libguile/struct.c (scm_make_struct_layout): Add support for "hidde= n" >> fields, writable fields that are not visible to make-struct. This >> allows us to add fields to vtables and not break existing make-str= uct >> invocations. > > My first reaction was that it may make the struct layout code yet > hairier. Would opaque fields be usable for that purpose? In what sense > does it attempt to =E2=80=9Cnot break existing make-struct invocations=E2= =80=9D? Imagine you have a vtable vtable with an extra field. The make-struct invocation to make a vtable of that vtable-vtable is (make-struct vtable-vtable layout printer extra-field). Hidden fields allow us to add more fields to e.g. all vtables -- like a name -- without having "extra-field" being interpreted as that extra field. Opaque fields work but they're not readable or writable, which you want a name to be. It's not that bad actually. >> * libguile/struct.h: Lay things out cleaner. There are no more hidden >> (negative) words. Names are nicer. The exposition is nicer. But the >> basics are the same. The incompatibilities are that has m= ore >> slots now, and that scm_alloc_struct's signature has changed. The >> former is ameliorated by the "hidden" slots mentioned before, and = the >> latter, well, it was always a very internal thing... > > Could you eventually make the log slightly more formal, describing the > changes more than the feelings? :-) I don't know, that was such a big commit that it's hard to separate things... I'll check to see if there's something more useful I can say. > + for (i =3D 0; i < n; i++) > + { scm_t_wchar c =3D scm_i_symbol_ref (layout, i*2); > > Could you make a pass of GNU Indent or similar, Ah sorry, it's work's coding style infecting me...=20 > and =E2=80=98c-backslash-region=E2=80=99 for macros like =E2=80=98SCM_CLA= SS_CLASS_LAYOUT=E2=80=99? Sure. > + /* Class objects */ > + /* if ((SCM_CLASS_FLAGS (class) & SCM_CLASSF_METACLASS) > + && (SCM_SUBCLASSP (class, scm_class_entity_class))) > + SCM_SET_CLASS_FLAGS (obj, SCM_VTABLE_FLAG_APPLICABLE); */ > > Maybe this comment can be removed? I'm coming back to it :) > + ret =3D (scm_t_bits)scm_gc_malloc (sizeof (scm_t_bits) * (n_words + 2)= , "struct"); > + /* Now that all platforms support scm_t_uint64, I would think that mal= loc on > + all platforms is required to return 8-byte-aligned blocks. This tes= t will > + let us find out quickly though ;-) */ > + if (ret & 7) > + abort (); > > Rest assured: libgc returns 8-byte aligned data Great! Will remove. > -typedef void (*scm_t_struct_free) (scm_t_bits * vtable, scm_t_bits * dat= a); > +typedef void (*scm_t_struct_finalize) (SCM obj); > > I=E2=80=99m slightly concerned about the incompatibility. What=E2=80=99s= the rationale? > (I reckon that passing =E2=80=98scm_t_bits=E2=80=99 pointers to user code= is not very > elegant.) It was never documented, and only used by guile-gnome afaik. Better to change it to do the right thing, then document it :-) I think it should be possible to resuscitate objects too... But that's another topic. > - (let* ((vtable (make-vtable-vtable "pr" 0)) > + (let* ((vtable (make-vtable "pr")) > > Does that mean that "hello" as a layout specifier was not detected as > erroneous? Yes. Later commits cause this to raise an error. > (I=E2=80=99ve always thought that =E2=80=98make-vtable-vtable=E2=80=99 ha= s no good raison > d=E2=80=99=C3=AAtre. The GOOPS/CLOS model has only =E2=80=98make=E2=80= =99, and it makes perfect > sense to have a single procedure to =E2=80=9Cmake things out of meta-thin= gs=E2=80=9D.) A struct is an object. A vtable is a class. A vtable-vtable is a metaclass. Metaclasses are themselves classes; and classes are themselves objects. You need make-vtable-vtable to make a new strange loop at the top, like being an instance of itself. It's confusing a bit, and delightful :) See http://wingolog.org/pub/goops-inline-slots.png. >> remove support for "entities" -- a form of applicable struct >>=20=20=20=20=20 >> Entities were meant to be a form of applicable struct. Unfortunately, >> the implementation is intertwingled with generics. Removing them, for >> now, will make it possible to cleanly re-add applicable struct suppo= rt. > > Sounds good to me. It seems unlikely that these were used outside of > Guile. What do you think? I think that's about right. But they correspond to a useful thing -- applicable structs that are not generics. They'll come back, but with a less confusing name. (I hate that name, "entity".) Thanks very much for the review, I'll go through this mail and poke the branch before merging. Andy --=20 http://wingolog.org/