unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Robert Cochran <robert-emacs@cochranmail.com>
To: Ikumi Keita <ikumi@ikumi.que.jp>
Cc: Andreas Schwab <schwab@suse.de>,
	31718@debbugs.gnu.org, Vibhav Pant <vibhavp@gmail.com>
Subject: bug#31718: 26.1; Strange behavior of `cond'
Date: Wed, 06 Jun 2018 00:41:50 -0700	[thread overview]
Message-ID: <87wovc8ipt.fsf@cochranmail.com> (raw)
In-Reply-To: <46058.1528263715@localhost> (Ikumi Keita's message of "Wed, 06 Jun 2018 14:41:55 +0900")

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

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

> Hi Andreas, thanks for your reply.
>
>>>>>> Andreas Schwab <schwab@suse.de> writes:
>> On Jun 05 2018, Ikumi Keita <ikumi@ikumi.que.jp> wrote:
>>> (defun xyz (arg)
>>> "dummy"
>>> ;    (cond ((eq arg nil) ; OK
>>> ;    (cond ((eq arg 'abc) ; OK
>>> ;    (cond ((eq arg 'def) ; OK
>>> (cond ((eq arg 'default) ; NG
>
>> The byte-compiler uses 'default as a magic symbol, which breaks this
>> case.
>
> Does this mean that this behavior is a (new) designed feature of elisp
> and not a bug?
> If so, is it the respoisibility of the authors of the codes to rewrite
> not to use `default' or else to make sure to set
> `byte-compile-cond-use-jump-table' to nil at byte compile?

I for one consider this a bug, for 2 reasons:

1) It's not reasonable to expect a Lisp programmer to just know that
using the symbol default is problematic.

2) It creates diverging behavior between compiled and non-compiled Lisp.

To that end, I've made a small patch to rectify the behavior. Instead of
hardcoding a symbol, it uses gensym to create a unique one. I did a full
build of Emacs, as well as ran 'make check' and had identical results
pre- and post-change, so I'm reasonably sure it's correct.

Comments and corrections are of course welcomed.

HTH,
-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2

-----


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Use a gensym for byte compiler cond switch --]
[-- Type: text/x-patch, Size: 3118 bytes --]

From 4a025170b2b293810cf03c964b402963495fe7d7 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
Date: Wed, 6 Jun 2018 00:31:25 -0700
Subject: [PATCH] Use a gensym for the default case in
 byte-compile-cond-jump-table

* lisp/bytecomp.el (byte-compile-cond-jump-table): Create gensym to
  use as default case symbol
  (byte-compile-cond-jump-table-info): new argument `default-sym'; use
  it when generating default case clause
---
 lisp/emacs-lisp/bytecomp.el | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index ad6b5b7ce2..0fedfd0868 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -4092,7 +4092,7 @@ byte-compile-cond-vars
    (and (symbolp obj1) (macroexp-const-p obj2) (cons obj1 obj2))
    (and (symbolp obj2) (macroexp-const-p obj1) (cons obj2 obj1))))
 
-(defun byte-compile-cond-jump-table-info (clauses)
+(defun byte-compile-cond-jump-table-info (clauses default-sym)
   "If CLAUSES is a `cond' form where:
 The condition for each clause is of the form (TEST VAR VALUE).
 VAR is a variable.
@@ -4124,14 +4124,15 @@ byte-compile-cond-jump-table-info
                         (not (assq obj2 cases)))
                    (push (list (if (consp obj2) (eval obj2) obj2) body) cases)
                  (if (and (macroexp-const-p condition) condition)
-                     (progn (push (list 'default (or body `(,condition))) cases)
+                     (progn (push (list default-sym (or body `(,condition))) cases)
                             (throw 'break t))
                    (setq ok nil)
                    (throw 'break nil))))))
          (list (cons prev-test prev-var) (nreverse cases)))))
 
 (defun byte-compile-cond-jump-table (clauses)
-  (let* ((table-info (byte-compile-cond-jump-table-info clauses))
+  (let* ((default-sym (gensym "byte-compile--cond-default-sym"))
+         (table-info (byte-compile-cond-jump-table-info clauses default-sym))
          (test (caar table-info))
          (var (cdar table-info))
          (cases (cadr table-info))
@@ -4141,7 +4142,7 @@ byte-compile-cond-jump-table
       ;; set it to `t' for cond forms with a small number of cases.
       (setq jump-table (make-hash-table :test test
                                         :purecopy t
-                                        :size (if (assq 'default cases)
+                                        :size (if (assq default-sym cases)
                                                   (1- (length cases))
                                                 (length cases)))
             default-tag (byte-compile-make-tag)
@@ -4175,8 +4176,8 @@ byte-compile-cond-jump-table
       (let ((byte-compile-depth byte-compile-depth))
         (byte-compile-goto 'byte-goto default-tag))
 
-      (when (assq 'default cases)
-        (setq default-case (cadr (assq 'default cases))
+      (when (assq default-sym cases)
+        (setq default-case (cadr (assq default-sym cases))
               cases (butlast cases 1)))
 
       (dolist (case cases)
-- 
2.17.1


  reply	other threads:[~2018-06-06  7:41 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-05  6:26 bug#31718: 26.1; Strange behavior of `cond' Ikumi Keita
2018-06-05  8:07 ` Andreas Schwab
2018-06-06  5:41   ` Ikumi Keita
2018-06-06  7:41     ` Robert Cochran [this message]
2018-06-06  9:14       ` Ikumi Keita
2018-06-12  1:34         ` Robert Cochran
2018-06-12 22:22           ` Noam Postavsky
2018-06-13  5:51             ` Ikumi Keita
2018-06-13  6:26               ` Vibhav Pant
2018-06-16 15:10 ` Paul Eggert
2018-06-16 15:20   ` Eli Zaretskii
2018-06-16 15:28     ` Paul Eggert
2018-06-16 16:02       ` Eli Zaretskii
2018-06-16 16:45         ` Paul Eggert
2018-06-16 15:23   ` Noam Postavsky
2018-06-16 15:30     ` Paul Eggert
2018-06-16 23:00       ` Drew Adams
2018-06-17  4:44   ` Michael Heerdegen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wovc8ipt.fsf@cochranmail.com \
    --to=robert-emacs@cochranmail.com \
    --cc=31718@debbugs.gnu.org \
    --cc=ikumi@ikumi.que.jp \
    --cc=schwab@suse.de \
    --cc=vibhavp@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).