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

[Thuna <thuna.cing@gmail.com>]

> >   (defun foo () (cl-return "ruh roh"))
> >   (cl-block nil (foo) (cl-return t)) ; => "ruh ruh"
> 
> 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).

Yes, with my first patch applied, the call to `foo' signals a `no-catch'
in (cl-return "ruh roh").  It also emits a warning while macroexpanding.

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

No, sorry, I didn't.  I am currently working on a different thing which
changes the definitions for these macros yet again so I proposed this
more as an interliminary change so that my later changes don't end up
moving where cl-block is calculated AND modify what it does all in one
step.

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

Correct, I have not changed that behavior; if no appropriate
`cl-return-from's are found within the body of the `cl-block', then the
`cl-block' simply expands into the (macroexpanded, though that can be
fixed) body.

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

This was a variable which already existed; I simply moved it here,
though I wouldn't mind documenting it.  A decent place to start from is:
"A list of the currently active `cl-block' forms.

Each element is of the form (NAME VAR FOUNDP) where NAME is the name of
the block as it is written in `cl-block' and `cl-return-from', VAR is
the tag as it is caught and thrown by `catch' and `throw', and FOUNDP is
non-nil if a `cl-return' or `cl-return-from' with the same name was
found.

This variable should be set such that the modifications can only be seen
within the same lexical scope."