all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#43100: 28.0.50; pcase not binding variables conditionally
@ 2020-08-29  9:41 Pip Cet
  2020-08-29 12:01 ` Philipp Stephani
  2021-03-02 13:38 ` Pip Cet
  0 siblings, 2 replies; 13+ messages in thread
From: Pip Cet @ 2020-08-29  9:41 UTC (permalink / raw)
  To: 43100, monnier

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

I'm having trouble with pcase's behavior.

(pcase "a"
  ((or (pred symbolp) name)
   (let ((foo 'bar)) name)))

throws an error. It shouldn't. (Note that the dummy "let" is necessary
to force the pcase code generation to use a function call).

I believe the culprit is the code around this comment in pcase.el

;; If some of `vars' were not found in `prevvars', that's
;; OK it just means those vars aren't present in all
;; branches, so they can be used within the pattern
;; (e.g. by a `guard/let/pred') but not in the branch.

I believe that's incorrect: using the variable in a condition-case
should work, as should conditional shadowing of an existing binding,
as in this case:

(let ((name "default"))
  (pcase "a"
    ((or (pred symbolp) name)
     name)))

(which works), or this case:

(let ((name "default"))
  (pcase "a"
    ((or (pred symbolp) name)
     (let ((foo 'bar)) name))))

(which doesn't).

I believe the right fix is not to share code for the same branch if it
uses different variables, as in the attached patch. It's possible this
increases codegen complexity in some construed cases, but in practice
that shouldn't be a problem.

[-- Attachment #2: 0001-Allow-variable-bindings-to-differ-across-pcase-alter.patch --]
[-- Type: text/x-patch, Size: 1135 bytes --]

From ec7e4a92ab08adf4eb036ea50f7d70bade63719b Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Sat, 29 Aug 2020 09:35:41 +0000
Subject: [PATCH] Allow variable bindings to differ across pcase alternatives.

This fixes pcase cases like ((or (pred stringp) name) name), which
should use the pcase-local binding only if EXPVAL isn't a string.

* lisp/emacs-lisp/pcase.el (pcase--expand): Do not share code if
variables differ.
---
 lisp/emacs-lisp/pcase.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index a8ce23284c..72916296cb 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -346,7 +346,8 @@ pcase--expand
             (lambda (code vars)
               (let ((vars (pcase--fgrep vars code))
                     (prev (assq code seen)))
-                (if (not prev)
+                (if (or (not prev)
+                        (not (equal (cadr prev) vars)))
                     (let ((res (pcase-codegen code vars)))
                       (push (list code vars res) seen)
                       res)
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-03-02 13:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-29  9:41 bug#43100: 28.0.50; pcase not binding variables conditionally Pip Cet
2020-08-29 12:01 ` Philipp Stephani
2020-08-29 14:27   ` Pip Cet
2020-08-29 16:06     ` Stefan Monnier
2020-08-30 16:21       ` Pip Cet
2020-08-30 18:07         ` Stefan Monnier
2020-08-31 19:32           ` Pip Cet
2020-09-01  3:12             ` Stefan Monnier
2020-09-02  8:38               ` Pip Cet
2020-09-02 14:16                 ` Stefan Monnier
2020-09-05 14:36                   ` Pip Cet
2020-09-05 16:52                     ` Stefan Monnier
2021-03-02 13:38 ` Pip Cet

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.