bug#43100: 28.0.50; pcase not binding variables conditionally

[Pip Cet <pipcet@gmail.com>]

On Sat, Aug 29, 2020 at 4:06 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> >> > I'm having trouble with pcase's behavior.
> >> >
> >> > (pcase "a"
> >> >   ((or (pred symbolp) name)
> >> >    (let ((foo 'bar)) name)))
> >> >
> >> > throws an error. It shouldn't.
> >>
> >> Isn't this case documented in the manual? The last section of
> >> https://www.gnu.org/software/emacs/manual/html_node/elisp/pcase-Macro.html
> >> states:
> >> "It makes no sense for each sub-pattern [in an `or' sequence] to
> >> let-bind a different set of symbols because the body forms have no way
> >> to distinguish which sub-pattern matched and choose among the
> >> different sets."
> >
> > Thanks for pointing this out.
> >
> > I disagree with what the documentation says there: it does make
> > perfect sense, to me, to conditionally shadow a (lexical) let binding
> > with a pcase-let one, and body forms will have no trouble in practice
> > distinguishing between the outer and the inner let-binding.
>
> How do you expect them to distinguish?

By repeating the pcase pattern's predicate, usually, or by relying on
other knowledge about the previous value. That's in practice, in
theory you can use gensym or an inaccessible cons.

> IIUC you want
>
>     (pcase V
>       ((or (pred symbolp) name)
>        (let ((foo 'bar)) name)))
>
> to behave like
>
>     (cond
>      ((symbolp V) (let ((foo 'bar)) name))
>      (t (let ((name V)) (let ((foo 'bar)) name))))
>
> ?

Yes, that's correct. It's also how (pcase V ((or (pred symbolp) name)
name) behaves...

> I'd rather not go there

You'd rather have the behavior of (pcase V ((or pred symbolp) name)
EXPR) depend on the complexity of EXPR?

> since it means that the single occurrence of the
> `name` identifier in the branch's body refers to two different variable
> bindings depending on which pattern was matched.  It smells of dynamic
> scoping, tho it is admittedly compatible with lexical-scoping but only at
> the cost of having to duplicate the branch's body.

Well, I'm afraid pcase does smell of dynamic scoping :-)

More precisely, to me, what pcase simulates is building a lexical
environment, then evaluating the BODY argument with the environment as
second argument. (pcase EXP (PAT BODY)) is very roughly:

(for-each-possible-environment ENV (if (equalish (eval PAT ENV) EXP)
(return (eval BODY ENV)))

where for-each-possible-environment magically finds the smallest (or
"best") ENV first.

I think it would be nice to have a lexical three-argument version of
pcase which specifies which variables are output values, treating the
remaining ones as input values, to make it easier to build
non-constant patterns. But that would be very different from what
pcase does do today, which is closer to what I hinted at above.

IOW, if you want to call a function with arguments determined by
pcase-like patterns, why not introduce pcase-call so something like
the following would work:

(defun f (hello world) (cons hello world))

(let ((space " ") (hw "hello world"))
  (pcase-call 'f ((concat hello space world) hw)))

(that would introduce named function arguments, which I think would be
a nice bonus)?

But it's not what pcase is! There's no function call or argument list
in there, just expressions involved in environments.

As for duplicating the body, that is an implementation detail. You can
easily avoid it by producing

(let ((name name))
  (cond ((symbolp V) X)
    (progn (setq name V) X)))

disallowing the modification of name in X.

> The "intended" behavior instead would be to behave like
>
>     (cond
>      ((symbolp V) (let ((name nil)) (let ((foo 'bar)) name)))
>      (t (let ((name V)) (let ((foo 'bar)) name))))
>
> That's already the behavior you get if you switch the two:
>
>     (macroexpand '(pcase V
>                     ((or (and (pred foo) name) (pred symbolp))
>                      (let ((foo 'bar)) name))))
>     =>
>     (let* ((pcase-0 (lambda (name) (let ((foo 'bar)) name))))
>       (cond ((foo V) (funcall pcase-0 V))
>             ((symbolp V) (funcall pcase-0 nil))
>             (t nil)))

I don't see where the nil comes from, or why it's a useful choice for
a default value.

> the fact that the behavior depends on the order of elements in `or` is
> an undesirable side effect of the implementation technique.

It also depends on the complexity of the branch.

It seems to me there are at least three consistent ways of behaving
(throw an error, bind name to nil, bind name to name), with an
inconsistent fourth way being what's currently implemented.

> > Things like the behavior of
> >
> > (pcase-let ((`(,a ,@b) (list 3 4)))
> >   a)
> >
> > just seem puzzling to me. There are good reasons not to implement
> > sublist matching (though I don't think those reasons are sufficient
> > not to have a simple implementation anyway),
>
> I don't know of a simple implementation.

Here's my better-than-nothing attempt. I don't think that's complex;
if anything, it's too trivial.

(pcase-defmacro append (&rest patterns)
  (if patterns
      (let* ((r1 (gensym))
         (r2 (gensym))
         (pat (list '\` (cons (list '\, (list 'and r1 (car patterns)))
                  (list '\, (list 'and r2 (cons 'append
                          (cdr patterns)))))))
         (f (eval `(lambda (l)
             (catch 'pcase--append
               (dotimes (i (1+ (length l)))
                 (let ((l1 (seq-subseq l 0 i))
                   (l2 (seq-subseq l i)))
                   (pcase (cons l1 l2)
                 (,pat (throw 'pcase--append
                          (cons ,r1 ,r2))))))))
              t)))
    `(app ,f ,pat))
    ''nil))



>
> > so an error message would be acceptable,
>
> You're probably right.
>
> > Sorry for complaining. Here's a patch.
>
> LGTM,
>
>
>         Stefan
>