GNU bug report logs - #68863
Add support for using setf with seq-subseq

Previous Next

To reply to this bug, email your comments to 68863 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Okamsn <okamsn <at> protonmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Add support for using setf with seq-subseq
Date: Thu, 01 Feb 2024 03:31:10 +0000

[Message part 1 (text/plain, inline)]

Hello,

This patch adds support for using `seq-subseq` with `setf`, as in

     ;; => [0 1 2 10 11]
     (let ((seq (vector 0 1 2 3 4)))
       (setf (seq-subseq seq -2) (list 10 11 12 13 14))
       seq)

The patch adds a generic version which uses the existing `setf` support 
of `seq-elt` and a specialized version for modifying lists.  Both 
versions use `seq-do` to map a function over the values that should 
replace the values in the modified sequence.

To avoid modifying more values than specified, that modifying function 
uses a `when` condition. I'm not sure of a good way to stop `seq-do` 
early when we know that it can stop calling the modifying function. 
Normally, I would use `cl-block` and `cl-return`. Is it OK to use those 
features in `seq.el`? If not, is it worth adding something like a 
`seq-map-while` or a `seq-do-while`?

Thank you.

[0001-Add-setf-support-for-seq-subseq.patch (text/x-patch, attachment)]

Message #8 received at 68863 <at> debbugs.gnu.org (full text, mbox):

From: Okamsn <okamsn <at> protonmail.com>
To: 68863 <at> debbugs.gnu.org
Cc: Nicolas Petton <nicolas <at> petton.fr>
Subject: [PATCH] Add support for using setf with seq-subseq
Date: Sun, 04 Feb 2024 18:33:14 +0000

[Message part 1 (text/plain, inline)]

Hello,

I've added the maintainers e-mail address to the discussion and have 
added "[PATCH]" to the subject to make it more clear that there is a 
patch file attached.

I tested a version that uses a `seq-map-while`, but it only made a 
difference when the sequence of values to copy to the existing sequence 
was longer than 100 elements, which I would guess is a less common use case.

Thank you,
Earl

[0001-Add-setf-support-for-seq-subseq.patch (text/x-patch, attachment)]

Message #11 received at 68863 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Okamsn <okamsn <at> protonmail.com>, Nicolas Petton <nicolas <at> petton.fr>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 68863 <at> debbugs.gnu.org
Subject: Re: bug#68863: Add support for using setf with seq-subseq
Date: Thu, 08 Feb 2024 13:39:48 +0200

> Date: Thu, 01 Feb 2024 03:31:10 +0000
> From:  Okamsn via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> This patch adds support for using `seq-subseq` with `setf`, as in
> 
>      ;; => [0 1 2 10 11]
>      (let ((seq (vector 0 1 2 3 4)))
>        (setf (seq-subseq seq -2) (list 10 11 12 13 14))
>        seq)
> 
> The patch adds a generic version which uses the existing `setf` support 
> of `seq-elt` and a specialized version for modifying lists.  Both 
> versions use `seq-do` to map a function over the values that should 
> replace the values in the modified sequence.
> 
> To avoid modifying more values than specified, that modifying function 
> uses a `when` condition. I'm not sure of a good way to stop `seq-do` 
> early when we know that it can stop calling the modifying function. 
> Normally, I would use `cl-block` and `cl-return`. Is it OK to use those 
> features in `seq.el`? If not, is it worth adding something like a 
> `seq-map-while` or a `seq-do-while`?

Thanks.

Nicolas, Stefan: any comments?  seq.el is preloaded, so we should
consider whether this addition is important enough to have it in seq
or elsewhere, if we think it's useful.

Message #14 received at 68863 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Okamsn <okamsn <at> protonmail.com>, 68863 <at> debbugs.gnu.org,
 Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#68863: Add support for using setf with seq-subseq
Date: Thu, 08 Feb 2024 09:25:51 -0500

