From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection Date: Mon, 21 Apr 2014 11:38:16 -0400 Message-ID: References: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1398095119 20856 80.91.229.3 (21 Apr 2014 15:45:19 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 21 Apr 2014 15:45:19 +0000 (UTC) Cc: emacs-devel@gnu.org To: Daniel Colascione Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Apr 21 17:45:13 2014 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1WcGP8-0008MO-V4 for ged-emacs-devel@m.gmane.org; Mon, 21 Apr 2014 17:45:11 +0200 Original-Received: from localhost ([::1]:49929 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WcGP8-00070g-J6 for ged-emacs-devel@m.gmane.org; Mon, 21 Apr 2014 11:45:10 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:42389) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WcGIi-000623-E9 for emacs-devel@gnu.org; Mon, 21 Apr 2014 11:38:39 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WcGIX-0002oY-RV for emacs-devel@gnu.org; Mon, 21 Apr 2014 11:38:32 -0400 Original-Received: from chene.dit.umontreal.ca ([132.204.246.20]:40299) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WcGIX-0002o9-KP for emacs-devel@gnu.org; Mon, 21 Apr 2014 11:38:21 -0400 Original-Received: from pastel.home (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.1/8.14.1) with ESMTP id s3LFcGq1013535; Mon, 21 Apr 2014 11:38:17 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id A74FD600CA; Mon, 21 Apr 2014 11:38:16 -0400 (EDT) In-Reply-To: (Daniel Colascione's message of "Sun, 20 Apr 2014 02:51:25 +0000") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4.50 (gnu/linux) X-NAI-Spam-Flag: NO X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 0 X-NAI-Spam-Rules: 1 Rules triggered RV4919=0 X-NAI-Spam-Version: 2.3.0.9378 : core <4919> : inlines <751> : streams <1164048> : uri <1736408> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 132.204.246.20 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:171533 Archived-At: [ 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