Re: master d62766305a: Add `take` and `ntake` (bug#56521)

[Robert Pluim]

>>>>> On Mon, 18 Jul 2022 12:59:51 +0200, Mattias Engdegård <mattiase@acm.org> 
>>>>> said:

    Mattias> 18 juli 2022 kl. 10.48 skrev Robert Pluim <rpluim@gmail.com>:
    >> Does this need the usual blurb to do
    >> 
    >> (setq list-var (ntake 3 list-var))
    >> 
    >> to ensure `list-var' is updated?

    Mattias> Sort of; I added some text to that effect. Using the return value 
is only needed if N can be nonpositive.

    >> I think this would be clearer as
    >> 
    >> "If N is >= the length of LIST, return LIST (or a copy)."
    >> 
    >> since weʼre talking about the effect of various values of N.

    Mattias> Right, it's now been changed to something like that. Thank you!

That looks good, thanks

    Mattias> Now `seq-take` uses `take`, and so does `seq-subseq`. This made 
both
    Mattias> faster, but the latter was boosted immensely by not formatting an
    Mattias> error string up-front.

(meta-comment: youʼve made a number of speedups recently, it would be
great if you could show some numbers of the effects)
(meta-meta-comment: this is not a criticism of your work, just a nice-to-have)

    Mattias> `seq-subseq` would be even faster if simply written as

    Mattias>   (take (- end start) (nthcdr start sequence))

    Mattias> but the interface requirements force a much more cumbersome
    Mattias> implementation. (And does it really need `copy-sequence`? The docs 
are
    Mattias> unclear.)

(info "(elisp) Sequence Functions") says:

       All functions defined in this library are free of side-effects; i.e.,
    they do not modify any sequence (list, vector, or string) that you pass
    as an argument.

It doesnʼt say anything about the result not sharing structure with
the inputs. Probably code in the wild already implicitly depends on
the `copy-sequence', though, so Iʼd think twice before modifying that
contract.

Robert
--