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

[Andrea Corallo <acorallo@gnu.org>]

Thuna <thuna.cing@gmail.com> writes:

> Since the thrown and caught tags in `cl-block' and `cl-return-from' are
> interned and deterministic, a `cl-block' catches `cl-return-from's with
> the corresponding names even from outside the current lexical scope.
>
> The only mechanism in place to stop this is the compiler macro around
> cl--block-catch, which removes the block if no approriate returns are
> found, however not only is this bound to break if the compiler macro
> fails to expand, a valid exit is all that is needed to work around this.
>
>   (defun foo () (cl-return "ruh roh"))
>   (cl-block nil (foo) (cl-return t)) ; => "ruh ruh"

Hi Thuna,

yes I agree this is not optimal.  We should not even get to evaluating
the second form as we should error on the first (I think your patch
does that).

> 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).

Have you tried some general use with Emacs compiled with this patch?

> 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.


IIUC installing first.patch we keep the same capability of cleaning-up
all un-necessary catches if no approriate throws are found correct?

> PS. I would also like to have a discussion about a problem that I have
> noticed when trying to build with the second patch, maybe here maybe in
> another bug: Because struct slots are defined using `cl-defsubst', the
> whole body is wrapped in a `cl-block'.  The only reason `setf' works
> with such slots is because `cl-block' expands into the body itself when
> there are no `cl-return's.  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.  I see two
> possible solutions, either define a (setf catch) or switch to defsubst
> instead of cl-defsubst.
>
>>From 4027c50645260a202e45a2a074dfeb48468394c1 Mon Sep 17 00:00:00 2001
> From: Thuna <thuna.cing@gmail.com>
> Date: Wed, 24 Jul 2024 18:41:25 +0200
> Subject: [PATCH 1/2] Use uninterned tags in cl-block, remove block wrappers
>

[...]

> diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
> index 2e501005bf7..31b88aec889 100644
> --- a/lisp/emacs-lisp/cl-macs.el
> +++ b/lisp/emacs-lisp/cl-macs.el
> @@ -889,6 +889,8 @@ cl-etypecase
>  
>  ;;; Blocks and exits.
>  
> +(defvar cl--active-block-names nil)

I think would be nice to document this variable with how its content is
supposed to look like.

I'm Ccing Stefan maybe he has opinions on these patches.

Also I don't see you in the copyright file.  Would you like to do the
FSF paperwork in order to be able to contribute non trivial patches to
Emacs?

Thanks!

  Andrea