GNU bug report logs - #40570
[PATCH] Alias cl-subseq to seq-subseq, define gv-setter in the latter

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 40570 in the body.
You can then email your comments to 40570 AT debbugs.gnu.org in the normal way.

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

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

From: Štěpán Němec <stepnem <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Alias cl-subseq to seq-subseq, define gv-setter in the latter
Date: Sun, 12 Apr 2020 11:46:27 +0200

The definition was moved in

2019-10-27T13:25:00-04:00!monnier <at> iro.umontreal.ca
0e4dd67aae (* lisp/emacs-lisp/seq.el: Don't require cl-lib.)

already, but not the gv-setter declaration, so 'setf' worked with
'cl-subseq', but not with 'seq-subseq'.  Moving also the gv-setter to
seq-subseq and defining 'cl-subseq' as an alias of the former makes
'setf' work with both.

* lisp/emacs-lisp/cl-extra.el (cl-subseq): Redefine as an alias of
'seq-subseq'.

* lisp/emacs-lisp/seq.el (seq-subseq): Move gv-setter declaration here
from 'cl-subseq' so that 'setf' works.
---
 lisp/emacs-lisp/cl-extra.el | 12 +++---------
 lisp/emacs-lisp/seq.el      |  5 +++++
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/lisp/emacs-lisp/cl-extra.el b/lisp/emacs-lisp/cl-extra.el
index 5bf74792c0..2583fdb19e 100644
--- a/lisp/emacs-lisp/cl-extra.el
+++ b/lisp/emacs-lisp/cl-extra.el
@@ -538,18 +538,12 @@ cl-float-limits
 ;;; Sequence functions.
 
 ;;;###autoload
-(defun cl-subseq (seq start &optional end)
-  "Return the subsequence of SEQ from START to END.
+(defalias 'cl-subseq #'seq-subseq
+  "Return the subsequence of SEQUENCE from START to END.
 If END is omitted, it defaults to the length of the sequence.
 If START or END is negative, it counts from the end.
 Signal an error if START or END are outside of the sequence (i.e
-too large if positive or too small if negative)."
-  (declare (gv-setter
-            (lambda (new)
-              (macroexp-let2 nil new new
-		`(progn (cl-replace ,seq ,new :start1 ,start :end1 ,end)
-			,new)))))
-  (seq-subseq seq start end))
+too large if positive or too small if negative).")
 
 ;;;###autoload
 (defalias 'cl-concatenate #'seq-concatenate
diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index e3037a7190..936c38283e 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -154,6 +154,11 @@ seq-subseq
 START or END is negative, it counts from the end.  Signal an
 error if START or END are outside of the sequence (i.e too large
 if positive or too small if negative)."
+  (declare (gv-setter
+            (lambda (new)
+              (macroexp-let2 nil new new
+		`(progn (cl-replace ,sequence ,new :start1 ,start :end1 ,end)
+			,new)))))
   (cond
    ((or (stringp sequence) (vectorp sequence)) (substring sequence start end))
    ((listp sequence)
-- 
2.26.0

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Štěpán Němec <stepnem <at> gmail.com>
Cc: 40570 <at> debbugs.gnu.org
Subject: Re: bug#40570: [PATCH] Alias cl-subseq to seq-subseq, define
 gv-setter in the latter
Date: Sun, 12 Apr 2020 12:18:15 -0400

> The definition was moved in
>
> 2019-10-27T13:25:00-04:00!monnier <at> iro.umontreal.ca
> 0e4dd67aae (* lisp/emacs-lisp/seq.el: Don't require cl-lib.)
>
> already, but not the gv-setter declaration, so 'setf' worked with
> 'cl-subseq', but not with 'seq-subseq'.

Indeed, when I made the move I just wanted to change the implementation
but not the featureset (AFAIK seq-subseq never supported `setf`).

So this bug report is fundamentally a feature request: make `seq-subseq`
into a (gv) generalized variable.

> --- a/lisp/emacs-lisp/seq.el
> +++ b/lisp/emacs-lisp/seq.el
> @@ -154,6 +154,11 @@ seq-subseq
>  START or END is negative, it counts from the end.  Signal an
>  error if START or END are outside of the sequence (i.e too large
>  if positive or too small if negative)."
> +  (declare (gv-setter
> +            (lambda (new)
> +              (macroexp-let2 nil new new
> +		`(progn (cl-replace ,sequence ,new :start1 ,start :end1 ,end)
> +			,new)))))

The main purpose of the move was to reverse the order of dependency so
that `cl-lib` would depend on `seq` rather than the reverse.
This implies that `seq` shouldn't use `cl-lib`.  The above `cl-replace`
is hence problematic.

Another issue is that `seq-subseq` is a generic function, so its
gv-setter should also use generic functions so that it can also be made
to work on other sequence types than the predefined ones.

IOW we should probably introduce a new `seq` generic function which does
something similar to `cl-replace`, then make `seq-subseq` use it in its
gv-setter, and ideally also make `cl-replace` use it ;-)


        Stefan

Message #13 received at 40570-done <at> debbugs.gnu.org (full text, mbox):

From: Štěpán Němec <stepnem <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 40570-done <at> debbugs.gnu.org
Subject: Re: bug#40570: [PATCH] Alias cl-subseq to seq-subseq, define
 gv-setter in the latter
Date: Sun, 12 Apr 2020 19:16:08 +0200

On Sun, 12 Apr 2020 12:18:15 -0400
Stefan Monnier wrote:

>> The definition was moved in
>>
>> 2019-10-27T13:25:00-04:00!monnier <at> iro.umontreal.ca
>> 0e4dd67aae (* lisp/emacs-lisp/seq.el: Don't require cl-lib.)
>>
>> already, but not the gv-setter declaration, so 'setf' worked with
>> 'cl-subseq', but not with 'seq-subseq'.
>
> Indeed, when I made the move I just wanted to change the implementation
> but not the featureset (AFAIK seq-subseq never supported `setf`).
>
> So this bug report is fundamentally a feature request: make `seq-subseq`
> into a (gv) generalized variable.
>
>> --- a/lisp/emacs-lisp/seq.el
>> +++ b/lisp/emacs-lisp/seq.el
>> @@ -154,6 +154,11 @@ seq-subseq
>>  START or END is negative, it counts from the end.  Signal an
>>  error if START or END are outside of the sequence (i.e too large
>>  if positive or too small if negative)."
>> +  (declare (gv-setter
>> +            (lambda (new)
>> +              (macroexp-let2 nil new new
>> +		`(progn (cl-replace ,sequence ,new :start1 ,start :end1 ,end)
>> +			,new)))))
>
> The main purpose of the move was to reverse the order of dependency so
> that `cl-lib` would depend on `seq` rather than the reverse.
> This implies that `seq` shouldn't use `cl-lib`.  The above `cl-replace`
> is hence problematic.

Oh, right... sorry.

> Another issue is that `seq-subseq` is a generic function, so its
> gv-setter should also use generic functions so that it can also be made
> to work on other sequence types than the predefined ones.
>
> IOW we should probably introduce a new `seq` generic function which does
> something similar to `cl-replace`, then make `seq-subseq` use it in its
> gv-setter, and ideally also make `cl-replace` use it ;-)

Indeed. The definition of `cl-replace' looks like not for the faint of
heart, also a lot of that impression comes from all the cl- prefixed
args and variables. Is that just an artifact of some automatic
replacement process, or is there a reason those have to have the cl-
prefix? Or a conspiracy to make cl-*.el even more impenetrable?

-- 
Štěpán

Message #16 received at 40570-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Štěpán Němec <stepnem <at> gmail.com>
Cc: 40570-done <at> debbugs.gnu.org
Subject: Re: bug#40570: [PATCH] Alias cl-subseq to seq-subseq, define
 gv-setter in the latter
Date: Sun, 12 Apr 2020 15:18:17 -0400

> Indeed. The definition of `cl-replace' looks like not for the faint of
> heart, also a lot of that impression comes from all the cl- prefixed
> args and variables. Is that just an artifact of some automatic
> replacement process, or is there a reason those have to have the cl-
> prefix?

IIRC it's just a remnant of the dynamic scoping days where higher-order
functions needed to obfuscate their local var names to avoid accident
name capture.

`cl` used it fairly systematically (not just when and where needed).

IIRC I did remove this prefix in a number of places (and kept it at
a few places where those vars are actually accessed via dynamic
scoping), but I'm sure there are lots more to clean up.


        Stefan

Previous Next

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