bug#39761: Package 'seq' uses cl-concatenate but doesn't require cl-extra

bug#39761: Package 'seq' uses cl-concatenate but doesn't require cl-extra

[Clément Pit-Claudel]

Hi there,

The following snippet throws an error:

  $ emacs -Q --batch -l seq --eval "(seq-concatenate 'list '(1 2 3) '(4 5 6))"
  Symbol’s function definition is void: cl-concatenate

Should seq.el require cl-extra?
Clément.

In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.10)
 of 2020-01-28 built on clem-w50-mint
Repository revision: 60a3c5d56c5684913e1ae77464d7c9e71bc04560
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description: Linux Mint 19.3

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD
JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LC_MONETARY: en_DK.UTF-8
  value of $LC_NUMERIC: en_DK.UTF-8
  value of $LC_TIME: sv_SE.UTF-8
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 44825 7517)
 (symbols 48 6335 1)
 (strings 32 15869 1925)
 (string-bytes 1 514592)
 (vectors 16 9396)
 (vector-slots 8 133262 8592)
 (floats 8 19 43)
 (intervals 56 185 0)
 (buffers 1000 11))

bug#39761: Package 'seq' uses cl-concatenate but doesn't require cl-extra

[Noam Postavsky]

Clément Pit-Claudel <cpitclaudel@gmail.com> writes:

> The following snippet throws an error:
>
>   $ emacs -Q --batch -l seq --eval "(seq-concatenate 'list '(1 2 3) '(4 5 6))"
>   Symbol’s function definition is void: cl-concatenate
>
> Should seq.el require cl-extra?

Originally, seq depended on cl-lib, but there was a push to reverse this
dependency, so we should do for *-concatenate what [1: 0e4dd67aae] did
for *-subseq.

[1: 0e4dd67aae]: 2019-10-27 13:25:00 -0400
  * lisp/emacs-lisp/seq.el: Don't require cl-lib.
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=0e4dd67aae8b10032317a29a6bd99d2d4a64c897

bug#39761: [PATCH] Eliminate use of cl-concatenate in 'seq' package

[Andrew Eggenberger]

Fixes (Bug#39761) by making cl-extra dependent on seq rather than
vice versa.
* lisp/emacs-lisp/seq.el (seq-concatenate): Move cl-concatenate's
code here instead of calling it.
* lisp/emacs-lisp/cl-extra.el (cl-concatenate): Use cl-concatenate.
---
 lisp/emacs-lisp/cl-extra.el | 6 +-----
 lisp/emacs-lisp/seq.el      | 6 +++++-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lisp/emacs-lisp/cl-extra.el b/lisp/emacs-lisp/cl-extra.el
index e3dabdfcef..e9bfe8df5f 100644
--- a/lisp/emacs-lisp/cl-extra.el
+++ b/lisp/emacs-lisp/cl-extra.el
@@ -556,11 +556,7 @@ cl-subseq
 (defun cl-concatenate (type &rest sequences)
   "Concatenate, into a sequence of type TYPE, the argument SEQUENCEs.
 \n(fn TYPE SEQUENCE...)"
-  (pcase type
-    ('vector (apply #'vconcat sequences))
-    ('string (apply #'concat sequences))
-    ('list (apply #'append (append sequences '(nil))))
-    (_ (error "Not a sequence type name: %S" type))))
+  (seq-concatenate type sequences))
 
 ;;; List functions.
 
diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 0b946dd736..629a7a5fb3 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -285,7 +285,11 @@ seq-concatenate
 TYPE must be one of following symbols: vector, string or list.
 
 \n(fn TYPE SEQUENCE...)"
