* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
@ 2019-05-17 9:33 Mattias Engdegård
2019-05-17 12:16 ` Stefan Monnier
0 siblings, 1 reply; 16+ messages in thread
From: Mattias Engdegård @ 2019-05-17 9:33 UTC (permalink / raw)
To: 35770; +Cc: Stefan Monnier, vibhavp
[-- Attachment #1: Type: text/plain, Size: 543 bytes --]
The byte-code compiler attempts to eliminate duplicated cases when turning a `cond' into a switch table, as in
(cond ((eq x 'a) 1)
(eq x 'b) 2)
(eq x 'a) 3)) ; remove
but it doesn't work properly, because of a confusion between expressions and their values, and a logic error that would have discarded the entire table instead of just skipped the duplicate.
This patch attempts to rectify that. I also removed a seemingly redundant condition, the (consp condition), which should always be true at that point.
[-- Attachment #2: 0001-Correctly-eliminate-duplicate-cases-in-switch-compil.patch --]
[-- Type: application/octet-stream, Size: 2503 bytes --]
From adb7adf2271e65023adc0ce5d6251054b55c6452 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Fri, 17 May 2019 11:25:06 +0200
Subject: [PATCH] Correctly eliminate duplicate cases in switch compilation
Fix code mistakes that prevented the correct elimination of duplicated
cases when compiling a `cond' form to a switch bytecode, as in
(cond ((eq x 'a) 1)
((eq x 'b) 2)
((eq x 'a) 3)) ; should be elided
* lisp/emacs-lisp/bytecomp.el (byte-compile-cond-vars): Return obj2 eval'ed.
(byte-compile-cond-jump-table-info):
Discard redundant condition. Use `obj2' as evaluated.
Discard duplicated cases instead of failing the table generation.
---
lisp/emacs-lisp/bytecomp.el | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index e76baf5ed0..7f7cbe5a68 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -4091,8 +4091,8 @@ that suppresses all warnings during execution of BODY."
;; and the other is a constant expression whose value can be
;; compared with `eq' (with `macroexp-const-p').
(or
- (and (symbolp obj1) (macroexp-const-p obj2) (cons obj1 obj2))
- (and (symbolp obj2) (macroexp-const-p obj1) (cons obj2 obj1))))
+ (and (symbolp obj1) (macroexp-const-p obj2) (cons obj1 (eval obj2)))
+ (and (symbolp obj2) (macroexp-const-p obj1) (cons obj2 (eval obj1)))))
(defconst byte-compile--default-val (cons nil nil) "A unique object.")
@@ -4121,12 +4121,11 @@ Return a list of the form ((TEST . VAR) ((VALUE BODY) ...))"
(unless prev-test
(setq prev-test test))
(if (and obj1 (memq test '(eq eql equal))
- (consp condition)
(eq test prev-test)
- (eq obj1 prev-var)
- ;; discard duplicate clauses
- (not (assq obj2 cases)))
- (push (list (if (consp obj2) (eval obj2) obj2) body) cases)
+ (eq obj1 prev-var))
+ ;; discard duplicate clauses
+ (unless (assq obj2 cases)
+ (push (list obj2 body) cases))
(if (and (macroexp-const-p condition) condition)
(progn (push (list byte-compile--default-val
(or body `(,condition)))
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-17 9:33 bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation Mattias Engdegård
@ 2019-05-17 12:16 ` Stefan Monnier
2019-05-17 14:48 ` Mattias Engdegård
0 siblings, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2019-05-17 12:16 UTC (permalink / raw)
To: Mattias Engdegård; +Cc: 35770, vibhavp
> * lisp/emacs-lisp/bytecomp.el (byte-compile-cond-vars): Return obj2 eval'ed.
> (byte-compile-cond-jump-table-info):
> Discard redundant condition. Use `obj2' as evaluated.
> Discard duplicated cases instead of failing the table generation.
The patch looks fine, but could you add corresponding regression tests?
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-17 12:16 ` Stefan Monnier
@ 2019-05-17 14:48 ` Mattias Engdegård
2019-05-22 10:52 ` Mattias Engdegård
0 siblings, 1 reply; 16+ messages in thread
From: Mattias Engdegård @ 2019-05-17 14:48 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 35770, vibhavp
[-- Attachment #1: Type: text/plain, Size: 629 bytes --]
fre 2019-05-17 klockan 08:16 -0400 skrev Stefan Monnier:
> The patch looks fine, but could you add corresponding regression
> tests?
Added. I wasn't sure how you test these things, but I both added some
functional tests and some kind of check of the bytecode. If you find
the latter superfluous, I can drop it; I think the functional tests are
reliable enough. In other words, we have a case of miscompilation.
There was another bug in the original code which survived into the
first patch: the check for duplicate keys was done with `assq', which
of course doesn't work for stuff compared with `eql' or `equal'. Now
fixed.
[-- Attachment #2: 0001-Correctly-eliminate-duplicate-cases-in-switch-compil.patch --]
[-- Type: text/x-patch, Size: 5886 bytes --]
From 8438bc036bb60622c5c2e03582ba2cf471baf998 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Fri, 17 May 2019 11:25:06 +0200
Subject: [PATCH] Correctly eliminate duplicate cases in switch compilation
Fix code mistakes that prevented the correct elimination of duplicated
cases when compiling a `cond' form to a switch bytecode, as in
(cond ((eq x 'a) 1)
((eq x 'b) 2)
((eq x 'a) 3) ; should be elided
((eq x 'c) 4))
Sometimes, this caused the bytecode to use the wrong branch (bug#35770).
* lisp/emacs-lisp/bytecomp.el (byte-compile-cond-vars): Return obj2 eval'ed.
(byte-compile-cond-jump-table-info):
Discard redundant condition. Use `obj2' as evaluated.
Discard duplicated cases instead of failing the table generation.
* test/lisp/emacs-lisp/bytecomp-tests.el (toplevel): Require subr-x.
(byte-opt-testsuite-arith-data, bytecomp-test--switch-duplicates): Test.
---
lisp/emacs-lisp/bytecomp.el | 13 +++---
test/lisp/emacs-lisp/bytecomp-tests.el | 55 +++++++++++++++++++++++++-
2 files changed, 60 insertions(+), 8 deletions(-)
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index e76baf5ed0..ce348ed313 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -4091,8 +4091,8 @@ byte-compile-cond-vars
;; and the other is a constant expression whose value can be
;; compared with `eq' (with `macroexp-const-p').
(or
- (and (symbolp obj1) (macroexp-const-p obj2) (cons obj1 obj2))
- (and (symbolp obj2) (macroexp-const-p obj1) (cons obj2 obj1))))
+ (and (symbolp obj1) (macroexp-const-p obj2) (cons obj1 (eval obj2)))
+ (and (symbolp obj2) (macroexp-const-p obj1) (cons obj2 (eval obj1)))))
(defconst byte-compile--default-val (cons nil nil) "A unique object.")
@@ -4121,12 +4121,11 @@ byte-compile-cond-jump-table-info
(unless prev-test
(setq prev-test test))
(if (and obj1 (memq test '(eq eql equal))
- (consp condition)
(eq test prev-test)
- (eq obj1 prev-var)
- ;; discard duplicate clauses
- (not (assq obj2 cases)))
- (push (list (if (consp obj2) (eval obj2) obj2) body) cases)
+ (eq obj1 prev-var))
+ ;; discard duplicate clauses
+ (unless (assoc obj2 cases test)
+ (push (list obj2 body) cases))
(if (and (macroexp-const-p condition) condition)
(progn (push (list byte-compile--default-val
(or body `(,condition)))
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 5fb64ff288..0c5f8e7250 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -27,6 +27,7 @@
(require 'ert)
(require 'cl-lib)
+(require 'subr-x)
(require 'bytecomp)
;;; Code:
@@ -296,7 +297,21 @@ byte-opt-testsuite-arith-data
((eq variable 'default)
(message "equal"))
(t
- (message "not equal")))))
+ (message "not equal"))))
+ ;; Bug#35770
+ (let ((x 'a)) (cond ((eq x 'a) 'correct)
+ ((eq x 'b) 'incorrect)
+ ((eq x 'a) 'incorrect)
+ ((eq x 'c) 'incorrect)))
+ (let ((x #x10000000000000000))
+ (cond ((eql x #x10000000000000000) 'correct)
+ ((eql x #x10000000000000001) 'incorrect)
+ ((eql x #x10000000000000000) 'incorrect)
+ ((eql x #x10000000000000002) 'incorrect)))
+ (let ((x "a")) (cond ((equal x "a") 'correct)
+ ((equal x "b") 'incorrect)
+ ((equal x "a") 'incorrect)
+ ((equal x "c") 'incorrect))))
"List of expression for test.
Each element will be executed by interpreter and with
bytecompiled code, and their results compared.")
@@ -613,6 +628,44 @@ bytecomp-tests--with-temp-file
(if (buffer-live-p byte-compile-log-buffer)
(kill-buffer byte-compile-log-buffer)))))
+(ert-deftest bytecomp-test--switch-duplicates ()
+ "Check that duplicates in switches are eliminated correctly (bug#35770)."
+ (dolist (params
+ '(((lambda (x)
+ (cond ((eq x 'a) 111)
+ ((eq x 'b) 222)
+ ((eq x 'a) 333)
+ ((eq x 'c) 444)))
+ (a b c)
+ (lambda (u v) (string< (symbol-name u) (symbol-name v))))
+ ((lambda (x)
+ (cond ((eql x #x10000000000000000) 111)
+ ((eql x #x10000000000000001) 222)
+ ((eql x #x10000000000000000) 333)
+ ((eql x #x10000000000000002) 444)))
+ (#x10000000000000000 #x10000000000000001 #x10000000000000002)
+ <)
+ ((lambda (x)
+ (cond ((equal x "a") 111)
+ ((equal x "b") 222)
+ ((equal x "a") 333)
+ ((equal x "c") 444)))
+ ("a" "b" "c")
+ string<)))
+ (let* ((lisp (nth 0 params))
+ (keys (nth 1 params))
+ (lessp (nth 2 params))
+ (bc (byte-compile lisp))
+ (lap (byte-decompile-bytecode (aref bc 1) (aref bc 2)))
+ ;; Assume the first constant is the switch table.
+ (table (cadr (assq 'byte-constant lap))))
+ (should (hash-table-p table))
+ (should (equal (sort (hash-table-keys table) lessp) keys))
+ (should (member '(byte-constant 111) lap))
+ (should (member '(byte-constant 222) lap))
+ (should-not (member '(byte-constant 333) lap))
+ (should (member '(byte-constant 444) lap)))))
+
;; Local Variables:
;; no-byte-compile: t
;; End:
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-17 14:48 ` Mattias Engdegård
@ 2019-05-22 10:52 ` Mattias Engdegård
2019-05-22 10:58 ` Eli Zaretskii
0 siblings, 1 reply; 16+ messages in thread
From: Mattias Engdegård @ 2019-05-22 10:52 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 35770, vibhavp
No hurry in reviewing the patch, but since this is a wrong-code bug, I suppose it should be back-ported to emacs-26. Does that mean that it should be written for that branch and then merged to master, or the other way around?
The test uses bignums for numbers that aren't eq. In Emacs 26, those would be floats instead, but bignums seemed more robust since it's not unreasonable to have unboxed floats in the future.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-22 10:52 ` Mattias Engdegård
@ 2019-05-22 10:58 ` Eli Zaretskii
2019-05-22 11:11 ` Mattias Engdegård
0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-05-22 10:58 UTC (permalink / raw)
To: Mattias Engdegård; +Cc: monnier, vibhavp, 35770
> From: Mattias Engdegård <mattiase@acm.org>
> Date: Wed, 22 May 2019 12:52:08 +0200
> Cc: 35770@debbugs.gnu.org, vibhavp@gmail.com
>
> No hurry in reviewing the patch, but since this is a wrong-code bug, I suppose it should be back-ported to emacs-26. Does that mean that it should be written for that branch and then merged to master, or the other way around?
The former.
However, in this case I really wonder why we should be so eager to
install the change on the emacs-26 branch. How long this wrong code
has been with us?
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-22 10:58 ` Eli Zaretskii
@ 2019-05-22 11:11 ` Mattias Engdegård
2019-05-22 11:21 ` Noam Postavsky
0 siblings, 1 reply; 16+ messages in thread
From: Mattias Engdegård @ 2019-05-22 11:11 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Stefan Monnier, vibhavp, 35770
22 maj 2019 kl. 12.58 skrev Eli Zaretskii <eliz@gnu.org>:
>
> However, in this case I really wonder why we should be so eager to
> install the change on the emacs-26 branch. How long this wrong code
> has been with us?
About two years, and it only matters for (arguably) buggy code, so you are probably right.
On the other hand, different behaviour in compiled and interpreted code does not make it easier to find such bugs.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-22 11:11 ` Mattias Engdegård
@ 2019-05-22 11:21 ` Noam Postavsky
2019-05-22 11:23 ` Eli Zaretskii
0 siblings, 1 reply; 16+ messages in thread
From: Noam Postavsky @ 2019-05-22 11:21 UTC (permalink / raw)
To: Mattias Engdegård; +Cc: Stefan Monnier, vibhavp, 35770
Mattias Engdegård <mattiase@acm.org> writes:
> 22 maj 2019 kl. 12.58 skrev Eli Zaretskii <eliz@gnu.org>:
>>
>> However, in this case I really wonder why we should be so eager to
>> install the change on the emacs-26 branch. How long this wrong code
>> has been with us?
>
> About two years, and it only matters for (arguably) buggy code, so you are probably right.
I think the significant milestone is that it's been with us since 26.1,
i.e., this is a regression since Emacs 25.
> On the other hand, different behaviour in compiled and interpreted
> code does not make it easier to find such bugs.
Another option is turning off byte-compile-cond-use-jump-table by
default.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-22 11:21 ` Noam Postavsky
@ 2019-05-22 11:23 ` Eli Zaretskii
2019-05-22 14:19 ` Stefan Monnier
0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-05-22 11:23 UTC (permalink / raw)
To: Noam Postavsky; +Cc: mattiase, monnier, vibhavp, 35770
> From: Noam Postavsky <npostavs@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>, vibhavp@gmail.com, 35770@debbugs.gnu.org
> Date: Wed, 22 May 2019 07:21:12 -0400
>
> Mattias Engdegård <mattiase@acm.org> writes:
>
> > 22 maj 2019 kl. 12.58 skrev Eli Zaretskii <eliz@gnu.org>:
> >>
> >> However, in this case I really wonder why we should be so eager to
> >> install the change on the emacs-26 branch. How long this wrong code
> >> has been with us?
> >
> > About two years, and it only matters for (arguably) buggy code, so you are probably right.
>
> I think the significant milestone is that it's been with us since 26.1,
> i.e., this is a regression since Emacs 25.
Then maybe we should again consider reverting on emacs-26 the change
which caused the regression.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-22 11:23 ` Eli Zaretskii
@ 2019-05-22 14:19 ` Stefan Monnier
2019-05-26 17:05 ` Mattias Engdegård
0 siblings, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2019-05-22 14:19 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: mattiase, Noam Postavsky, vibhavp, 35770
> Then maybe we should again consider reverting on emacs-26 the change
> which caused the regression.
turning off byte-compile-cond-use-jump-table by default is the safer way
to do that, I think.
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-22 14:19 ` Stefan Monnier
@ 2019-05-26 17:05 ` Mattias Engdegård
2019-05-26 18:43 ` Eli Zaretskii
0 siblings, 1 reply; 16+ messages in thread
From: Mattias Engdegård @ 2019-05-26 17:05 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Noam Postavsky, vibhavp, 35770
22 maj 2019 kl. 16.19 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
>
> turning off byte-compile-cond-use-jump-table by default is the safer way
> to do that, I think.
Is the consensus then that the patch is fine for master, and that the switch op generation should be disabled by default in emacs-26? Maybe with an explanation in the doc string:
-(defcustom byte-compile-cond-use-jump-table t
- "Compile `cond' clauses to a jump table implementation (using a hash-table)."
+(defcustom byte-compile-cond-use-jump-table nil
+ "Compile `cond' clauses to a jump table implementation (using a hash-table).
+
+In Emacs 26, this feature is disabled by default because of a bug
+in the code generation of `cond' forms with duplicated test clauses."
:version "26.1"
:group 'bytecomp
:type 'boolean)
If so, I'll push. No NEWS change needed in either branch, right?
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-26 17:05 ` Mattias Engdegård
@ 2019-05-26 18:43 ` Eli Zaretskii
2019-05-26 22:06 ` Stefan Monnier
0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-05-26 18:43 UTC (permalink / raw)
To: Mattias Engdegård; +Cc: npostavs, 35770, vibhavp, monnier
> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sun, 26 May 2019 19:05:55 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, Noam Postavsky <npostavs@gmail.com>,
> vibhavp@gmail.com, 35770@debbugs.gnu.org
>
> 22 maj 2019 kl. 16.19 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
> >
> > turning off byte-compile-cond-use-jump-table by default is the safer way
> > to do that, I think.
>
> Is the consensus then that the patch is fine for master, and that the switch op generation should be disabled by default in emacs-26? Maybe with an explanation in the doc string:
>
> -(defcustom byte-compile-cond-use-jump-table t
> - "Compile `cond' clauses to a jump table implementation (using a hash-table)."
> +(defcustom byte-compile-cond-use-jump-table nil
> + "Compile `cond' clauses to a jump table implementation (using a hash-table).
> +
> +In Emacs 26, this feature is disabled by default because of a bug
> +in the code generation of `cond' forms with duplicated test clauses."
> :version "26.1"
> :group 'bytecomp
> :type 'boolean)
I don't think we should explain why we did this in a doc string. And
:version should change as well.
More generally, I can't say I like this, and don't understand why this
would be better than reverting the offending change on emacs-26. Why
is it a problem to defer the features which caused the regression to
Emacs 27?
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-26 18:43 ` Eli Zaretskii
@ 2019-05-26 22:06 ` Stefan Monnier
2019-05-27 11:27 ` Mattias Engdegård
0 siblings, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2019-05-26 22:06 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Mattias Engdegård, npostavs, vibhavp, 35770
> More generally, I can't say I like this, and don't understand why this
> would be better than reverting the offending change on emacs-26. Why
> is it a problem to defer the features which caused the regression to
> Emacs 27?
I personally don't have a preference. I find reverting the change to be
a more intrusive change with more risks of errors, but it's likely
a wash either way.
FWIW I also think it's generally good to introduce new byte-codes in one
version and only enable the compiler to use them in the next version
(that's what I did for the condition-case byte codes, for example).
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-26 22:06 ` Stefan Monnier
@ 2019-05-27 11:27 ` Mattias Engdegård
2019-06-01 13:58 ` Noam Postavsky
0 siblings, 1 reply; 16+ messages in thread
From: Mattias Engdegård @ 2019-05-27 11:27 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Noam Postavsky, vibhavp, 35770
27 maj 2019 kl. 00.06 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
>
> I personally don't have a preference. I find reverting the change to be
> a more intrusive change with more risks of errors, but it's likely
> a wash either way.
>
> FWIW I also think it's generally good to introduce new byte-codes in one
> version and only enable the compiler to use them in the next version
> (that's what I did for the condition-case byte codes, for example).
Trying to revert 'the rest' (the switch code generation) while keeping the switch op itself seems indeed to be risky and a lot of work compared to just turning off the generation. It is not a matter of finding a few commits to revert; there are fixes upon fixes, and changes that need to be untangled. It does not sound very practical.
Since nobody complained about the actual bug fix, I'm boldly pushing it to master.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-05-27 11:27 ` Mattias Engdegård
@ 2019-06-01 13:58 ` Noam Postavsky
2019-06-01 15:47 ` Eli Zaretskii
0 siblings, 1 reply; 16+ messages in thread
From: Noam Postavsky @ 2019-06-01 13:58 UTC (permalink / raw)
To: Mattias Engdegård; +Cc: Stefan Monnier, vibhavp, 35770
[-- Attachment #1: Type: text/plain, Size: 442 bytes --]
Mattias Engdegård <mattiase@acm.org> writes:
> Trying to revert 'the rest' (the switch code generation) while keeping
> the switch op itself seems indeed to be risky and a lot of work
> compared to just turning off the generation. It is not a matter of
> finding a few commits to revert; there are fixes upon fixes, and
> changes that need to be untangled. It does not sound very practical.
So, is this patch okay for emacs-26?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1034 bytes --]
From 257a750a10ebb77277a5d7d771c01d66a69a66e4 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 1 Jun 2019 09:53:35 -0400
Subject: [PATCH] Disable byte-compile-cond-use-jump-table (Bug#35770)
* lisp/emacs-lisp/bytecomp.el (byte-compile-cond-use-jump-table): Set
to nil by default.
Don't merge to master, the bug is already fixed there.
---
lisp/emacs-lisp/bytecomp.el | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index e3b34c189f..9273626c80 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -234,9 +234,9 @@ (defcustom byte-compile-delete-errors nil
:group 'bytecomp
:type 'boolean)
-(defcustom byte-compile-cond-use-jump-table t
+(defcustom byte-compile-cond-use-jump-table nil
"Compile `cond' clauses to a jump table implementation (using a hash-table)."
- :version "26.1"
+ :version "26.3" ;; Disabled due to Bug#35770.
:group 'bytecomp
:type 'boolean)
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-06-01 13:58 ` Noam Postavsky
@ 2019-06-01 15:47 ` Eli Zaretskii
2019-06-01 21:53 ` Noam Postavsky
0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-06-01 15:47 UTC (permalink / raw)
To: Noam Postavsky; +Cc: mattiase, monnier, vibhavp, 35770
> From: Noam Postavsky <npostavs@gmail.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Eli Zaretskii <eliz@gnu.org>, vibhavp@gmail.com, 35770@debbugs.gnu.org
> Date: Sat, 01 Jun 2019 09:58:42 -0400
>
> So, is this patch okay for emacs-26?
Yes, thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation
2019-06-01 15:47 ` Eli Zaretskii
@ 2019-06-01 21:53 ` Noam Postavsky
0 siblings, 0 replies; 16+ messages in thread
From: Noam Postavsky @ 2019-06-01 21:53 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: mattiase, monnier, vibhavp, 35770
tags 35770 fixed
close 35770 26.3
quit
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Noam Postavsky <npostavs@gmail.com>
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Eli Zaretskii <eliz@gnu.org>, vibhavp@gmail.com, 35770@debbugs.gnu.org
>> Date: Sat, 01 Jun 2019 09:58:42 -0400
>>
>> So, is this patch okay for emacs-26?
>
> Yes, thanks.
Pushed.
04f13a5d9b 2019-06-01T17:48:43-04:00 "Disable byte-compile-cond-use-jump-table (Bug#35770)"
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=04f13a5d9bc19cfe0382e4257f1a1d856aa354ed
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-06-01 21:53 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-17 9:33 bug#35770: [PATCH] Broken duplicate case elimination in switch byte-compilation Mattias Engdegård
2019-05-17 12:16 ` Stefan Monnier
2019-05-17 14:48 ` Mattias Engdegård
2019-05-22 10:52 ` Mattias Engdegård
2019-05-22 10:58 ` Eli Zaretskii
2019-05-22 11:11 ` Mattias Engdegård
2019-05-22 11:21 ` Noam Postavsky
2019-05-22 11:23 ` Eli Zaretskii
2019-05-22 14:19 ` Stefan Monnier
2019-05-26 17:05 ` Mattias Engdegård
2019-05-26 18:43 ` Eli Zaretskii
2019-05-26 22:06 ` Stefan Monnier
2019-05-27 11:27 ` Mattias Engdegård
2019-06-01 13:58 ` Noam Postavsky
2019-06-01 15:47 ` Eli Zaretskii
2019-06-01 21:53 ` Noam Postavsky
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).