diff options
| author | Mattias EngdegÄrd | 2020-08-19 14:59:29 +0200 |
|---|---|---|
| committer | Mattias EngdegÄrd | 2020-08-19 15:26:34 +0200 |
| commit | 5fcb97dabd3f7b00ebc574d6be4bad16a64482de (patch) | |
| tree | 0ec796e0803703a9c1750dff6b0c63cabb693351 | |
| parent | 362ca83a3b9d74c51ac325a6490551272aa25f9a (diff) | |
| download | emacs-5fcb97dabd3f7b00ebc574d6be4bad16a64482de.tar.gz emacs-5fcb97dabd3f7b00ebc574d6be4bad16a64482de.zip | |
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.
| -rw-r--r-- | lisp/emacs-lisp/bytecomp.el | 52 | ||||
| -rw-r--r-- | 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 5479e6536a3..90745a3a2f3 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el | |||
| @@ -4172,40 +4172,44 @@ Return (TAIL VAR TEST CASES), where: | |||
| 4172 | (switch-var nil) | 4172 | (switch-var nil) |
| 4173 | (switch-test 'eq)) | 4173 | (switch-test 'eq)) |
| 4174 | (while (pcase (car clauses) | 4174 | (while (pcase (car clauses) |
| 4175 | (`((,fn ,expr1 ,expr2) . ,body) | 4175 | (`((,(and fn (or 'eq 'eql 'equal)) ,expr1 ,expr2) . ,body) |
| 4176 | (let* ((vars (byte-compile--cond-vars expr1 expr2)) | 4176 | (let* ((vars (byte-compile--cond-vars expr1 expr2)) |
| 4177 | (var (car vars)) | 4177 | (var (car vars)) |
| 4178 | (value (cdr vars))) | 4178 | (value (cdr vars))) |
| 4179 | (and var (or (eq var switch-var) (not switch-var)) | 4179 | (and var (or (eq var switch-var) (not switch-var)) |
| 4180 | (cond | 4180 | (progn |
| 4181 | ((memq fn '(eq eql equal)) | ||
| 4182 | (setq switch-var var) | 4181 | (setq switch-var var) |
| 4183 | (setq switch-test | 4182 | (setq switch-test |
| 4184 | (byte-compile--common-test switch-test fn)) | 4183 | (byte-compile--common-test switch-test fn)) |
| 4185 | (unless (member value keys) | 4184 | (unless (member value keys) |
| 4186 | (push value keys) | 4185 | (push value keys) |
| 4187 | (push (cons (list value) (or body '(t))) cases)) | 4186 | (push (cons (list value) (or body '(t))) cases)) |
| 4188 | t) | 4187 | t)))) |
| 4189 | ((and (memq fn '(memq memql member)) | 4188 | (`((,(and fn (or 'memq 'memql 'member)) ,var ,expr) . ,body) |
| 4190 | (listp value) | 4189 | (and (symbolp var) |
| 4191 | ;; Require a non-empty body, since the member | 4190 | (or (eq var switch-var) (not switch-var)) |
| 4192 | ;; function value depends on the switch | 4191 | (macroexp-const-p expr) |
| 4193 | ;; argument. | 4192 | ;; Require a non-empty body, since the member |
| 4194 | body) | 4193 | ;; function value depends on the switch argument. |
| 4195 | (setq switch-var var) | 4194 | body |
| 4196 | (setq switch-test | 4195 | (let ((value (eval expr))) |
| 4197 | (byte-compile--common-test | 4196 | (and (proper-list-p value) |
| 4198 | switch-test (cdr (assq fn '((memq . eq) | 4197 | (progn |
| 4199 | (memql . eql) | 4198 | (setq switch-var var) |
| 4200 | (member . equal)))))) | 4199 | (setq switch-test |
| 4201 | (let ((vals nil)) | 4200 | (byte-compile--common-test |
| 4202 | (dolist (elem value) | 4201 | switch-test |
| 4203 | (unless (funcall fn elem keys) | 4202 | (cdr (assq fn '((memq . eq) |
| 4204 | (push elem vals))) | 4203 | (memql . eql) |
| 4205 | (when vals | 4204 | (member . equal)))))) |
| 4206 | (setq keys (append vals keys)) | 4205 | (let ((vals nil)) |
| 4207 | (push (cons (nreverse vals) body) cases))) | 4206 | (dolist (elem value) |
| 4208 | t)))))) | 4207 | (unless (funcall fn elem keys) |
| 4208 | (push elem vals))) | ||
| 4209 | (when vals | ||
| 4210 | (setq keys (append vals keys)) | ||
| 4211 | (push (cons (nreverse vals) body) cases))) | ||
| 4212 | t)))))) | ||
| 4209 | (setq clauses (cdr clauses))) | 4213 | (setq clauses (cdr clauses))) |
| 4210 | ;; Assume that a single switch is cheaper than two or more discrete | 4214 | ;; Assume that a single switch is cheaper than two or more discrete |
| 4211 | ;; compare clauses. This could be tuned, possibly taking into | 4215 | ;; 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 a16adfedfb8..3aba9af3e79 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el | |||
| @@ -347,7 +347,20 @@ | |||
| 347 | ((eq x 't) 99) | 347 | ((eq x 't) 99) |
| 348 | (t 999)))) | 348 | (t 999)))) |
| 349 | '((a c) (b c) (7 c) (-3 c) (nil nil) (t c) (q c) (r c) (s c) | 349 | '((a c) (b c) (7 c) (-3 c) (nil nil) (t c) (q c) (r c) (s c) |
| 350 | (t c) (x "a") (x "c") (x c) (x d) (x e)))) | 350 | (t c) (x "a") (x "c") (x c) (x d) (x e))) |
| 351 | |||
| 352 | (mapcar (lambda (x) (cond ((member '(a . b) x) 1) | ||
| 353 | ((equal x '(c)) 2))) | ||
| 354 | '(((a . b)) a b (c) (d))) | ||
| 355 | (mapcar (lambda (x) (cond ((memq '(a . b) x) 1) | ||
| 356 | ((equal x '(c)) 2))) | ||
| 357 | '(((a . b)) a b (c) (d))) | ||
| 358 | (mapcar (lambda (x) (cond ((member '(a b) x) 1) | ||
| 359 | ((equal x '(c)) 2))) | ||
| 360 | '(((a b)) a b (c) (d))) | ||
| 361 | (mapcar (lambda (x) (cond ((memq '(a b) x) 1) | ||
| 362 | ((equal x '(c)) 2))) | ||
| 363 | '(((a b)) a b (c) (d)))) | ||
| 351 | "List of expression for test. | 364 | "List of expression for test. |
| 352 | Each element will be executed by interpreter and with | 365 | Each element will be executed by interpreter and with |
| 353 | bytecompiled code, and their results compared.") | 366 | bytecompiled code, and their results compared.") |