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

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Štěpán Němec
 <stepnem <at> gmail.com>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#40570: closed ([PATCH] Alias cl-subseq to seq-subseq, define
 gv-setter in the latter)
Date: Sun, 12 Apr 2020 17:16:02 +0000

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

Your message dated Sun, 12 Apr 2020 19:16:08 +0200
with message-id <87lfn0bp5z.fsf <at> gmail.com>
and subject line Re: bug#40570: [PATCH] Alias cl-subseq to seq-subseq, define gv-setter in the latter
has caused the debbugs.gnu.org bug report #40570,
regarding [PATCH] Alias cl-subseq to seq-subseq, define gv-setter in the latter
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
40570: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=40570
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

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 part 3 (message/rfc822, inline)]

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

Previous Next

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