-  (apply #'cl-concatenate type (seq-map #'seq-into-sequence sequences)))
+  (pcase type
+    ('vector (apply #'vconcat sequences))
+    ('string (apply #'concat sequences))
+    ('list (apply #'append (append sequences '(nil))))
+    (_ (error "Not a sequence type name: %S" type))))
 
 (cl-defgeneric seq-into-sequence (sequence)
   "Convert SEQUENCE into a sequence.
-- 
2.25.1

bug#39761: [PATCH] Eliminate use of cl-concatenate in 'seq' package

[Noam Postavsky]

tags 39761 + patch
quit

Andrew Eggenberger <andrew.eggenberger@gmail.com> writes:

> Fixes (Bug#39761) by making cl-extra dependent on seq rather than
> vice versa.
> * lisp/emacs-lisp/seq.el (seq-concatenate): Move cl-concatenate's
> code here instead of calling it.
> * lisp/emacs-lisp/cl-extra.el (cl-concatenate): Use cl-concatenate.

Thanks, this looks good to me.  Since this bug is a regression from
Emacs 26, it should be fixed before releasing 27.1.  Eli, is this patch
okay for emacs-27?  If no, we might also consider reverting [0e4dd67aae]
2019-10-27 "* lisp/emacs-lisp/seq.el: Don't require cl-lib." (cc'ing
Stefan, author of that patch).

bug#39761: [PATCH] Eliminate use of cl-concatenate in 'seq' package

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@gmail.com>
> Date: Tue, 03 Mar 2020 22:20:30 -0500
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 39761@debbugs.gnu.org
> 
> Andrew Eggenberger <andrew.eggenberger@gmail.com> writes:
> 
> > Fixes (Bug#39761) by making cl-extra dependent on seq rather than
> > vice versa.
> > * lisp/emacs-lisp/seq.el (seq-concatenate): Move cl-concatenate's
> > code here instead of calling it.
> > * lisp/emacs-lisp/cl-extra.el (cl-concatenate): Use cl-concatenate.
> 
> Thanks, this looks good to me.  Since this bug is a regression from
> Emacs 26, it should be fixed before releasing 27.1.  Eli, is this patch
> okay for emacs-27?

From the safety POV, I think it does.  But isn't this somewhat
inelegant?  I mean, cl-extra is lower-level infrastructure in my book
than seq.el, so this sounds backwards.  No?  Although I see that a
similar change you cited up-thread already did go in that backwards
direction...

Remind me why simply adding (require 'cl-extra) wouldn't do?

bug#39761: [PATCH] Eliminate use of cl-concatenate in 'seq' package

[Stefan Monnier]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/x-markdown; coding=UTF-8, Size: 623 bytes --]

> From the safety POV, I think it does.  But isn't this somewhat
> inelegant?  I mean, cl-extra is lower-level infrastructure in my book
> than seq.el, so this sounds backwards.

seq.el is a data structure library providing operations on sequences.
cl-extra is a mix of various things, many of them is operations on
sequences.

I don't see how/why one is naturally "above" the other.

> Remind me why simply adding (require 'cl-extra) wouldn't do?

The motivation was to be able to use sequence operations in preloaded
packages and we don't want to preload cl-lib yet (and even less
cl-extra, AFAIK).


        Stefan

bug#39761: [PATCH] Eliminate use of cl-concatenate in 'seq' package

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Noam Postavsky <npostavs@gmail.com>,  andrew.eggenberger@gmail.com,
>   39761@debbugs.gnu.org
> Date: Wed, 04 Mar 2020 12:47:25 -0500
> 
> The motivation was to be able to use sequence operations in preloaded
> packages and we don't want to preload cl-lib yet (and even less
> cl-extra, AFAIK).

Somebody needs to record these intentions in some comment, so that we
won't need to have these discussions time and again.

bug#39761: [PATCH] Eliminate use of cl-concatenate in 'seq' package

[Noam Postavsky]

tags 39761 fixed
close 39761 27.1
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> The motivation was to be able to use sequence operations in preloaded
>> packages and we don't want to preload cl-lib yet (and even less
>> cl-extra, AFAIK).
>
> Somebody needs to record these intentions in some comment, so that we
> won't need to have these discussions time and again.

Done.  For the record the emacs-devel thread about the previous change
is at <https://lists.gnu.org/r/emacs-devel/2019-10/msg00945.html>.

[1: 3cbf4cb796]: 2020-03-07 18:39:01 -0500
  Eliminate use of cl-concatenate in 'seq' package
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3cbf4cb79600ade39a186f31448e56e0e6fdd364

[2: b16ba4041d]: 2020-03-07 18:45:23 -0500
  ; lisp/emacs-lisp/seq.el: Explain why we don't use cl-lib here
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b16ba4041db928826df5f58e9bfac9fb38208145

PS Andrew, I see you did copyright assignment for some GNU other
projects, but not Emacs.  If you intend to contribute more patches to
Emacs, you might want to extend the assignment (since this one just
moved around existing lines, I just went ahead and marked it as
copyright exempt).