bug#54399: 27.2; Problems with (let ((custom-variable ...)) (autoload-function ...))

[Ignacio Casso]

Hello,

I reported this org-mode bug in
https://lists.gnu.org/archive/html/emacs-orgmode/2022-03/msg00085.html,
but after some discussion we figured out that the issue is not
particular to org-mode but generic to all Emacs and decided to bring our
conclusions and questions  here. The problem is the following:

Calling an autoload function under the following circumstances does not
always work as expected:

  - the function uses a variable defined with defcustom in the same file
    as the function.

  - the function is called inside a let form that binds that same
    variable.

  - the file defining the function and the variable has not been loaded
    yet at the time the function is called, and the variable has not
    been set either.

I would expect to work exactly the same as if the file had already been
loaded. Instead, the following happens depending on the scoping and the
defcustom setter:

1) If the let form is evaluated with lexical-binding, and the variable
is not also autoloaded, Emacs does not know the variable is special by
the time it evaluates let, so it uses lexical binding. Later, when it
evaluates defcustom and it finds out that it is special and it should
have used dynamic binding, Emacs 29 produces the error (error "Defining
as dynamic an already lexical var"). Emacs 27 does not perform this
check and keeps going, and since it uses lexical binding, the let
binding has not effect at all inside the function as the user would
expect. The exact same thing happens also if the variable is defined
with defvar. I guess there is nothing that can be done about it
otherwise Emacs 29 would have done so instead of throwing an error. The
following form reproduces this behavior (please ensure to evaluate it
with lexical binding):

(progn

  (defun my-load ()
    (defvar my-var 1)
    (message "Value while loading: %s" my-var))

  (defun my-var-alias () my-var)

  (let ((my-var 2))
    (my-load)
    (message "Lexical value inside let: %s" my-var)
    (message "Dynamic value inside let: %s" (my-var-alias)))
  (message "Value ouside let: %s" my-var))


2) If the let form is evaluated with dynamic binding, or the variable
has also an autoload cookie so Emacs already knows is dynamic, then the
behavior depends on the :set argument of defcustom:

2.1) If no explicit argument is passed, then defcustom uses as default
set-default-toplevel-value. In that case everything works as
expected. Note however that the documentation and comments says in many
places that the default :set argument is just set-default instead of
set-default-toplevel-value.

2.2) If the :set argument is set or set-default (the suggested default
choice in the documentation), that function is called with arguments the
variable symbol and the standard value passed as argument to defcustom.
But those functions only affect the scope of the let binding, which
means that a) they overwrite the let binding, which is not what the user
expect, and b) when the evaluation of the let form finish the variable
is void. Thus, any further use of that variable or functions that use it
will produce a void variable error. And this is not trivial to fix:
requiring the feature again will do nothing since it's already provided,
so the user needs to finds it's definition and evaluate defvar/defcustom
again himself, or restart Emacs. The following form reproduces this
behavior (please ensure to evaluate it with dynamic binding):

(progn

  (defun my-load ()
    (defcustom my-other-var 1 "Test variable" :set 'set-default)
    (message "Value while loading: %s" my-other-var))

  (let ((my-other-var 2))
    (my-load)
    (message "Value inside let: %s" (my-other-var-alias)))
  (message "Value ouside let: %s" my-other-var))


I think that something should be done about point 2.2. Some suggestions
are:

- A warning when defcustom of a variable is called inside a let binding
  of that same variable.

- Update documentation of defcustom to say that the default choice of
  the :set argument is set-default-toplevel-value

- Document default-value, default-boundp, and set-default to say that
  they may not work as the user expects when called inside a let binding
  with dynamic binding enabled. The snippets below show how I expected
  them to work (please evaluate them with dynamic binding):

  (let ((fresh-var 1))
    (default-value 'fresh-var)) ;; I expect and error, it returns 1

  (let ((another-fresh-var 1))
    (default-boundp 'another-fresh-var)) ;; I expect nil, it returns t

  (defvar yet-another-fresh-var 1)
  (let ((yet-another-fresh-var 2))
    (set-default 'yet-another-fresh-var 3)
    yet-another-fresh-var) ;; I expect 2, it returns 3
  yet-another-fresh-var ;; I expect 3, it returns 1


What do you think?