Re: Pattern matching on match-string groups #elisp #question

[Stefan Monnier]

> Thank you, I think this is good enough -- I've pushed the fix (with
> tests, so it matters less whether I've understood it) to master. (If
> pcase one day gets uppity enough to optimise based on the target
> expression as well, then a lot of tests will become meaningless.)

BTW, I was thinking about making the optimization more conservative, so
it only throws away the actual `if` but keeps the computation of the test:

    diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
    index c7288b7fa2..0036c1882d 100644
    --- a/lisp/emacs-lisp/pcase.el
    +++ b/lisp/emacs-lisp/pcase.el
    @@ -469,8 +469,8 @@ pcase--small-branch-p
     ;; the depth of the generated tree.
     (defun pcase--if (test then else)
       (cond
    -   ((eq else :pcase--dontcare) then)
    -   ((eq then :pcase--dontcare) (debug) else) ;Can/should this ever happen?
    +   ((eq else :pcase--dontcare) `(progn ,test ,then))
    +   ((eq then :pcase--dontcare) `(progn ,test ,else))
        (t (macroexp-if test then else))))
     
     ;; Note about MATCH:

and it does fix the `pcase-let` problem with your original code.
The problem, OTOH is that we now end up with warnings like

    In toplevel form:
    progmodes/elisp-mode.el:896:38: Warning: value returned from
        (memq (type-of l) cl-struct-xref-elisp-location-tags) is unused

and these are rather hard/inconvenient to fix (short of not using
`pcase-let`).  [ Of course, another problem is that we generate
marginally less efficient code in some cases, but that should be very
minor and that's more a failure of the byte-compiler than a problem in
`pcase`.  ]

This said, there is a fundamental problem with both the previous and the
new code:

        (pcase STR
          ((and (rx (group-n 1 (* "a"))) (guard (not (looking-at "^")))) nil)
          ((rx (let FOO (* "a"))) FOO))

It should macroexpand to something morally equivalent to:

    (cond ((not (stringp STR)) nil)
          ((not (string-match "\\(?1:a*\\)" STR)) nil)
          ((looking-at "^"")
           (let* ((x1464 (match-string 1 STR)))
             (let ((FOO x1464)) FOO))))

at which point you'll presumably notice that the match data is likely
garbled when we try to use it.
[ Cue old message about how side effects are bad for your health.  ]

> A clearer but less efficient pattern would be something like
>
> (app (lambda (s) (and (string-match REGEXP s)
>                       (list (match-string 1 s)
>                             (match-string 2 s)
>                             ...)))
>      `(,VAR1 ,VAR2 ...))

That's indeed what I did in my code (tho using a vector instead of
a list), probably because the above risk did occur to me back then.

> Of course a sufficiently optimising compiler would eliminate the consing!

Indeed, and it's not a difficult optimization (at least if you can
presume that this data is immutable).

>> It's linked to the special undocumented pcase pattern `pcase--dontcare`
>> (whose name is not well chosen, suggestions for better names are
>> welcome)
>
> pcase--give-up

Hmm... probably not much more explanatory than "dontcare".

> pcase--!,fail

That begs the question of what does it mean for the overall pcase to "fail".

I was thinking of `pcase--impossible` as well.


        Stefan