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: Mon, 21 Apr 2014 11:38:16 -0400 [thread overview]
Message-ID: <jwvy4yypvd6.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")
[ I wrote this yesterday already, but somehow it got eaten by Murphy or
something. ]
> cl-lib defstruct introspection
Please always use the ChangeLog entry as commit message.
> +The @code{cl-defstruct} package also provides a few structure
> +introspection functions.
I'm curious: when/where did you bump against a need for that?
> +@defun cl-struct-set-slot-value struct-type slot-name inst value
We don't need this, since we can always use setf instead.
> -(defun cl--const-expr-val (x)
> +(defun cl--const-expr-val (x &optional environment default)
`environment' is always macroexpand-all-environment, and `default' is
never used, so you can revert this part of the change.
> - `'(nil ,(cl--const-expr-val def))
> + `'(nil ,(cl--const-expr-val
> + def macroexpand-all-environment))
Don't go past the 80th column, please. Undoing the cl--const-expr-val
signature change will fix this problem in most of your patch.
> +(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)))
> - form)
> + (if (not (or (not (cl--compiling-file))
Unless absolutely needed, we should drop this (cl--compiling-file) test,
because cl--compiling-file is an ugly hack.
> +(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)
Please add `side-effect-free' to defun-declarations-alist, so we can
move these into `declare's, which is cleaner (it associates them with
the function itself rather than with the shared symbol).
> +(defsetf cl-struct-slot-value cl-struct-set-slot-value)
Please use (declare (gv-setter ...)) or (declare (gv-expand ...)).
Also, we should define it better such that (incf (cl-struct-slot-value ..))
only computes the offset and tests the type once.
> +(cl-define-compiler-macro cl-struct-slot-value
Please use (declare (compiler-macro ..)).
> + (&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))
How important is this optimization? I mean, if struct-type and
slot-name are constants, then presumably, the code could have used the
accessor function instead, no?
I guess this goes back to the earlier question about when/where the use
for this functionality came up.
Stefan
next prev parent reply other threads:[~2014-04-21 15:38 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 ` [Emacs-diffs] trunk r116995: cl-lib defstruct introspection Stefan Monnier
2014-04-23 12:56 ` Stefan Monnier
2014-04-21 15:38 ` Stefan Monnier [this message]
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=jwvy4yypvd6.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.