>> This patch adds support for using `seq-subseq` with `setf`, as in
>> 
>>      ;; => [0 1 2 10 11]
>>      (let ((seq (vector 0 1 2 3 4)))
>>        (setf (seq-subseq seq -2) (list 10 11 12 13 14))
>>        seq)
>> 
>> The patch adds a generic version which uses the existing `setf` support 
>> of `seq-elt` and a specialized version for modifying lists.  Both 
>> versions use `seq-do` to map a function over the values that should 
>> replace the values in the modified sequence.
> Nicolas, Stefan: any comments?

Fine by me.

>> To avoid modifying more values than specified, that modifying function 
>> uses a `when` condition. I'm not sure of a good way to stop `seq-do` 
>> early when we know that it can stop calling the modifying function. 
>> Normally, I would use `cl-block` and `cl-return`. Is it OK to use those 
>> features in `seq.el`? If not, is it worth adding something like a 
>> `seq-map-while` or a `seq-do-while`?

`seq.el` is used by some parts of the implementation of `cl-lib`, so
the use of `cl-lib` risks introducing a circular dependency.  Maybe using
`cl-block/return` would be OK, but I wouldn't be surprised if it causes
bootstrap trouble.  You can use catch/throw, OTOH.


        Stefan

Message #17 received at 68863 <at> debbugs.gnu.org (full text, mbox):

From: Okamsn <okamsn <at> protonmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 68863 <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#68863: Add support for using setf with seq-subseq
Date: Fri, 09 Feb 2024 03:54:32 +0000

[Message part 1 (text/plain, inline)]

Stefan Monnier wrote:
>>> To avoid modifying more values than specified, that modifying function
>>> uses a `when` condition. I'm not sure of a good way to stop `seq-do`
>>> early when we know that it can stop calling the modifying function.
>>> Normally, I would use `cl-block` and `cl-return`. Is it OK to use those
>>> features in `seq.el`? If not, is it worth adding something like a
>>> `seq-map-while` or a `seq-do-while`?
> 
> `seq.el` is used by some parts of the implementation of `cl-lib`, so
> the use of `cl-lib` risks introducing a circular dependency.  Maybe using
> `cl-block/return` would be OK, but I wouldn't be surprised if it causes
> bootstrap trouble.  You can use catch/throw, OTOH.
> 
> 
>          Stefan
> 

Attached is an updated version using `catch` and `throw`. Thank you for 
pointing those out to me. The patch is also changed to signal 
`args-out-of-range` for the start and end indexes to be more like 
`seq-subseq`.

How does it look?

[v2-0001-Add-setf-support-for-seq-subseq.patch (text/x-patch, attachment)]

Message #20 received at 68863 <at> debbugs.gnu.org (full text, mbox):

From: Okamsn <okamsn <at> protonmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 68863 <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#68863: Add support for using setf with seq-subseq
Date: Wed, 14 Feb 2024 02:50:29 +0000

Okamsn wrote:
> Stefan Monnier wrote:
>>>> To avoid modifying more values than specified, that modifying function
>>>> uses a `when` condition. I'm not sure of a good way to stop `seq-do`
>>>> early when we know that it can stop calling the modifying function.
>>>> Normally, I would use `cl-block` and `cl-return`. Is it OK to use those
>>>> features in `seq.el`? If not, is it worth adding something like a
>>>> `seq-map-while` or a `seq-do-while`?
>>
>> `seq.el` is used by some parts of the implementation of `cl-lib`, so
>> the use of `cl-lib` risks introducing a circular dependency.  Maybe using
>> `cl-block/return` would be OK, but I wouldn't be surprised if it causes
>> bootstrap trouble.  You can use catch/throw, OTOH.
>>
>>
>>           Stefan
>>
> 
> Attached is an updated version using `catch` and `throw`. Thank you for
> pointing those out to me. The patch is also changed to signal
> `args-out-of-range` for the start and end indexes to be more like
> `seq-subseq`.
> 
> How does it look?

Hello,

After testing it more, I see that what I've written does not work as I 
expected in the case

