bug#42919: 27.1; Strange byte compile error with `cond' involving cons

bug#42919: 27.1; Strange byte compile error with `cond' involving cons

[Ikumi Keita]

I encountered a strange byte compile error with simple elisp code.

[How to reproduce]
1. Save the following file as /tmp/foo.el:
--- /tmp/foo.el ----------------------------
(defun xyz (arg)
  (cond
;   ((member '("image") arg) ; OK
;   ((member '(rel "image") arg) ; OK
;   ((remove '(rel . "image") arg) ; OK
;   ((memq '(rel . "image") arg) ; NG
   ((member '(rel . "image") arg) ; NG
    1)))
--------------------------------------------
2. emacs-27.1 -Q
3. M-x byte-compile-file /tmp/foo.el RET
4. Then byte compile fails with the following error:
Compiling file /tmp/foo.el at Wed Aug 19 03:38:06 2020
Entering directory ‘/tmp/’
foo.el:1:13:Error: Wrong type argument: listp, "image"
in *Compile-Log* buffer. I don't see foo.elc in /tmp after that.
Expected result is that byte compile finishes without error and foo.elc
is generated.

[Additional info]
a. As written in the above code, only cons (rel . "image") causes this
   error. Both lists (rel "image") and ("image") are OK.
b. As written in the above code, both `member' and `memq' fail while
   `remove' succeeds.
c. Emacs 26.3 works as expected.

Regards,
Ikumi Keita


In GNU Emacs 27.1 (build 1, x86_64-unknown-freebsd12.1, GTK+ Version 3.24.20)
 of 2020-08-19 built on freebsd.vmware
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: 12.1-RELEASE-p8

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Compiling /tmp/foo.el... (xyz)
You can run the command ‘byte-compile-file’ with M-x by-c RET
Compiling /tmp/foo.el... (xyz)
Mark set
next-line: End of buffer [2 times]
Mark set
Making completion list...

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND DBUS GSETTINGS GLIB NOTIFY KQUEUE ACL
GNUTLS LIBXML2 FREETYPE HARFBUZZ XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11
XDBE XIM MODULES THREADS JSON PDUMPER LCMS2 GMP

Important settings:
  value of $EMACSLOADPATH: /home/keita/elisp:
  value of $LANG: C
  locale-coding-system: nil

Major mode: Emacs-Lisp

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
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/keita/elisp/reftex-parse hides /home/keita/scr/emacs-27.1/lisp/textmodes/reftex-parse

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 seq gv
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils time-date subr-x cl-loaddefs cl-lib warnings byte-opt compile
comint ansi-color ring bytecomp byte-compile cconv 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 kqueue lcms2 dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 51013 10241)
 (symbols 48 6580 1)
 (strings 32 16915 1623)
 (string-bytes 1 565681)
 (vectors 16 10152)
 (vector-slots 8 141502 12592)
 (floats 8 25 119)
 (intervals 56 304 125)
 (buffers 1000 16))

bug#42919: 27.1; Strange byte compile error with `cond' involving cons

[Basil L. Contovounesios]

severity 42919 important
tags 42919 + confirmed
quit

Ikumi Keita <ikumi@ikumi.que.jp> writes:

> I encountered a strange byte compile error with simple elisp code.
>
> [How to reproduce]
> 1. Save the following file as /tmp/foo.el:
> --- /tmp/foo.el ----------------------------
> (defun xyz (arg)
>   (cond
> ;   ((member '("image") arg) ; OK
> ;   ((member '(rel "image") arg) ; OK
> ;   ((remove '(rel . "image") arg) ; OK
> ;   ((memq '(rel . "image") arg) ; NG
>    ((member '(rel . "image") arg) ; NG
>     1)))
> --------------------------------------------
> 2. emacs-27.1 -Q
> 3. M-x byte-compile-file /tmp/foo.el RET
> 4. Then byte compile fails with the following error:
> Compiling file /tmp/foo.el at Wed Aug 19 03:38:06 2020
> Entering directory ‘/tmp/’
> foo.el:1:13:Error: Wrong type argument: listp, "image"
> in *Compile-Log* buffer. I don't see foo.elc in /tmp after that.
> Expected result is that byte compile finishes without error and foo.elc
> is generated.
>
> [Additional info]
> a. As written in the above code, only cons (rel . "image") causes this
>    error. Both lists (rel "image") and ("image") are OK.
> b. As written in the above code, both `member' and `memq' fail while
>    `remove' succeeds.
> c. Emacs 26.3 works as expected.

Thanks, bisected to the following commit:

Compile list member functions in cond to switch (bug#36139)
36ab408207 2019-06-19 11:20:58 +0200
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=36ab408207d7adf94fd1396922e0df38d746a948

-- 
Basil

bug#42919: 27.1; Strange byte compile error with `cond' involving cons

[Mattias Engdegård]

Thank you for the excellent report! I know what is wrong; will fix immediately.

bug#42919: 27.1; Strange byte compile error with `cond' involving cons

[Mattias Engdegård]

[-- Attachment #1: Type: text/plain, Size: 132 bytes --]

I'm so sorry about this stupid error, and am very grateful that you found it.
Would you try this patch (applicable to Emacs 27.1)?


[-- Attachment #2: 0001-Fix-cond-jump-table-compilation-bug-42919.patch --]
[-- Type: application/octet-stream, Size: 6121 bytes --]

From 5fcb97dabd3f7b00ebc574d6be4bad16a64482de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Wed, 19 Aug 2020 14:59:29 +0200
Subject: [PATCH] Fix cond jump table compilation (bug#42919)

This bug affected compilation of

 (cond ((member '(some list) variable) ...) ...)

While equal is symmetric, member is not; in the latter case the
arguments must be a variable and a constant list, in that order.

Reported by Ikumi Keita.

* lisp/emacs-lisp/bytecomp.el (byte-compile--cond-switch-prefix):
Don't treat equality and member predicates in the same way; only
the former are symmetric in their arguments.
* test/lisp/emacs-lisp/bytecomp-tests.el
(byte-opt-testsuite-arith-data): Add test cases.
---
 lisp/emacs-lisp/bytecomp.el            | 52 ++++++++++++++------------
 test/lisp/emacs-lisp/bytecomp-tests.el | 15 +++++++-
 2 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 5479e6536a..90745a3a2f 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -4172,40 +4172,44 @@ byte-compile--cond-switch-prefix
         (switch-var nil)
         (switch-test 'eq))
     (while (pcase (car clauses)
-             (`((,fn ,expr1 ,expr2) . ,body)
+             (`((,(and fn (or 'eq 'eql 'equal)) ,expr1 ,expr2) . ,body)
               (let* ((vars (byte-compile--cond-vars expr1 expr2))
                      (var (car vars))
                      (value (cdr vars)))
                 (and var (or (eq var switch-var) (not switch-var))
-                     (cond
-                      ((memq fn '(eq eql equal))
+                     (progn
                        (setq switch-var var)
                        (setq switch-test
                              (byte-compile--common-test switch-test fn))
                        (unless (member value keys)
                          (push value keys)
                          (push (cons (list value) (or body '(t))) cases))
-                       t)
-                      ((and (memq fn '(memq memql member))
-                            (listp value)
-                            ;; Require a non-empty body, since the member
-                            ;; function value depends on the switch
-                            ;; argument.
-                            body)
-                       (setq switch-var var)
-                       (setq switch-test
-                             (byte-compile--common-test
-                              switch-test (cdr (assq fn '((memq   . eq)
-                                                          (memql  . eql)
-                                                          (member . equal))))))
-                       (let ((vals nil))
-                         (dolist (elem value)
-                           (unless (funcall fn elem keys)
-                             (push elem vals)))
-                         (when vals
-                           (setq keys (append vals keys))
-                           (push (cons (nreverse vals) body) cases)))
-                       t))))))
+                       t))))
+             (`((,(and fn (or 'memq 'memql 'member)) ,var ,expr) . ,body)
+              (and (symbolp var)
+                   (or (eq var switch-var) (not switch-var))
+                   (macroexp-const-p expr)
+                   ;; Require a non-empty body, since the member
+                   ;; function value depends on the switch argument.
+                   body
+                   (let ((value (eval expr)))
+                     (and (proper-list-p value)
+                          (progn
+                            (setq switch-var var)
+                            (setq switch-test
+                                  (byte-compile--common-test
+                                   switch-test
+                                   (cdr (assq fn '((memq   . eq)
+                                                   (memql  . eql)
+                                                   (member . equal))))))
+                            (let ((vals nil))
+                              (dolist (elem value)
+                                (unless (funcall fn elem keys)
+                                  (push elem vals)))
+                              (when vals
+                                (setq keys (append vals keys))
+                                (push (cons (nreverse vals) body) cases)))
+                            t))))))
       (setq clauses (cdr clauses)))
     ;; Assume that a single switch is cheaper than two or more discrete
     ;; compare clauses.  This could be tuned, possibly taking into
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index a16adfedfb..3aba9af3e7 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -347,7 +347,20 @@ byte-opt-testsuite-arith-data
                                 ((eq x 't) 99)
                                 (t 999))))
             '((a c) (b c) (7 c) (-3 c) (nil nil) (t c) (q c) (r c) (s c)
-              (t c) (x "a") (x "c") (x c) (x d) (x e))))
+              (t c) (x "a") (x "c") (x c) (x d) (x e)))
+
+    (mapcar (lambda (x) (cond ((member '(a . b) x) 1)
+                              ((equal x '(c)) 2)))
+            '(((a . b)) a b (c) (d)))
+    (mapcar (lambda (x) (cond ((memq '(a . b) x) 1)
+                              ((equal x '(c)) 2)))
+            '(((a . b)) a b (c) (d)))
+    (mapcar (lambda (x) (cond ((member '(a b) x) 1)
+                              ((equal x '(c)) 2)))
+            '(((a b)) a b (c) (d)))
+    (mapcar (lambda (x) (cond ((memq '(a b) x) 1)
+                              ((equal x '(c)) 2)))
+            '(((a b)) a b (c) (d))))
   "List of expression for test.
 Each element will be executed by interpreter and with
 bytecompiled code, and their results compared.")
-- 
2.21.1 (Apple Git-122.3)

bug#42919: 27.1; Strange byte compile error with `cond' involving cons

[Ikumi Keita]

Hi Mattias,

>>>>> Mattias Engdegård <mattiase@acm.org> writes:
> I'm so sorry about this stupid error, and am very grateful that you found it.
> Would you try this patch (applicable to Emacs 27.1)?

Thanks, it now runs as expected! :-)

Regards,
Ikumi Keita

bug#42919: 27.1; Strange byte compile error with `cond' involving cons

[Mattias Engdegård]

19 aug. 2020 kl. 15.57 skrev Ikumi Keita <ikumi@ikumi.que.jp>:

> Thanks, it now runs as expected! :-)

Good with such a speedy confirmation! Since it will go into the release branch, I'll just wait for Stefan to say that it doesn't look completely bonkers before pushing.

bug#42919: 27.1; Strange byte compile error with `cond' involving cons

[Mattias Engdegård]

Pushed to emacs-27; closing.