From: Stefan Monnier <monnier@IRO.UMontreal.CA>
To: Daniel Colascione <dancol@dancol.org>
Cc: emacs-devel@gnu.org
Subject: Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection
Date: Sun, 20 Apr 2014 08:49:50 -0400 [thread overview]
Message-ID: <jwveh0sfaur.fsf-monnier+emacsdiffs@gnu.org> (raw)
In-Reply-To: <E1Wbhqn-0001CJ-4X@vcs.savannah.gnu.org> (Daniel Colascione's message of "Sun, 20 Apr 2014 02:51:25 +0000")
Thanks, Daniel. A few comments about this new facility.
> cl-lib defstruct introspection
This is rather short: The commit message should be a copy of the
ChangeLog text.
> +The @code{cl-defstruct} package also provides a few structure
> +introspection functions.
Curious: when/where did you bump into this need?
> +@code{setf} place. @code{cl-struct-slot-value} uses
> +@code{cl-struct-slot-offset} internally and can signal the same
> +errors.
Don't say what it does internally: just mention that it signals the same
errors as cl-struct-slot-offset. Same for cl-struct-set-slot-value
(even more so there, since you copy&pasted the text without updating
cl-struct-slot-value to cl-struct-set-slot-value).
> - `'(nil ,(cl--const-expr-val def))
> + `'(nil ,(cl--const-expr-val
> + def macroexpand-all-environment))
Please stay within 80 columns.
> +(defmacro cl-the (type form)
> + "Return FORM. If type-checking is enabled, assert that it is of TYPE."
> (declare (indent 1) (debug (cl-type-spec form)))
> + (if (not (or (not (cl--compiling-file))
Do we really need this cl--compiling-file here? If it's not absolutely
indispensible, I'd rather avoid using it, since its implementation is
a hideous unreliable hack.
this pu> +(defun cl-struct-sequence-type (struct-type)
> + "Return the sequence used to build STRUCT-TYPE.
> +STRUCT-TYPE is a symbol naming a struct type. Return 'vector or
> +'list, or nil if STRUCT-TYPE is not a struct type. "
> + (car (get struct-type 'cl-struct-type)))
> +(put 'cl-struct-sequence-type 'side-effect-free t)
Could you add a `side-effect-free' thingy to defun-declarations-alist and move
these `put' into a declare?
> +(defsetf cl-struct-slot-value cl-struct-set-slot-value)
Please don't use cl.el facilities here. Use (declare (gv-setter ...))
instead. Also, the setter should be improved such that
(incf (cl-struct-slot-value ...)) only computes the offset once.
> +(cl-define-compiler-macro cl-struct-slot-value
> +(cl-define-compiler-macro cl-struct-set-slot-value
These should use the (declare (compiler-macro ...)) facility.
This said, I wonder why they're needed (gets us back to my earlier
question about when you bumped into a need for these facilities).
> + (&whole orig struct-type slot-name inst)
> + (or (let* ((macenv macroexpand-all-environment)
> + (struct-type (cl--const-expr-val struct-type macenv))
> + (slot-name (cl--const-expr-val slot-name macenv)))
> + (and struct-type (symbolp struct-type)
> + slot-name (symbolp slot-name)
> + (assq slot-name (cl-struct-slot-info struct-type))
> + (let ((idx (cl-struct-slot-offset struct-type slot-name)))
> + (cl-ecase (cl-struct-sequence-type struct-type)
> + (vector `(aref (cl-the ,struct-type ,inst) ,idx))
> + (list `(nth ,idx (cl-the ,struct-type ,inst)))))))
> + orig))
[...]
> + (&whole orig struct-type slot-name inst value)
> + (or (let* ((macenv macroexpand-all-environment)
> + (struct-type (cl--const-expr-val struct-type macenv))
> + (slot-name (cl--const-expr-val slot-name macenv)))
> + (and struct-type (symbolp struct-type)
> + slot-name (symbolp slot-name)
> + (assq slot-name (cl-struct-slot-info struct-type))
> + (let ((idx (cl-struct-slot-offset struct-type slot-name)))
> + (cl-ecase (cl-struct-sequence-type struct-type)
> + (vector `(setf (aref (cl-the ,struct-type ,inst) ,idx)
> + ,value))
> + (list `(setf (nth ,idx (cl-the ,struct-type ,inst))
> + ,value))))))
> + orig))
Try to share code between these two.
Stefan
next parent reply other threads:[~2014-04-20 12:49 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <E1Wbhqn-0001CJ-4X@vcs.savannah.gnu.org>
2014-04-20 12:49 ` Stefan Monnier [this message]
2014-04-23 12:56 ` [Emacs-diffs] trunk r116995: cl-lib defstruct introspection Stefan Monnier
2014-04-21 15:38 ` Stefan Monnier
2014-04-21 17:40 ` Daniel Colascione
2014-04-21 22:03 ` Stefan Monnier
2014-04-21 22:26 ` Daniel Colascione
2014-04-22 2:03 ` Stefan Monnier
2014-04-22 2:07 ` Daniel Colascione
2014-04-22 3:28 ` Stefan Monnier
2014-04-23 3:18 ` Declaim and proclaim (Was: Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection) Daniel Colascione
2014-04-23 13:14 ` Declaim and proclaim Stefan Monnier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwveh0sfaur.fsf-monnier+emacsdiffs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=dancol@dancol.org \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.