(let ((v    (vector (vector 0 1)
                     (vector 2 3)
                     (vector 4 5))))
   (setf (seq-subseq (seq-subseq (elt v 0) 0) 0)
         [10])
   v)

in which I would expect it to replace the first element of the first 
sub-vector with 10. I will take more time to continue working on this.

Thank you for your patience.

Message #23 received at 68863 <at> debbugs.gnu.org (full text, mbox):

From: Okamsn <okamsn <at> protonmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 68863 <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#68863: Add support for using setf with seq-subseq
Date: Thu, 18 Apr 2024 02:54:46 +0000

[Message part 1 (text/plain, inline)]

Okamsn wrote:
> Hello,
> 
> After testing it more, I see that what I've written does not work as I
> expected in the case
> 
> (let ((v    (vector (vector 0 1)
>                       (vector 2 3)
>                       (vector 4 5))))
>     (setf (seq-subseq (seq-subseq (elt v 0) 0) 0)
>           [10])
>     v)
> 
> in which I would expect it to replace the first element of the first
> sub-vector with 10. I will take more time to continue working on this.
> 
> Thank you for your patience.
> 
> 

Hello,

I found a way to work with subplaces, like in the example in my previous 
e-mail message. Instead of creating the generic feature `(setf 
seq-subseq)` like what is done for `seq-elt`, I created a generic 
function `seq-replace`, which is used in a new `gv-expander` for 
`seq-subseq`. This way of doing it is like what is done for `substring`, 
which has the behavior that I wanted.

What do you think about this approach?

Thank you.

[v3-0001-Add-seq-replace-and-setf-support-for-seq-subseq.patch (text/x-patch, attachment)]

Message #26 received at 68863 <at> debbugs.gnu.org (full text, mbox):

From: Okamsn <okamsn <at> protonmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>,
 Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 68863 <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#68863: Add support for using setf with seq-subseq
Date: Tue, 07 May 2024 01:45:44 +0000

[Message part 1 (text/plain, inline)]

Okamsn wrote:
> Okamsn wrote:
>> Hello,
>>
>> After testing it more, I see that what I've written does not work as I
>> expected in the case
>>
>> (let ((v    (vector (vector 0 1)
>>                        (vector 2 3)
>>                        (vector 4 5))))
>>      (setf (seq-subseq (seq-subseq (elt v 0) 0) 0)
>>            [10])
>>      v)
>>
>> in which I would expect it to replace the first element of the first
>> sub-vector with 10. I will take more time to continue working on this.
>>
>> Thank you for your patience.
>>
>>
> 
> Hello,
> 
> I found a way to work with subplaces, like in the example in my previous
> e-mail message. Instead of creating the generic feature `(setf
> seq-subseq)` like what is done for `seq-elt`, I created a generic
> function `seq-replace`, which is used in a new `gv-expander` for
> `seq-subseq`. This way of doing it is like what is done for `substring`,
> which has the behavior that I wanted.
> 
> What do you think about this approach?
> 
> Thank you.

Hello,

Since supporting sub-places is controversial, would you please review 
version 2 of the patch that I sent, which I have re-attached for 
convenience. This version /does not/ support sub-places.

I have added Michael Heerdegen to the recipients list in case they would 
like to comment.

Thank you.

[v2-0001-Add-setf-support-for-seq-subseq.patch (text/x-patch, attachment)]

Message #29 received at 68863 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Okamsn <okamsn <at> protonmail.com>
Cc: 68863 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Nicolas Petton <nicolas <at> petton.fr>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#68863: Add support for using setf with seq-subseq
Date: Wed, 08 May 2024 23:01:22 +0200

Okamsn <okamsn <at> protonmail.com> writes:

> Subject: [PATCH v2] Add setf support for seq-subseq.

Thank you for the update.

I wonder about the implementation for listp STOREs:

