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: Wed, 23 Apr 2014 08:56:34 -0400 [thread overview]
Message-ID: <jwvwqegfbot.fsf-monnier+emacsdiffs@gnu.org> (raw)
In-Reply-To: <jwveh0sfaur.fsf-monnier+emacsdiffs@gnu.org> (Stefan Monnier's message of "Sun, 20 Apr 2014 08:49:50 -0400")
Oh, well, looks like the original answer wasn't completely lost
after all. It was just stuck in some mail queue!
Make sure you disregard it, tho. I think we've hashed this enough already.
Stefan
>>>>> "Stefan" == Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
> 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 prev parent reply other threads:[~2014-04-23 12:56 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 [this message]
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=jwvwqegfbot.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.