I looked in the manual for a place where length= should be documented, but didn't find one. I thought the docstring was sufficient. Perhaps I was mistaken. As for improving the docstring, the following is what CL Alexandria uses:
"Takes any number of sequences or integers in any order. Returns true iff the length of all the sequences and the integers are equal."
I tried to be as clear as possible without plagiarizing. An example would probably help.
The change of Fnumber_or_markerp makes sense. The call to Feqlsign was left to avoid more code duplication in light of length_eqlsign2. I found less than ten calls to length= which had more than two arguments, so the full function call to Flength_eqlsign should rarely happen, and when it does the primary time sink of the call will be to linked list traversal. But changing it is straightforward.
The attached patch implements the changes you've asked for.
-------- Original Message --------
Subject: Re: [PATCH] Add new lisp function length= with bytecode support
Local Time: February 27, 2017 11:14 AM
UTC Time: February 27, 2017 4:14 PM
From: address@hidden
To: Gdobbins <address@hidden>
address@hidden
> Date: Sun, 26 Feb 2017 17:04:26 -0500
> From: Gdobbins <address@hidden>
>
> 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?
0001-Open-code-numberp-and-markerp-and-calls-in-length.patch