> +(cl-defmethod (setf seq-subseq) (store (sequence list) start &optional end)
> +  "Modify the elements of SEQUENCE from START to END to be those of STORE.
> +END is exclusive.
> +
> +If END is omitted, it defaults to the length of the sequence.  If
> +START or END is negative, it counts from the end. [...]

In the case where we don't count from the end:

> +error if START or END are outside of the sequence (i.e too large
> +if positive or too small if negative).
> +
> +SEQUENCE is neither lengthened nor shortened."
> +  (let* ((len (seq-length sequence))

running through the complete list just to get its length which we don't
need is a waste of time - right?

> +      (catch tag
> +        (seq-do (let ((replaced (nthcdr idx sequence)))
> +                  (lambda (v)
> +                    (if (< idx idx-end)
> +                        (setf (car replaced) v
> +                              replaced (cdr replaced)
> +                              idx (1+ idx))
> +                      (throw tag nil))))
> +                store))))
> +  store)

And wouldn't it here be more efficient to use `setcar' and `setcdr'
manipulations to insert the complete sequence (if it's a list, else we
may convert it into one) in one go instead of replacing elements one by
one?


Thanks,

Michael.

Message #32 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Okamsn via "Bug reports for GNU Emacs, the Swiss army knife of text
 editors" <bug-gnu-emacs <at> gnu.org>
Cc: Okamsn <okamsn <at> protonmail.com>, 68863 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Nicolas Petton <nicolas <at> petton.fr>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#68863: Add support for using setf with seq-subseq
Date: Thu, 09 May 2024 14:16:28 +0200

Hello again,

There is a second point I want to hint at/ discuss:

Some time ago we have removed lots of or even the majority of the
so far implemented generalized variables.

The main criteria were: are they useful, and is the semantics clear.  We
should think about whether this is the case here.

For `seq-subseq' the semantics is not completely unambiguous.  For
example, the setter could exchange a subsequence - but also simply
insert an additional piece, creating a longer sequence.  Both operations
can be useful.

Likewise, when an index (e.g. the starting position) is going beyond the
end of a sequence, Emacs could raise an error, or provide functionality
to append to the existing sequence.  Again, both kinds of behavior could
make sense in different situations.

Finally, the question of whether the operation is destructive, or which
of the involved sequences are reused, is not trivial.

Any thoughts about these points?  Is this generalized variable useful
enough to make this acceptable?  Which kind of implementation do we
prefer then?


Michael.

Message #38 received at 68863 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 68863 <at> debbugs.gnu.org, okamsn <at> protonmail.com, eliz <at> gnu.org,
 nicolas <at> petton.fr
Subject: Re: bug#68863: Add support for using setf with seq-subseq
Date: Thu, 09 May 2024 09:55:45 -0400

> Some time ago we have removed lots of or even the majority of the
> so far implemented generalized variables.
>
> The main criteria were: are they useful, and is the semantics clear.  We
> should think about whether this is the case here.

Agreed.  We should not lose track of the fact that we're talking about
"generalized variables".  Treating `(seq-subset ...)` as a "generalized
variable" (aka a "place") is stretching the idea, which we can see in the
fact that there are several different alternative ways to implement the
feature with incompatible semantics.

IOW, as a general rule a function that returns a fresh new value is
often not a good candidate for a "gv-place".  That doesn't mean we can't
accept such a change (I'm pretty sure we do have and (ab)use such
gv-places), but that it has to be weighed against its practical
usefulness.

IOW, I'd like to see existing code where we could make use of it to
simplify the code.  That might also help decide decide which semantics
would be preferable.


        Stefan

Message #41 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Okamsn <okamsn <at> protonmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>,
 "Okamsn via \"Bug reports for GNU Emacs,
 the Swiss army knife of text editors\"" <bug-gnu-emacs <at> gnu.org>
Cc: 68863 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Nicolas Petton <nicolas <at> petton.fr>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#68863: Add support for using setf with seq-subseq
Date: Tue, 14 May 2024 12:47:14 +0000

[Message part 1 (text/plain, inline)]

