Lexical Binding, DEFUN inside LET - bound value changes without being set?

[Rainer Joswig]

SBCL 1.3.11 on a Mac.

;-------------------------

(defun f ()
  (print :f0))

(let ((old-f #'f))
  (funcall old-f)
  (defun f () (print :f1))
  (funcall old-f))

;-------------------------

Loading/executing the above code prints

:F0
:F1

I would expect:

:F0
:F0

The funcall should call the same function object both times.

;-------------------------

(defun f ()
  (print :f0))

(let ((old-f #'f))
  (setf old-f #’f) ;; added
  (funcall old-f)
  (defun f () (print :f1))
  (funcall old-f))

;-------------------------

Above then prints:

:F0
:F0

Setting the variable to the same value it is already bound to then doesn't show the error.
Declaring the old-f variable to be special also avoids the error.

[Stas Boukarev (stassats)]

This is basically the same as https://bugs.launchpad.net/sbcl/+bug/1289779

[Fedorov Alexander (gleefre)]

> This is basically the same as https://bugs.launchpad.net/sbcl/+bug/1289779

It seems it is not since this bug is still present while the other one was fixed.

[Douglas Katzman (dougk)]

Either set *EVALUATOR-MODE* to :INTERPRET or declare F as NOTINLINE if you "need" this to do what you think it should do.
(Multiple definitions of F aren't allowed, it doesn't matter that one isn't toplevel.)

[Fedorov Alexander (gleefre)]

AFAIK It is allowed to have multiple definitions of F in different files / in REPL.

Consider the following example:

;;;; file: test-a.lisp

(defun f (x) (* x x))

;;;; file: test-b.lisp

(defmacro memoize-function (function-name)
  `(let ((old-function (function ,function-name))
         (cache (make-hash-table)))
     (defun ,function-name (x)
       (or (gethash x cache)
           (setf (gethash x cache)
                 (funcall old-function x))))))

;;;; REPL session:

* (load "test-a.lisp")
T
* (load "test-b.lisp")
T
* (memoize-function f)
WARNING: redefining COMMON-LISP-USER::F in DEFUN
F
* (f 10)
INFO: Control stack guard page unprotected
Control stack guard page temporarily disabled: proceed with caution

debugger invoked on a SB-KERNEL::CONTROL-STACK-EXHAUSTED in thread
#<THREAD tid=381179 "main thread" RUNNING {1004960113}>:
  Control stack exhausted (no more space for function call frames).
This is probably due to heavily nested or infinitely recursive function
calls, or a tail call that SBCL cannot or has not optimized away.

This happens because the binding of old-function magically changes after the function is redefined, resulting in a recursive call instead of a call to the old definition.

[Douglas Katzman (dougk)]

I get that you're trying to make the claim that this is NOT in violation of:

"A call within a file to a named function that is defined in the same file refers to that function, unless that function has been declared notinline. The consequences are unspecified if functions are redefined individually at run time or multiply defined in the same file."
and
"Within a function named F, the compiler may (but is not required to) assume that an apparent recursive call to a function named F refers to the same definition of F, unless that function has been declared notinline. The consequences of redefining such a recursively defined function F while it is executing are undefined."

but consider what the macroexpansion actually is:

(LET ((OLD-FUNCTION #'F) (CACHE (MAKE-HASH-TABLE)))
  (DEFUN F (X)
    (OR (GETHASH X CACHE) (SETF (GETHASH X CACHE) (FUNCALL OLD-FUNCTION X)))))

F is a call to the _function_ _named_ F. Had this not been a macro, but a closure which captures the old value, then it would not be a named call to F. But it is.
I don't think there is a compelling argument that FUNCALL should _not_ consider this to be a self-recursive call.

[Fedorov Alexander (gleefre)]

I'd like to argue that (funcall old-function x) is not an apparent recursive call. Isn't it a closure that captures the old value?

We have LET that binds OLD-FUNCTION to #'F which must return the function definition in the lexical environment - and since there is no FLET/LABELS/... it should return the global functional definition.

[Fedorov Alexander (gleefre)]

I think that the problem is that (FUNCTION FOO) searches for the definition of FOO in the same compilation block, and such reference is considered constant. In particular that means that a LET variable with value #'FOO (that is not setf'ed but only referenced) can be substituted.

However, Section 3.2.2.3 Semantic Constraints talks only about function calls (and apparent function calls), while FUNCTION itself is not a function call.

Here some more examples (session in REPL)

;;;; ((λ () ...)) is used to have a single compilation block
;;;; Probably WITH-COMPILATION-UNIT can be used instead

((lambda ()
   (when nil (defun never-defined ()))
   (let ((func #'never-defined)) ;; should be an error!
     (lambda () func))))
; => #<FUNCTION (LAMBDA ()) {539B9EBB}>

(defparameter *old-foo*
  ((lambda ()
     (defun foo () "OLD")
     (constantly #'foo))))
; => *OLD-FOO*
(funcall (funcall *old-foo*)) ; should always call old definition of FOO
; => "OLD" ; so far so good
(defun foo () "NEW")
; => FOO
(funcall (funcall *old-foo*)) ; the old definition of FOO should be called
; => "BAR" ; but the new one was called instead

;; That means that CONSTANTLY, which is a function, somehow returns a
;; function that evaluates the expression #'FOO each time it is called.

[Rainer Joswig (joswig-k)]

> Am 29.06.2023 um 00:32 schrieb Douglas Katzman <email address hidden>:
>
> I get that you're trying to make the claim that this is NOT in violation
> of:
>
> "A call within a file to a named function that is defined in the same file refers to that function, unless that function has been declared notinline. The consequences are unspecified if functions are redefined individually at run time or multiply defined in the same file."
> and

Above seems only concerned with the file compiler. Neither COMPILE not EVAL compile files, but may compile code.

> "Within a function named F, the compiler may (but is not required to) assume that an apparent recursive call to a function named F refers to the same definition of F, unless that function has been declared notinline. The consequences of redefining such a recursively defined function F while it is executing are undefined."
>
> but consider what the macroexpansion actually is:
>
> (LET ((OLD-FUNCTION #'F) (CACHE (MAKE-HASH-TABLE)))
> (DEFUN F (X)
> (OR (GETHASH X CACHE) (SETF (GETHASH X CACHE) (FUNCALL OLD-FUNCTION X)))))
>
> F is a call to the _function_ _named_ F. Had this not been a macro, but a closure which captures the old value, then it would not be a named call to F. But it is.
> I don't think there is a compelling argument that FUNCALL should _not_ consider this to be a self-recursive call.

FUNCALL is not concerned with names of functions. It calls the result of an evaluation of a form. In this case it is called with a function object.

>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1653370
>
> Title:
> Lexical Binding, DEFUN inside LET - bound value changes without being
> set?
>
> Status in SBCL:
> New
>
> Bug description:
> SBCL 1.3.11 on a Mac.
>
> ;-------------------------
>
> (defun f ()
> (print :f0))
>
> (let ((old-f #'f))
> (funcall old-f)
> (defun f () (print :f1))
> (funcall old-f))
>
> ;-------------------------
>
>
> Loading/executing the above code prints
>
>
> :F0
> :F1
>
>
> I would expect:
>
> :F0
> :F0
>
> The funcall should call the same function object both times.
>
> ;-------------------------
>
> (defun f ()
> (print :f0))
>
> (let ((old-f #'f))
> (setf old-f #’f) ;; added
> (funcall old-f)
> (defun f () (print :f1))
> (funcall old-f))
>
> ;-------------------------
>
> Above then prints:
>
> :F0
> :F0
>
> Setting the variable to the same value it is already bound to then doesn't show the error.
> Declaring the old-f variable to be special also avoids the error.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/sbcl/+bug/1653370/+subscriptions

[Douglas Katzman (dougk)]

> Above seems only concerned with the file compiler. Neither COMPILE not EVAL compile files, but may compile code.

Ok then - SBCL being an "always compiled" implementation of Lisp treats the REPL as an anonymous file. Will that make you happy to think of it as such? Global redefinitions are subject to its rules about file compilation. Turn *EVALUATOR-MODE* to :INTERPRET if you don't want stdin to be file-like.

Or to be slighty more politic about it, these examples all skate on the edge of what it is permissible to do, then the user objects to compiler inserting decisions about eager versus late name capture which is an explicit liberty given in several other parts of the language spec, e.g.
 (foo (setf (symbol-function 'foo) (something)) arg)
will either call the "old" FOO or the "new" FOO - you don't get to decide. Or :TEST 'F could lookup F at each iteration. The implementation has liberty here.

So you're playing the same game with a slightly different setup and I'm not sure if all the examples are trolling or serious. In the case of "should be an error!", perhaps in safety 3, I wouldn't argue against it. But otherwise it needn't signal. (You can't write erroneous code just for its side-effect of signaling)

Basically when there are lexically apparent references to #'F where the compiler either delays or doesn't delay the lookup, it does so based on what is most efficient to compile. There are ways to make it do what you want in the presence of global function redefinition:
 - declaim things notinline
 - use (FDEFINITION 'name) or SYMBOL-FUNCTION to explicitly call a lookup at the exact time you say to. Those being ordinary functions they respect ordinary function call semantics.

Essentially what's going on here is that the compiler prefers to call via name - meaning the _current_ binding of the name - when possible because it's generally faster than calling anonymous objects.

Technique 1 to do what you want:
(defparameter *old-foo*
  ((lambda ()
     (defun foo () "OLD")
     (constantly (symbol-function 'foo))))) ; forces mapping of name to funarg
(funcall (funcall *old-foo*)) => "OLD"
(defun foo () "NEW")
(funcall (funcall *old-foo*)) => "OLD"

Technique 2 to do what you want:
(defparameter *old-foo*
  ((lambda ()
     (declare (notinline foo)) ; inhibits optimization
     (defun foo () "OLD")
     (constantly #'foo))))
(funcall (funcall *old-foo*)) => "OLD"
(defun foo () "NEW")
(funcall (funcall *old-foo*)) => "OLD"

The deal with way 1 is that SYMBOL-FUNCTION is asking explicitly to lookup FOO "now" and make a closure that calls that. Pretty clear. The deal with way 2 is that even though you think #'FOO is captured when the closure is made, it's actually only making a closure that calls the globally named thing because named call is faster (And with the original example about old-F, it's the same - the compiler has a strong predilection to forward #'f to its point of use, but call via name because it's faster)

"But we shouldn't have to" you might say. Ok, then feel free to enable a global go-slower switch, of which there are several. Declaiming (DEBUG 2) will obtain a semantic that accords with your desired outcome.
But I don'...

[Fedorov Alexander (gleefre)]

> (foo (setf (symbol-function 'foo) (something)) arg)
> will either call the "old" FOO or the "new" FOO - you don't get to decide

This is not an applicable comparison. The problem is not with the function call, but with obtaining the function definition with FUNCTION.

(let ((old-foo #'foo))
  ...)

OLD-FOO must not magically change its value [because #'foo was considered to be constant for no reason and the variable was substituted].

> But otherwise it needn't signal.
Sure, it is not required by ANSI, but not signalling is not consistent with FUNCTION's usual behavior in SBCL (it does signal an error except for that case).

> which is an explicit liberty given in several other parts of the language spec
I didn't found anything similar related to FUNCTION operator.
(FUNCTION F) must *return* the global functional definition [ when there is no definition in FLET/... ].

[Rainer Joswig (joswig-k)]

> Am 29.06.2023 um 23:23 schrieb Douglas Katzman <email address hidden>:
>
>> Above seems only concerned with the file compiler. Neither COMPILE not
> EVAL compile files, but may compile code.
>
> Ok then - SBCL being an "always compiled" implementation of Lisp treats
> the REPL as an anonymous file. Will that make you happy to think of it
> as such? Global redefinitions are subject to its rules about file
> compilation. Turn *EVALUATOR-MODE* to :INTERPRET if you don't want
> stdin to be file-like.
>
> Or to be slighty more politic about it, these examples all skate on the edge of what it is permissible to do, then the user objects to compiler inserting decisions about eager versus late name capture which is an explicit liberty given in several other parts of the language spec, e.g.
> (foo (setf (symbol-function 'foo) (something)) arg)
> will either call the "old" FOO or the "new" FOO - you don't get to decide. Or :TEST 'F could lookup F at each iteration. The implementation has liberty here.
>
> So you're playing the same game with a slightly different setup and I'm
> not sure if all the examples are trolling or serious. In the case of
> "should be an error!", perhaps in safety 3, I wouldn't argue against it.
> But otherwise it needn't signal. (You can't write erroneous code just
> for its side-effect of signaling)
>
> Basically when there are lexically apparent references to #'F where the compiler either delays or doesn't delay the lookup, it does so based on what is most efficient to compile. There are ways to make it do what you want in the presence of global function redefinition:
> - declaim things notinline
> - use (FDEFINITION 'name) or SYMBOL-FUNCTION to explicitly call a lookup at the exact time you say to. Those being ordinary functions they respect ordinary function call semantics.
>
> Essentially what's going on here is that the compiler prefers to call
> via name - meaning the _current_ binding of the name - when possible
> because it's generally faster than calling anonymous objects.
>
> Technique 1 to do what you want:
> (defparameter *old-foo*
> ((lambda ()
> (defun foo () "OLD")
> (constantly (symbol-function 'foo))))) ; forces mapping of name to funarg
> (funcall (funcall *old-foo*)) => "OLD"
> (defun foo () "NEW")
> (funcall (funcall *old-foo*)) => "OLD"
>
> Technique 2 to do what you want:
> (defparameter *old-foo*
> ((lambda ()
> (declare (notinline foo)) ; inhibits optimization
> (defun foo () "OLD")
> (constantly #'foo))))
> (funcall (funcall *old-foo*)) => "OLD"
> (defun foo () "NEW")
> (funcall (funcall *old-foo*)) => "OLD"
>
> The deal with way 1 is that SYMBOL-FUNCTION is asking explicitly to
> lookup FOO "now" and make a closure that calls that. Pretty clear. The
> deal with way 2 is that even though you think #'FOO is captured when the
> closure is made, it's actually only making a closure that calls the
> globally named thing because named call is faster (And with the original
> example about old-F, it's the same - the compiler has a strong
> predilection to forward #'f to its point of use, but call via name
> because it's faster)
>
> "But we shouldn't have to" you migh...

[Fedorov Alexander (gleefre)]

I still believe it is a bug, since the standard talks about freedom in function calls, not about #'FOO (no function call happens here). The actual problem is that (let ((old-function #'FOO)) ...) substitutes the variable (counts #'FOO as a constant expression).

After some studying, it seems that the sequence of events is the following: FUNCTION gets translated by ir1 translator using FIND-OR-CONVERT-FUN-LEAF, which calls FIND-LEXICALLY-APPARENT-FUN to get function definition. It uses FIND-FREE-FUN to get the global function definition which returns an instance of DEFINED-FUN when there is a DEFUN / named-lambda form in the compilation block. When function is not declared notinline DEFINED-FUN is counted as constant reference by CONSTANT-REFERENCE-P function, and thus it gets substituted during PROPAGATE-LET-ARGS. [ unless (debug 3) says to keep the variable / there are sets to the variable ]

[ By the way, it looks like this bug was inherited from CMUCL's compiler (it manifests in compiled functions there as well). ]

I think that DEFINED-FUN should not be treated as constant reference since the global function definition can be redefined in the body of the let. Another solution might be to make a global reference when translation FUNCTION (using FIND-GLOBAL-FUN instead of FIND-FREE-FUN).

[Fedorov Alexander (gleefre)]

Here is a simple patch with the first solution.