Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Maybe it might be worthwhile splitting it into 2 or 3 patches so as to
> better see how we got to the level of complexity.
> See more comments below.

Local setfs certainly can be added later (or simply dropped, until
better times).  It will simplify the patch.

> Why do we have/need this?  Does it work with other things that use
> gv-places, like `push`, `pop`, `cl-callf`, ...?  If so, how?
> If not, then we need another approach which does.

I added support for local setfs mostly

- because I decided to implement all features of flet since I'm dealing
  with low levels of cl-flet anyway and this might be the last time it
  happens;

- and since ignoring local setfs now might make it harder to implement
  them in future.

This functionality does not have any more to do with places than

(cl-defmethod (setf ..) ..)

so I guess it has no relation to gv-places, right?

>
> I thought handling `cl-flet` of (setf foo) would amount to calling
> `gv-setter` to get the symbol corresponding to `(setf foo)` and then
> c-flet-binding that symbol instead of `(setf foo).
>

At the very least, gv-setter will fail for local (setf car) and so on.
I don't know how cl-flet should treat such things (in Common Lisp it's
UB) but I saw that cl-flet performs local redefinition of car just fine
so I followed suit.

>> +(defun cl--expand-flet (env body &rest flet-expanders-plist)
>> +  "Return a form equivalent to `(cl-flet ,bindings BODY)
>> +where bindings correspond to FLET-EXPANDERS-PLIST as described below.
>> +
>> +ENV should be macroexpansion environment
>> +to be augmented with some definitions from FLET-EXPANDERS-PLIST
>> +to then expand forms in BODY with.
>> +
>> +FLET-EXPANDERS-PLIST should be a plist
>> +where keys are function names
>> +and values are 0-argument lambdas
>> +to be called if the corresponding function name is encountered
>> +in BODY and then only (that is, at most once).
>
> Why "at most once"?

We don't want to end up calling different functions in different
instances of local function calls which share the same name within a
single cl-flet form, for whatever unforseeable reason this might happen
(like poorly written flet-expander or poorly written exp in (func exp)).
The best way to ensure local definitions are consistent is to only
produce the local function once per local function used in the body.

If the local function is not encountered in the body at all, the
corresponding function object (or whatever exp in (func exp) evaluated
to) is never produced.  Hence, “at most once”.

Note also that in the use case this patch was motivated by, we only need
to evaluate arbitrary code once per encountered symbol:

(cl--expand-flet macroenv (cdr parsed-body)
  'cl-call-next-method (lambda () (push cnm uses-cnm)
                         (list nil cnm))
  'cl-next-method-p (lambda () (push nmp uses-cnm)
                      (list nil nmp)))

> Can we simplify this so only one of the two is supported?

Maybe but before I reply properly, I'd like to know if you have any
thoughts on
https://lists.gnu.org/archive/html/emacs-devel/2021-10/msg00522.html
(it is relevant).