Re: pcase generates an unprintable expansion for a form in test erc--restore-initialize-priors

[Stefan Monnier]

>   (should (pcase (macroexpand-1 '(erc--restore-initialize-priors erc-my-mode
>                                    foo (ignore 1 2 3)
>                                    bar #'spam
>                                    baz nil))
>             (`(let* ((,p (or erc--server-reconnecting erc--target-priors))
>                      (,q (and ,p (alist-get 'erc-my-mode ,p))))
>                 (unless (local-variable-if-set-p 'erc-my-mode)
>                   (error "Not a local minor mode var: %s" 'erc-my-mode))
>                 (setq foo (if ,q (alist-get 'foo ,p) (ignore 1 2 3))
>                       bar (if ,q (alist-get 'bar ,p) #'spam)
>                       baz (if ,q (alist-get 'baz ,p) nil)))
>              t))))

Hmm...
Looking at it from a `pcase.el` perspective, the above pattern represents
a really large number of conditions :-)

> ..  Note that the pcase form is expanding a backquoted form lacking in
> pcase features such as nested backquotes, pcase variable names, and the
> like.

[ I didn't understand what you meant here, sorry.  ]

> The pcase expansion thus consists of a deeply nested alternation of forms
> like 
>     (if (consp x1961) ...)
> and
>     (let* ((x1962 (car-safe x1961))) ...)
> ..

Oh, yes!

> The level of nesting is greater than, or close enough to 200 that
> printing it can lead to the detection of an apparent circular structure
> in print_object in src/print.c.  There PRINT_CIRCLE is #defined as 200.

I'm surprised it doesn't also cause errors during compilation (because
of too deeply nested recursive calls).

> The form being compared using pcase, although not tiny, is not all that
> big, and it would be easy to increase its size to cause it to violate any
> reasonable value of PRINT_CIRCLE.

Agreed.

> Would it be possible and a good idea to amend pcase such that it
> generates less deeply nested expansions for forms such as we have here?

A good idea?  Definitely.
Possible?  No doubt.
Not sure how easy it would be, OTOH.
FWIW, this is a case where the compilation strategy Richard uses in
`cond*` probably works better.

I think the main thing to do here would be to see how to recognize
subpatterns that don't have any `,` in them so we can match them with
a simple `equal` test.  E.g. the whole

                (unless (local-variable-if-set-p 'erc-my-mode)
                  (error "Not a local minor mode var: %s" 'erc-my-mode))

subpattern can be matched much more efficiently with a single `equal`,

I suspect it can be done with no change to the core pcase code by
rewriting

    (pcase-defmacro \` (qpat)

so it does the recursion on its own (e.g. instead of expanding `(A . B)
to a pattern which contains `A and `B).  Then at any level if it notices
that no `,` was found during the recursion it can just use `equal`.

Another approach would be to change the "back end" of pcase so that when
we built the nested `let*/if` monster we recognize specific patterns
that can be rewritten more compactly.

> Or does anybody have any ideas how better to resolve the problem?

FWIW, I'm wondering what's the value of this specific test.
Do we really care about the specific shape of the returned code?
It seems terribly brittle: every minute change to the macro will require
matching changes to the test, and I have no idea what is the intention
of this test so if it every fails I wouldn't know whether the error is
in the test or in the code.


        Stefan