|
»
»
Re: [Guile-commits] GNU Guile branch, goops-cleanup, created. release_1-9-4-72-gb1955b1
|
View:
New views
6 Messages
—
Rating Filter:
Alert me
|
 |
|
|
|
Re: [Guile-commits] GNU Guile branch, goops-cleanup, created. release_1-9-4-72-gb1955b1
by Andy Wingo
::
Rate this Message:
Hi,
On Thu 05 Nov 2009 19:13, ludo@... (Ludovic Courtès) writes: > Here’s a quick review of ‘goops-cleanup’. Thanks very much :-)) > "Andy Wingo" <wingo@...> writes: > >> * libguile/deprecated.h (scm_vtable_index_vtable): Define as a synonym >> for scm_vtable_index_self. >> (scm_vtable_index_printer): Alias scm_vtable_index_instance_printer. >> (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. 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. >> >> * libguile/struct.c (scm_make_struct_layout): Add support for "hidden" >> fields, writable fields that are not visible to make-struct. This >> allows us to add fields to vtables and not break existing make-struct >> 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 “not break existing make-struct invocations”? 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 <vtable> has more >> 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 = 0; i < n; i++) > + { scm_t_wchar c = 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... > and ‘c-backslash-region’ for macros like ‘SCM_CLASS_CLASS_LAYOUT’? 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 = (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 malloc on > + all platforms is required to return 8-byte-aligned blocks. This test 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 * data); > +typedef void (*scm_t_struct_finalize) (SCM obj); > > I’m slightly concerned about the incompatibility. What’s the rationale? > (I reckon that passing ‘scm_t_bits’ 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’ve always thought that ‘make-vtable-vtable’ has no good raison > d’être. The GOOPS/CLOS model has only ‘make’, and it makes perfect > sense to have a single procedure to “make things out of meta-things”.) 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 <class> 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 >> >> 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 support. > > 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 -- http://wingolog.org/ |
|
|
Re: [Guile-commits] GNU Guile branch, goops-cleanup, created. release_1-9-4-72-gb1955b1
by Ludovic Courtès-3
::
Rate this Message:
Hello!
Andy Wingo <wingo@...> writes: > On Thu 05 Nov 2009 19:13, ludo@... (Ludovic Courtès) writes: [...] >> "Andy Wingo" <wingo@...> writes: >> >>> * libguile/deprecated.h (scm_vtable_index_vtable): Define as a synonym >>> for scm_vtable_index_self. >>> (scm_vtable_index_printer): Alias scm_vtable_index_instance_printer. >>> (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. > 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. OK, I agree. >>> (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. OK. >>> Hidden slots. >>> >>> * libguile/struct.c (scm_make_struct_layout): Add support for "hidden" >>> fields, writable fields that are not visible to make-struct. This >>> allows us to add fields to vtables and not break existing make-struct >>> 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 “not break existing make-struct invocations”? > > 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. Understood. And what’s the use case that prompted you to implement this? >> -typedef void (*scm_t_struct_free) (scm_t_bits * vtable, scm_t_bits * data); >> +typedef void (*scm_t_struct_finalize) (SCM obj); (Can you make sure these two type names appear in the log? It makes it easier to search for them.) >> I’m slightly concerned about the incompatibility. What’s the rationale? >> (I reckon that passing ‘scm_t_bits’ 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 can’t help but think that if guile-gnome uses it, then others might as well use it. Could you make it a separate patch? >> - (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. Nice. >> (I’ve always thought that ‘make-vtable-vtable’ has no good raison >> d’être. The GOOPS/CLOS model has only ‘make’, and it makes perfect >> sense to have a single procedure to “make things out of meta-things”.) > > 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 <class> being an instance of itself. Hmm I don’t see why ‘make-vtable-vtable’ is required to make the loop. But that’s another story. > It's confusing a bit, and delightful :) See > http://wingolog.org/pub/goops-inline-slots.png. Nice diagram! :-) >> 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".) So do I. Cool, thanks! Ludo’. |
|
|
Re: [Guile-commits] GNU Guile branch, goops-cleanup, created. release_1-9-4-72-gb1955b1
by Andy Wingo
::
Rate this Message:
Hi!
On Sun 08 Nov 2009 01:41, ludo@... (Ludovic Courtès) writes: > Andy Wingo <wingo@...> writes: > >> On Thu 05 Nov 2009 19:13, ludo@... (Ludovic Courtès) writes: > >>> "Andy Wingo" <wingo@...> writes: >>> >>>> Hidden slots. >>>> >>>> * libguile/struct.c (scm_make_struct_layout): Add support for "hidden" >>>> fields, writable fields that are not visible to make-struct. This >>>> allows us to add fields to vtables and not break existing make-struct >>>> 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 “not break existing make-struct invocations”? >> >> 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. > > Understood. And what’s the use case that prompted you to implement > this? Adding a field to all vtables -- a name. Most all vtables need a name. >>> -typedef void (*scm_t_struct_free) (scm_t_bits * vtable, scm_t_bits * data); >>> +typedef void (*scm_t_struct_finalize) (SCM obj); > > (Can you make sure these two type names appear in the log? It makes it > easier to search for them.) OK. >>> I’m slightly concerned about the incompatibility. What’s the rationale? >>> (I reckon that passing ‘scm_t_bits’ 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 can’t help but think that if guile-gnome uses it, then others might as > well use it. Could you make it a separate patch? Guile-GNOME only uses it because I grubbed about for months and months and found it. It might be presumptuous, but I really don't think anyone else knew about it. But OK. I will try. I just hate revisiting that commit that I spent so much time on :/ >>> (I’ve always thought that ‘make-vtable-vtable’ has no good raison >>> d’être. The GOOPS/CLOS model has only ‘make’, and it makes perfect >>> sense to have a single procedure to “make things out of meta-things”.) >> >> 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 <class> being an instance of itself. > > Hmm I don’t see why ‘make-vtable-vtable’ is required to make the loop. > But that’s another story. CLOS also has inherent metacircularities. >> It's confusing a bit, and delightful :) See >> http://wingolog.org/pub/goops-inline-slots.png. > > Nice diagram! :-) Followed up by http://wingolog.org/archives/2009/11/09/class-redefinition-in-guile :) The branch is finished now, btw. I intend to merge whenever I get time to fix those patches. Andy -- http://wingolog.org/ |
|
|
Re: [Guile-commits] GNU Guile branch, goops-cleanup, created. release_1-9-4-72-gb1955b1
by Ken Raeburn-2
::
Rate this Message:
On Nov 21, 2009, at 13:22, Andy Wingo wrote:
>>> It's confusing a bit, and delightful :) See >>> http://wingolog.org/pub/goops-inline-slots.png. >> >> Nice diagram! :-) > > Followed up by > http://wingolog.org/archives/2009/11/09/class-redefinition-in-guile :) Nice description. But, I'm looking at it, and thinking... is that widget update done in a thread-safe manner? Ken |
|
|
Re: [Guile-commits] GNU Guile branch, goops-cleanup, created. release_1-9-4-72-gb1955b1
by Andy Wingo
::
Rate this Message:
On Mon 23 Nov 2009 09:05, Ken Raeburn <raeburn@...> writes:
> On Nov 21, 2009, at 13:22, Andy Wingo wrote: >>>> It's confusing a bit, and delightful :) See >>>> http://wingolog.org/pub/goops-inline-slots.png. >>> >>> Nice diagram! :-) >> >> Followed up by >> http://wingolog.org/archives/2009/11/09/class-redefinition-in-guile :) > > Nice description. > > But, I'm looking at it, and thinking... is that widget update done in a > thread-safe manner? There is a token nod to threadsafety with some locks in goops.c. However I would not be surprised if there were some corner cases. Andy -- http://wingolog.org/ |
| Free embeddable forum powered by Nabble | Forum Help |
