Subject: bug#55156: [PATCH] eval.c: New functions `defvar-f` and `defconst-f`

From: Stefan Monnier <[email-masked]>
Date: Thu, 28 Apr 2022 09:26:50 -0400

>> -If @var{symbol} is already lexically bound (e.g., if the @code{defvar}
>> -form occurs in a @code{let} form with lexical binding enabled), then
>> -@code{defvar} sets the dynamic value.  The lexical binding remains in
>> +If @var{symbol} is already let bound (e.g., if the @code{defvar}
>> +form occurs in a @code{let} form), then @code{defvar} sets the dynamic
>> +outer value.  The let binding remains in
>
> What is "dynamic outer value"?  We don't have such terminology
> anywhere in the manual.

Good point, I should try and use the same terminology used by
`default-toplevel-value` (and maybe refer to this function), thanks.

>> @@ -763,17 +763,14 @@ DEFUN ("defvar", Fdefvar, Sdefvar, 1, UNEVALLED, 0,
>>  so that it is always dynamically bound even if `lexical-binding' is t.
>>  
>>  If SYMBOL's value is void and the optional argument INITVALUE is
>> -provided, INITVALUE is evaluated and the result used to set SYMBOL's
>> -value.  If SYMBOL is buffer-local, its default value is what is set;
>> +provided, INITVALUE is used to set SYMBOL's value.
>> +If SYMBOL is buffer-local, its default value is what is set;
>>  buffer-local values are not affected.  If INITVALUE is missing,
>>  SYMBOL's value is not set.
>
> This loses information, AFAIU: the previous doc string said INITVALUE
> was evaluated, the new one says nothing at all about evaluating it.
> If it is evaluated in some cases, please mention that; if it isn't
> evaluated at all, please say that.

I hesitated here.  I preferred to remove the promise of when it's
evaluated (which we currently break in some cases) rather than make
a different promise, incompatible with the previous one.

But now that Lars made me see that we actually do hold the promise in
most cases, I think the better course of action is to keep the promise
and fix the cases where we break it.

>> -If SYMBOL has a local binding, then this form affects the local
>> -binding.  This is usually not what you want.  Thus, if you need to
>> -load a file defining variables, with this form or with `defconst' or
>> -`defcustom', you should always load that file _outside_ any bindings
>> -for these variables.  (`defconst' and `defcustom' behave similarly in
>> -this respect.)
>> +If SYMBOL is let-bound, then this form does not affect the local let
>> +binding but the outer (toplevel) binding.
>> +(`defcustom' behaves similarly in this respect.)
>
> Isn't _this_ change (if it indeed constitutes a change in behavior)
> scary?

It was scary, yes, but that change happened back in:

    commit a104f656c8217b027866d32e8d7bf024a671e3cc
    Author: Stefan Monnier <[email-masked]>
    Date:   Fri Aug 2 17:16:33 2013 -0400

    Make defvar affect the default binding outside of any let.
    * src/eval.c (default_toplevel_binding): New function.
    (Fdefvar): Use it.
    (unbind_to, backtrace_eval_unrewind): Do a bit of CSE simplification.
    (Fdefault_toplevel_value, Fset_default_toplevel_value): New subrs.
    (syms_of_eval): Export them.
    * src/data.c (Fdefault_value): Micro cleanup.
    * src/term.c (init_tty): Use "false".
    * lisp/custom.el (custom-initialize-default, custom-initialize-set)
    (custom-initialize-reset, custom-initialize-changed): Affect the
    toplevel-default-value (bug#6275, bug#14586).
    * lisp/emacs-lisp/advice.el (ad-compile-function): Undo previous workaround
    for bug#6275.
    * test/automated/core-elisp-tests.el: New file.

So this is just a long-overdue fix of its doc.

>> +DEFUN ("defvar-f", Fdefvar_f, Sdefvar_f, 2, 3, 0,
>> +       doc: /* Like `defvar' but as a function.  */)
>
> Please improve the doc string here.
>
>> +DEFUN ("defconst-f", Fdefconst_f, Sdefconst_f, 2, 3, 0,
>> +       doc: /* Like `defconst' but as a function.  */)
>
> Likewise.

How 'bout I use a double dash to make it more clear they're internal?

        Stefan