bug#72279: [PATCH] Non-local exits from outside the lexical scope are caught by cl-block

[Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

>   (defun foo () (cl-return "ruh roh"))
>   (cl-block nil (foo) (cl-return t)) ; => "ruh ruh"
>
> The first patch attached attempts to solve this by moving the
> functionality of the wrapper compiler macros to the macros themselves
> and by using uninterned symbols for the thrown and caught tags,
> communicated by the block to the corresponding returns.  All the
> existing tests seemed to run just fine but I did not do any
> comprehensive testing (and there doesn't appear to be any relevant
> suites either).

BTW, for the second patch a simpler solution is to expand

    (cl-block A FOO)
to
    (let ((cl--block--A ',(make-symbol "A")))
      (catch cl--block--A FOO))

and

    (cl-return B FOO)
to
    (throw cl--block--B FOO)

which will signal an "unknown variable cl--block--B" if a `cl-return` is
used outside of its block.

But the optimization of `cl-block` when the tag is not used is
important, so I don't think it's a good approach.

> I do take minor issue with `macroexpand-all'ing all things inside a
> block, making debugging via macrostep really annoying, but I don't know
> of a better solution, outside of communicating the tag during
> evaluation, which would look something like the second patch.

I think in theory it's possible to avoid the `macroexpand-all`, but it
requires a compiler-macro which (assuming we generate code like
I outlined above) will have to recognize those `(catch <VAR> ...)` where
the tag is not used anywhere else.

This is because compiler macros are executed by `macroexpand-all` both
before and after performing the normal macro expansions, so they do get
to see the code after `macroexpand-all`.  But it can be slightly tricky
to get it right and reliable (because it has to be careful to not mess
up the code if it hasn't been macroexpanded yet), and it has to walk the
whole code, which means that it needs to copy a fair bit of the code of
`macroexpand-all`.

> If it were to instead expand into a `catch' - whether because there is
> a `cl-return' or because `cl-block' is modified to always expandi into
> a `catch' as it is in my second patch - the setf will expand into
> (setf catch) which is not defined.

Yup, as I said, the "optimization" is important (most of the
`cl-block`s are implicit and unused, IME).

> I see two possible solutions, either define a (setf catch) or switch
> to defsubst instead of cl-defsubst.

I don't think we can implement a `setf catch` (tho we could probably
come up with a hack which works in practice for those
cl-defstruct slots).  🙁

IIRC for the slot accessors `defsubst` would lead to worse code than
what we currently get with `cl-defsubst` so it wouldn't be as good, but
I believe we could use `define-inline` instead.

AFAIC, your first patch looks good to me.


        Stefan