From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Add new lisp function length= with bytecode support Date: Mon, 27 Feb 2017 18:14:28 +0200 Message-ID: <83k28bpq57.fsf@gnu.org> References: <64Kl8OYdaKer-3Ey7GHVD9He6bX8yYHaS_NjEwp7Wqc4Zb7xu8IQV3ExvjCLKlBWHVVr_HNUhd55i_BVXNHnpxjnXc6hPgWvWkc3bIO8e7s=@protonmail.com> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org X-Trace: blaine.gmane.org 1488212494 26527 195.159.176.226 (27 Feb 2017 16:21:34 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 27 Feb 2017 16:21:34 +0000 (UTC) Cc: emacs-devel@gnu.org To: Gdobbins Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Feb 27 17:21:29 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ciO3D-0006P2-9t for ged-emacs-devel@m.gmane.org; Mon, 27 Feb 2017 17:21:27 +0100 Original-Received: from localhost ([::1]:54099 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ciO3J-0004a5-1n for ged-emacs-devel@m.gmane.org; Mon, 27 Feb 2017 11:21:33 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:40079) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ciNwa-0007Rs-SC for emacs-devel@gnu.org; Mon, 27 Feb 2017 11:14:38 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ciNwX-0000fO-MD for emacs-devel@gnu.org; Mon, 27 Feb 2017 11:14:36 -0500 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:60294) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ciNwX-0000fF-J5; Mon, 27 Feb 2017 11:14:33 -0500 Original-Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:2768 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1ciNwW-0001t6-Re; Mon, 27 Feb 2017 11:14:33 -0500 In-reply-to: <64Kl8OYdaKer-3Ey7GHVD9He6bX8yYHaS_NjEwp7Wqc4Zb7xu8IQV3ExvjCLKlBWHVVr_HNUhd55i_BVXNHnpxjnXc6hPgWvWkc3bIO8e7s=@protonmail.com> (message from Gdobbins on Sun, 26 Feb 2017 17:04:26 -0500) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 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" Xref: news.gmane.org gmane.emacs.devel:212621 Archived-At: > Date: Sun, 26 Feb 2017 17:04:26 -0500 > From: Gdobbins > > The attatched patch has two commits. The first implements a new lisp function, length=, which acts the same > as the function of the same name from the Common Lisp package Alexandria. Namely it compares the length > of sequences with numbers. The second modifies the byte interpreter to treat = the same as length=, and the > byte compiler to generate the appropriate code. By greping the Emacs repo's *.el files and comparing to *.elc > files after compiling with the compiler-macro in the second commit of this patch, I estimate 14% of all calls to > length in Emacs are then simply compared via =. This makes length= quite useful, and since a large amount > of linked list traversal can be avoided, potentially more efficient. Thanks. Please allow me a few comments. > diff --git a/etc/NEWS b/etc/NEWS > index 31b7e4789e..6abcda729b 100644 > --- a/etc/NEWS > +++ b/etc/NEWS > @@ -970,6 +970,10 @@ that does not exist. > operating recursively and when some other process deletes the directory > or its files before 'delete-directory' gets to them. > > ++++ > +** The new function 'length=' compares the lengths of sequences and > +numbers. The "+++" marker means that the following entry is already described in the appropriate manual. But your changeset doesn't include any changes for the manuals. Were they forgotten? > +DEFUN ("length=", Flength_eqlsign, Slength_eqlsign, 1, MANY, 0, > + doc: /* Each element of SEQUENCES may be any type accepted by > +`length' or `='. True if the length of each sequence is equal to each > +number, otherwise returns nil. > +usage: (length= &rest SEQUENCES) */) > + (ptrdiff_t nargs, Lisp_Object *args) IMO, this doc string needs to be clarified, and will probably benefit from an example. E.g., you refer to "number", but the list of arguments doesn't seem to describe any numbers. So the semantics of this function remains unclear after reading the doc string. > + if (Fnumber_or_marker_p (args[argnum])) It would be more efficient to call NUMBERP and MARKERP directly. > + else if (! CALLN (Feqlsign, val, args[argnum])) Not sure why you call Feqlsign here. The arguments are either numbers or markers, so their direct comparison should be much more efficient, right?