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: Sun, 20 Apr 2014 08:49:50 -0400 Message-ID: References: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1398222081 29008 80.91.229.3 (23 Apr 2014 03:01:21 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 23 Apr 2014 03:01:21 +0000 (UTC) Cc: emacs-devel@gnu.org To: Daniel Colascione Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Apr 23 05:01:14 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 1WcnQu-0005sZ-Ha for ged-emacs-devel@m.gmane.org; Wed, 23 Apr 2014 05:01:12 +0200 Original-Received: from localhost ([::1]:58346 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WcnQu-0001jK-6a for ged-emacs-devel@m.gmane.org; Tue, 22 Apr 2014 23:01:12 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:49848) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WcnQV-0000vP-Rs for emacs-devel@gnu.org; Tue, 22 Apr 2014 23:00:55 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WcnQL-0003Xp-4r for emacs-devel@gnu.org; Tue, 22 Apr 2014 23:00:47 -0400 Original-Received: from pruche.dit.umontreal.ca ([132.204.246.22]:38188) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WcnQK-0003Xj-JS for emacs-devel@gnu.org; Tue, 22 Apr 2014 23:00:36 -0400 Original-Received: from ceviche.home (lechon.iro.umontreal.ca [132.204.27.242]) by pruche.dit.umontreal.ca (8.14.1/8.14.1) with ESMTP id s3N30WiA010431; Tue, 22 Apr 2014 23:00:32 -0400 Original-Received: by ceviche.home (Postfix, from userid 20848) id D2A28665F4; Sun, 20 Apr 2014 08:49:50 -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 RV4920=0 X-NAI-Spam-Version: 2.3.0.9378 : core <4920> : inlines <759> : streams <1165012> : uri <1738093> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 132.204.246.22 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:171602 Archived-At: 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