Subject: 28.0.50; pcase not binding variables conditionally

From: Pip Cet <[email-masked]>
Date: Sat, 29 Aug 2020 09:41:10 +0000

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.
From ec7e4a92ab08adf4eb036ea50f7d70bade63719b Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
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