Michael Heerdegen wrote:
> Hello again,
> 
> There is a second point I want to hint at/ discuss:
> 
> Some time ago we have removed lots of or even the majority of the
> so far implemented generalized variables.
> 
> The main criteria were: are they useful, and is the semantics clear.  We
> should think about whether this is the case here.
> 
> For `seq-subseq' the semantics is not completely unambiguous.  For
> example, the setter could exchange a subsequence - but also simply
> insert an additional piece, creating a longer sequence.  Both operations
> can be useful.
> 
> Likewise, when an index (e.g. the starting position) is going beyond the
> end of a sequence, Emacs could raise an error, or provide functionality
> to append to the existing sequence.  Again, both kinds of behavior could
> make sense in different situations.
> 
> Finally, the question of whether the operation is destructive, or which
> of the involved sequences are reused, is not trivial.
> 
> Any thoughts about these points?  Is this generalized variable useful
> enough to make this acceptable?  Which kind of implementation do we
> prefer then?
> 
> 
> Michael.

Hello,

Here is what I have thought:

1. The setter should be consistent for different kinds of sequences. My 
understanding is that arrays cannot be extended, so I think that it 
should not be able to extend lists either. I think that something like a 
non-destructive `seq-replace` or a non-destructive `seq-splice` is a 
separate feature.

2. The setter should be consistent with how `seq-subseq` raises errors. 
`seq-subseq` raises an error if a given index is greater than the 
sequence's length, so the setter should too.

3. The setter should not modify the sequence containing the replacement 
values. The manual states: "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." I think that it makes 
sense for the setter to modify the target sequence, because `setf` is 
used to set places to values, but I think that it would be unexpected to 
modify the sequence of replacement values and that it would be contrary 
to what the manual states and how the other features behave.

I will try to find uses of `cl-replace` and maybe uses of `append` and 
`nconc` for examples. Also, I have attached a version of the patch that 
does not compute the length of the list unless needed.

Thank you.

[v4-0001-Add-setf-support-for-seq-subseq.patch (text/x-patch, attachment)]

Message #47 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Okamsn <okamsn <at> protonmail.com>
Cc: 68863 <at> debbugs.gnu.org, "Okamsn via \"Bug reports for GNU Emacs,
 the Swiss army knife of text
 editors\"" <bug-gnu-emacs <at> gnu.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#68863: Add support for using setf with seq-subseq
Date: Tue, 14 May 2024 17:52:49 +0200

Okamsn <okamsn <at> protonmail.com> writes:

> Here is what I have thought:
>
> 1. The setter should be consistent for different kinds of sequences. My
> understanding is that arrays cannot be extended, so I think that it
> should not be able to extend lists either. I think that something like a
> non-destructive `seq-replace` or a non-destructive `seq-splice` is a
> separate feature.

[...]

> 3. The setter should not modify the sequence containing the replacement
> values. The manual states: "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." I think that it makes
> sense for the setter to modify the target sequence, because `setf` is
> used to set places to values, but I think that it would be unexpected to
> modify the sequence of replacement values and that it would be contrary
> to what the manual states and how the other features behave.

That's a bit of a contradiction, since when using your patch we do
modify the original sequence.  That's a reason why your suggestion
doesn't fit that well into the current seq.el.

But The main question for me is: do we want something like you suggest
in seq.el.  And, if we do, should we then provide this functionality as
a generalized variable.  And if we do offer and advertise this
explicitly, it must be implemented as efficient as possible (which is
not the case now).

But I'm stuck with the first question.  And my gut feeling is a clear
"No".  First, I would not provide this as gv but as a normal function.
There is no necessity to go the gv way.

Second, it doesn't fit into seq.el for the reason you gave: the library
promises not to modify sequences.  This is not cut in stone, of course.


> I will try to find uses of `cl-replace` and maybe uses of `append` and
> `nconc` for examples. Also, I have attached a version of the patch that
> does not compute the length of the list unless needed.

I'm mostly interested in real-life examples where your patch would lead
to simpler, better understandable and better maintainable code; and I
think Stefan, too.

That you need such a functionality for your own code counts but is not
enough of its own.  You surely could modify your macro to do what you
want without changing Emacs.


Michael.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.