On Sun 08 Apr 2012 17:17, l...@gnu.org (Ludovic Courtès) writes: > I’d like to apply these patches and associated documentation in > stable-2.0. Thoughts?
Looks great to me. A couple of thoughts: > + (define-inlinable (modifier s val) > + (if (eq? (struct-vtable s) #,type-name) > + (struct-set! s index val) > + (throw 'wrong-type-arg 'modifier > + "Wrong type argument: ~S" (list s) > + (list s))))))))) Any better abstraction here? It can be a big win to just pass the vtable to some function, because you avoid emitting code and constants. (It will be nice to start to elide some of these checks in the optimizer...) E.g. (throw-bad-struct s 'modifier). Same for the getter. ("Getter" and "setter" are better names IMO, because of other uses of the name "accessor" in Guile.) Perhaps not germane to your patch, but hey :) > (syntax-case x () > - ((_ type-name constructor-spec predicate-name field-spec ...) > + ((_ immutable? type-name constructor-spec predicate-name > + field-spec ...) I realize this is an internal macro, but it would be nice to support keywords (#:immutable), possibly without arguments... > +;; Import (srfi srfi-9)'s private module, so we can use the private > +;; `%define-record-type' macro. > +(eval-when (compile eval load) > + (module-use! (current-module) (resolve-module '(srfi srfi-9)))) Why not just use (@@ (srfi srfi-9) %define-record-type) ? > (%define-record-type)[functional-accessors]: Mimic `define-inlinable', > but add support for (ACCESSOR obj val), when `%reveal-setter' allows > it. This is confusing naming. Functional setters? Surely getters are already functional. I find the set-fields interface a bit strange, FWIW. Are you happy with it, or do you think it could be better? If you think it's really the thing to do, OK. OK those were more comments than I intended! Peace, Andy -- http://wingolog.org/