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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 26 bytes --]

Tags: patch

Tags: patch


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-eval.c-New-functions-defvar-f-and-defconst-f.patch --]
[-- Type: text/patch, Size: 10462 bytes --]

From b0e07492cfe82ab3c49e663e72188ba5e90b7f76 Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Wed, 27 Apr 2022 17:44:20 -0400
Subject: [PATCH] eval.c: New functions `defvar-f` and `defconst-f`

The bytecode interpreter can't directly call special forms, so
the byte-compiler usually converts special forms into some sequence of
byte codes (basically, providing a duplicate definition of the special
form).  There are still two exceptions to this: `defconst` and `defvar`,
where the compiler instead generates a convoluted chunk of code like:

    (funcall '(lambda (x) (defvar <sym> x <doc>)) <value>)

where the quote makes sure we keep the function non-compiled, so as
to end up running the special form at run time.

The patch below gets rid of this workaround by introducing `defvar-f`
and `defconst-f` which provide a *functional* interface to the
functionality of the corresponding special form.

This changes the behavior of (defvar <sym> <exp>) because
(defvar-f '<sym> <exp>) will now always evaluate <exp> whereas
previously the doc promised that <exp> would only be evaluated if
<sym> was not yet bound.

This sounds scary, but the reality is less so: while the behavior of
the special form obeyed its doc in this respect, the behavior of the
convoluted code generated by the byte-compiler did not(!) and always
evaluated the <exp> part anyway.  So this patch also aligns the two
semantics to provide the same behavior.

* src/eval.c (Fdefvar_f, Fdefconst_f): New functions, extracted from
`Fdef(var|const)`.
(Fdefvar, Fdefconst): Use them.
(syms_of_eval): `defsubr` the new functions.

* lisp/emacs-lisp/bytecomp.el (byte-compile-tmp-var): Delete const.
(byte-compile-defvar): Simplify using the new `def(car|const)-f` functions.

* doc/lispref/variables.texi (Defining Variables): Adjust the doc of
`defvar` to reflect the actual semantics implemented.  Don't state
explicitly if the `value` is always evaluated or not.
---
 doc/lispref/variables.texi  | 14 ++++----
 lisp/emacs-lisp/bytecomp.el | 20 ++++-------
 src/eval.c                  | 72 +++++++++++++++++++++++--------------
 3 files changed, 58 insertions(+), 48 deletions(-)

diff --git a/doc/lispref/variables.texi b/doc/lispref/variables.texi
index f0e3f337a69..264fcbcfe8e 100644
--- a/doc/lispref/variables.texi
+++ b/doc/lispref/variables.texi
@@ -510,10 +510,10 @@ Defining Variables
 (@pxref{Variable Scoping}).
 
 If @var{value} is specified, and @var{symbol} is void (i.e., it has no
-dynamically bound value; @pxref{Void Variables}), then @var{value} is
-evaluated and @var{symbol} is set to the result.  But if @var{symbol}
-is not void, @var{value} is not evaluated, and @var{symbol}'s value is
-left unchanged.  If @var{value} is omitted, the value of @var{symbol}
+dynamically bound value; @pxref{Void Variables}), then @var{symbol} is
+set to the result of evaluating of @var{value}.  But if @var{symbol}
+is not void @var{symbol}'s value is left unchanged.
+If @var{value} is omitted, the value of @var{symbol}
 is not changed in any case.
 
 Note that specifying a value, even @code{nil}, marks the variable as
@@ -527,9 +527,9 @@ Defining Variables
 rather than the buffer-local binding.  It sets the default value if
 the default value is void.  @xref{Buffer-Local Variables}.
 
-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
 effect until its binding construct exits.  @xref{Variable Scoping}.
 
 @cindex @code{eval-defun}, and @code{defvar} forms
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index c0dffe544cf..68a664c7129 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -4887,8 +4887,6 @@ byte-compile-make-obsolete-variable
     (push (nth 1 (nth 1 form)) byte-compile-global-not-obsolete-vars))
   (byte-compile-normal-call form))
 
-(defconst byte-compile-tmp-var (make-symbol "def-tmp-var"))
-
 (defun byte-compile-defvar (form)
   ;; This is not used for file-level defvar/consts.
   (when (and (symbolp (nth 1 form))
@@ -4901,7 +4899,6 @@ byte-compile-defvar
   (byte-compile-docstring-length-warn form)
   (let ((fun (nth 0 form))
 	(var (nth 1 form))
-	(value (nth 2 form))
 	(string (nth 3 form)))
     (when (or (> (length form) 4)
 	      (and (eq fun 'defconst) (null (cddr form))))
@@ -4922,17 +4919,12 @@ byte-compile-defvar
        "third arg to `%s %s' is not a string: %s"
        fun var string))
     (byte-compile-form-do-effect
-     (if (cddr form)  ; `value' provided
-         ;; Quote with `quote' to prevent byte-compiling the body,
-         ;; which would lead to an inf-loop.
-         `(funcall '(lambda (,byte-compile-tmp-var)
-                      (,fun ,var ,byte-compile-tmp-var ,@(nthcdr 3 form)))
-                   ,value)
-        (if (eq fun 'defconst)
-            ;; This will signal an appropriate error at runtime.
-            `(eval ',form)
-          ;; A simple (defvar foo) just returns foo.
-          `',var)))))
+     (if (or (cddr form)  ; `value' provided
+             (eq fun 'defconst))
+         ;; Delegate the actual work to the `-f' version of the special form.
+         `(,(intern (format "%s-f" fun)) ',var ,@(nthcdr 2 form))
+       ;; A simple (defvar foo) just returns foo.
+       `',var))))
 
 (defun byte-compile-autoload (form)
   (and (macroexp-const-p (nth 1 form))
diff --git a/src/eval.c b/src/eval.c
index 77ec47e2b79..10212708c23 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -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.
 
-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.)
 
 The optional argument DOCSTRING is a documentation string for the
 variable.
@@ -784,7 +781,7 @@ DEFUN ("defvar", Fdefvar, Sdefvar, 1, UNEVALLED, 0,
 usage: (defvar SYMBOL &optional INITVALUE DOCSTRING)  */)
   (Lisp_Object args)
 {
-  Lisp_Object sym, tem, tail;
+  Lisp_Object sym, tail;
 
   sym = XCAR (args);
   tail = XCDR (args);
@@ -796,24 +793,8 @@ DEFUN ("defvar", Fdefvar, Sdefvar, 1, UNEVALLED, 0,
       if (!NILP (XCDR (tail)) && !NILP (XCDR (XCDR (tail))))
 	error ("Too many arguments");
       Lisp_Object exp = XCAR (tail);
-
-      tem = Fdefault_boundp (sym);
       tail = XCDR (tail);
-
-      /* Do it before evaluating the initial value, for self-references.  */
-      Finternal__define_uninitialized_variable (sym, CAR (tail));
-
-      if (NILP (tem))
-	Fset_default (sym, eval_sub (exp));
-      else
-	{ /* Check if there is really a global binding rather than just a let
-	     binding that shadows the global unboundness of the var.  */
-	  union specbinding *binding = default_toplevel_binding (sym);
-	  if (binding && EQ (specpdl_old_value (binding), Qunbound))
-	    {
-	      set_specpdl_old_value (binding, eval_sub (exp));
-	    }
-	}
+      return Fdefvar_f (sym, eval_sub (exp), CAR (tail));
     }
   else if (!NILP (Vinternal_interpreter_environment)
 	   && (SYMBOLP (sym) && !XSYMBOL (sym)->u.s.declared_special))
@@ -832,6 +813,33 @@ DEFUN ("defvar", Fdefvar, Sdefvar, 1, UNEVALLED, 0,
   return sym;
 }
 
+DEFUN ("defvar-f", Fdefvar_f, Sdefvar_f, 2, 3, 0,
+       doc: /* Like `defvar' but as a function.  */)
+  (Lisp_Object sym, Lisp_Object initvalue, Lisp_Object docstring)
+{
+  Lisp_Object tem;
+
+  CHECK_SYMBOL (sym);
+
+  tem = Fdefault_boundp (sym);
+
+  /* Do it before evaluating the initial value, for self-references.  */
+  Finternal__define_uninitialized_variable (sym, docstring);
+
+  if (NILP (tem))
+    Fset_default (sym, initvalue);
+  else
+    { /* Check if there is really a global binding rather than just a let
+	     binding that shadows the global unboundness of the var.  */
+      union specbinding *binding = default_toplevel_binding (sym);
+      if (binding && EQ (specpdl_old_value (binding), Qunbound))
+	{
+	  set_specpdl_old_value (binding, initvalue);
+	}
+    }
+  return sym;
+}
+
 DEFUN ("defconst", Fdefconst, Sdefconst, 2, UNEVALLED, 0,
        doc: /* Define SYMBOL as a constant variable.
 This declares that neither programs nor users should ever change the
@@ -861,9 +869,17 @@ DEFUN ("defconst", Fdefconst, Sdefconst, 2, UNEVALLED, 0,
 	error ("Too many arguments");
       docstring = XCAR (XCDR (XCDR (args)));
     }
+  tem = eval_sub (XCAR (XCDR (args)));
+  return Fdefconst_f (sym, tem, docstring);
+}
 
+DEFUN ("defconst-f", Fdefconst_f, Sdefconst_f, 2, 3, 0,
+       doc: /* Like `defconst' but as a function.  */)
+  (Lisp_Object sym, Lisp_Object initvalue, Lisp_Object docstring)
+{
+  CHECK_SYMBOL (sym);
+  Lisp_Object tem = initvalue;
   Finternal__define_uninitialized_variable (sym, docstring);
-  tem = eval_sub (XCAR (XCDR (args)));
   if (!NILP (Vpurify_flag))
     tem = Fpurecopy (tem);
   Fset_default (sym, tem);      /* FIXME: set-default-toplevel-value? */
@@ -4325,9 +4341,11 @@ syms_of_eval (void)
   defsubr (&Sdefault_toplevel_value);
   defsubr (&Sset_default_toplevel_value);
   defsubr (&Sdefvar);
+  defsubr (&Sdefvar_f);
   defsubr (&Sdefvaralias);
   DEFSYM (Qdefvaralias, "defvaralias");
   defsubr (&Sdefconst);
+  defsubr (&Sdefconst_f);
   defsubr (&Sinternal__define_uninitialized_variable);
   defsubr (&Smake_var_non_special);
   defsubr (&Slet);
-- 
2.35.1

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

[Lars Ingebrigtsen]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> This sounds scary, but the reality is less so: while the behavior of
> the special form obeyed its doc in this respect, the behavior of the
> convoluted code generated by the byte-compiler did not(!) and always
> evaluated the <exp> part anyway.  So this patch also aligns the two
> semantics to provide the same behavior.

Uhm -- are you saying that if you load an .elc file twice, the <exp>
parts in the defvars will be evaluated twice?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2022-04-28 00:11:05] wrote:
> Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
>
>> This sounds scary, but the reality is less so: while the behavior of
>> the special form obeyed its doc in this respect, the behavior of the
>> convoluted code generated by the byte-compiler did not(!) and always
>> evaluated the <exp> part anyway.  So this patch also aligns the two
>> semantics to provide the same behavior.
>
> Uhm -- are you saying that if you load an .elc file twice, the <exp>
> parts in the defvars will be evaluated twice?

Try:

    (let ((f (byte-compile
              '(lambda (x)
                 (defvar sm-x (progn (message "hello %S" x) x)))))) 
      (funcall f 5)
      (funcall f 6))

and check *Messages* :-(

If we prefer keeping the behavior we currently promise, we can of course
do that (just change `defvar-f` so it takes a function of no argument as
second arg, it makes the generated code (and the C code) a bit less
simple, but it's no worse than what we currently have).


        Stefan

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

[Lars Ingebrigtsen]

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

>> Uhm -- are you saying that if you load an .elc file twice, the <exp>
>> parts in the defvars will be evaluated twice?
>
> Try:
>
>     (let ((f (byte-compile
>               '(lambda (x)
>                  (defvar sm-x (progn (message "hello %S" x) x)))))) 
>       (funcall f 5)
>       (funcall f 6))
>
> and check *Messages* :-(

Oh, if we call a function containing the defvar...  Yes, that's probably
rare enough that nobody's noticed.

I tried to byte-compile a file and just load the .elc a few times, and
the message was only done once:

(defvar this-thing (message "hello"))

> If we prefer keeping the behavior we currently promise, we can of course
> do that (just change `defvar-f` so it takes a function of no argument as
> second arg, it makes the generated code (and the C code) a bit less
> simple, but it's no worse than what we currently have).

I think I'd prefer keeping the behaviour we currently promise, but I
don't have a strong opinion.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Try:
>>
>>     (let ((f (byte-compile
>>               '(lambda (x)
>>                  (defvar sm-x (progn (message "hello %S" x) x)))))) 
>>       (funcall f 5)
>>       (funcall f 6))
>>
>> and check *Messages* :-(
>
> Oh, if we call a function containing the defvar...  Yes, that's probably
> rare enough that nobody's noticed.
>
> I tried to byte-compile a file and just load the .elc a few times, and
> the message was only done once:
>
> (defvar this-thing (message "hello"))

Duh, I forgot about the toplevel special case, indeed.  OK, now it makes sense.

>> If we prefer keeping the behavior we currently promise, we can of course
>> do that (just change `defvar-f` so it takes a function of no argument as
>> second arg, it makes the generated code (and the C code) a bit less
>> simple, but it's no worse than what we currently have).
> I think I'd prefer keeping the behaviour we currently promise,

So do I.  I'll see about updating the patch.


        Stefan

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

[Eli Zaretskii]

> Date: Wed, 27 Apr 2022 17:46:22 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> The bytecode interpreter can't directly call special forms, so
> the byte-compiler usually converts special forms into some sequence of
> byte codes (basically, providing a duplicate definition of the special
> form).  There are still two exceptions to this: `defconst` and `defvar`,
> where the compiler instead generates a convoluted chunk of code like:
> 
>     (funcall '(lambda (x) (defvar <sym> x <doc>)) <value>)
> 
> where the quote makes sure we keep the function non-compiled, so as
> to end up running the special form at run time.
> 
> The patch below gets rid of this workaround by introducing `defvar-f`
> and `defconst-f` which provide a *functional* interface to the
> functionality of the corresponding special form.

I have (almost) no opinion on the code changes, but the documentation
changes "need work", IMO:

>  If @var{value} is specified, and @var{symbol} is void (i.e., it has no
> -dynamically bound value; @pxref{Void Variables}), then @var{value} is
> -evaluated and @var{symbol} is set to the result.  But if @var{symbol}
> -is not void, @var{value} is not evaluated, and @var{symbol}'s value is
> -left unchanged.  If @var{value} is omitted, the value of @var{symbol}
> +dynamically bound value; @pxref{Void Variables}), then @var{symbol} is
> +set to the result of evaluating of @var{value}.  But if @var{symbol}
> +is not void @var{symbol}'s value is left unchanged.
> +If @var{value} is omitted, the value of @var{symbol}
>  is not changed in any case.

The new text lacks a comma after "not void", and "result of evaluating
of VALUE" is IMO not good English.  If all you wanted was to remove
"is not evaluated", why not just do that and leave the rest as it was?

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

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

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

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

Thanks.

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

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 28 Apr 2022 00:33:47 +0200
> Cc: 55156@debbugs.gnu.org
> 
> Oh, if we call a function containing the defvar...  Yes, that's probably
> rare enough that nobody's noticed.

Famous last words.

> I think I'd prefer keeping the behaviour we currently promise, but I
> don't have a strong opinion.

I sincerely question the wisdom of messing with this, for the reasons
that were described, which seem to be some inelegant code somewhere in
the bowels of the byte compiler.  Do we really care enough about such
inelegance to make potentially breaking changes in code that works for
decades and causes zero trouble to Lisp programmers?

And I'm quite sure that the replacement code might look no more
elegant to people other than the author.

I suggest that we all take a step back and re-evaluate the need for
this.  It is IME exactly the kind of change that prevents Emacs from
becoming steadily more and more stable as time goes by.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> -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 <monnier@iro.umontreal.ca>
    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

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

[Lars Ingebrigtsen]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

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

Sounds good, but perhaps also make them less crypting?  I.e., something
like `defvar--as-function' or something along those lines.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Sounds good, but perhaps also make them less crypting?  I.e., something
> like `defvar--as-function' or something along those lines.

Or even `bytecomp--defvar-as-function'.  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 55156@debbugs.gnu.org
> Date: Thu, 28 Apr 2022 09:26:50 -0400
> 
> >> +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?

It should still say something about its intended usage, with Emacs
developers in mind, IMO.  Because I doubt that we are going to
document them in the ELisp reference manual, and nor do I think we
should.  So this place here is the only place where we can say
something about these functions, so that future hackers of the byte
compiler could maintain this.

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

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

We don't have a convention of an `-f' suffix in function names.
That form of name is extra cryptic.
Let's choose names that follow some existing pattern
rather than being inconsistent with old practice.

I suggest `defvar-internal', since it isn't meant for users to call.
Even `defvar-function' would be better than `defvar-f'.

For the doc string, it is better to say in a self-contained way what
the function does, rather than only make an analogy.

How about this:

   Define the variable VAR, with initial value INITVAL and doc string DOC.

Note that `defvar', being a special form, can distinguish between nil
as INITVAL and having only one argument.  However, a function cannot
do that: it will treat those two cases the same.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I sincerely question the wisdom of messing with this, for the reasons
  > that were described, which seem to be some inelegant code somewhere in
  > the bowels of the byte compiler.  Do we really care enough about such
  > inelegance to make potentially breaking changes in code that works for
  > decades and causes zero trouble to Lisp programmers?

I feel the same.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > This sounds scary, but the reality is less so: while the behavior of
  > the special form obeyed its doc in this respect, the behavior of the
  > convoluted code generated by the byte-compiler did not(!) and always
  > evaluated the <exp> part anyway.  So this patch also aligns the two
  > semantics to provide the same behavior.

I have a feeling that that discrepancy will cause real trouble some
day.  However, fixing it can cause trouble too.  I think this calls
for real thought.


-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Here's a new version of my patch.

Most importantly the "defvar function" now only assigns the value to the
var if the var was not already defined (i.e. it keeps the current
behavior).

> What is "dynamic outer value"?  We don't have such terminology
> anywhere in the manual.

I changed it to use the same terminology as
`set-toplevel-default-binding` (and to refer to that as well).

> 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 reverted this part of the change.

>> -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's only a change in the doc (the corresponding code
change took place several years ago).

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

I added a line which defines it precisely in terms of `defvar`.

Richard Stallman [2022-04-28 23:10:40] wrote:
> We don't have a convention of an `-f' suffix in function names.
> That form of name is extra cryptic.
> Let's choose names that follow some existing pattern
> rather than being inconsistent with old practice.

I see that at various other places where we have a macro expand to
a call to an "internal" function, we name that function with a "-1"
suffix, so I decided to follow that convention.


        Stefan


The bytecode interpreter can't directly call special forms, so
the byte-compiler usually converts special forms into some sequence of
byte codes (basically, providing a duplicate definition of the special
form).  There are still two exceptions to this: `defconst` and `defvar`,
where the compiler instead generates a convoluted chunk of code like:

    (funcall '(lambda (x) (defvar <sym> x <doc>)) <value>)

where the quote makes sure we keep the function non-compiled, so as
to end up running the special form at run time.

The patch below gets rid of this workaround by introducing `defvar-1`
and `defconst-1` which provide a *functional* interface to the
functionality of the corresponding special form.

* src/eval.c (defvar, Fdefvar_1, Fdefconst_1): New functions, extracted from
`Fdef(var|const)`.
(Fdefvar, Fdefconst): Use them.
(syms_of_eval): `defsubr` the new functions.

* lisp/emacs-lisp/bytecomp.el (byte-compile-tmp-var): Delete const.
(byte-compile-defvar): Simplify using the new `def(car|const)-1` functions.

* doc/lispref/variables.texi (Defining Variables): Adjust the doc of
`defvar` to reflect the actual semantics implemented.


diff --git a/doc/lispref/variables.texi b/doc/lispref/variables.texi
index f0e3f337a69..c29547d00db 100644
--- a/doc/lispref/variables.texi
+++ b/doc/lispref/variables.texi
@@ -527,10 +527,11 @@ Defining Variables
 rather than the buffer-local binding.  It sets the default value if
 the default value is void.  @xref{Buffer-Local Variables}.
 
-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
-effect until its binding construct exits.  @xref{Variable Scoping}.
+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 toplevel
+default value, like @code{set-default-toplevel-value}.
+The let binding remains in effect until its binding construct exits.
+@xref{Variable Scoping}.
 
 @cindex @code{eval-defun}, and @code{defvar} forms
 @cindex @code{eval-last-sexp}, and @code{defvar} forms
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 87798288fb5..b07e7b4e57f 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -4948,8 +4948,6 @@ byte-compile-make-obsolete-variable
     (push (nth 1 (nth 1 form)) byte-compile-global-not-obsolete-vars))
   (byte-compile-normal-call form))
 
-(defconst byte-compile-tmp-var (make-symbol "def-tmp-var"))
-
 (defun byte-compile-defvar (form)
   ;; This is not used for file-level defvar/consts.
   (when (and (symbolp (nth 1 form))
@@ -4962,7 +4960,6 @@ byte-compile-defvar
   (byte-compile-docstring-style-warn form)
   (let ((fun (nth 0 form))
 	(var (nth 1 form))
-	(value (nth 2 form))
 	(string (nth 3 form)))
     (when (or (> (length form) 4)
 	      (and (eq fun 'defconst) (null (cddr form))))
@@ -4982,18 +4979,16 @@ byte-compile-defvar
        string
        "third arg to `%s %s' is not a string: %s"
        fun var string))
+    ;; Delegate the actual work to the function version of the
+    ;; special form, named with a "-1" suffix.
     (byte-compile-form-do-effect
-     (if (cddr form)  ; `value' provided
-         ;; Quote with `quote' to prevent byte-compiling the body,
-         ;; which would lead to an inf-loop.
-         `(funcall '(lambda (,byte-compile-tmp-var)
-                      (,fun ,var ,byte-compile-tmp-var ,@(nthcdr 3 form)))
-                   ,value)
-        (if (eq fun 'defconst)
-            ;; This will signal an appropriate error at runtime.
-            `(eval ',form)
-          ;; A simple (defvar foo) just returns foo.
-          `',var)))))
+     (cond
+      ((eq fun 'defconst) `(defconst-1 ',var ,@(nthcdr 2 form)))
+      ((not (cddr form)) `',var) ; A simple (defvar foo) just returns foo.
+      (t `(defvar-1 ',var
+                    ;; Don't eval `value' if `defvar' wouldn't eval it either.
+                    (if (boundp ',var) nil ,(nth 2 form))
+                    ,@(nthcdr 3 form)))))))
 
 (defun byte-compile-autoload (form)
   (and (macroexp-const-p (nth 1 form))
diff --git a/src/eval.c b/src/eval.c
index 08e2dce61e4..c3be1dc12c8 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -756,6 +756,33 @@ DEFUN ("internal--define-uninitialized-variable",
   return Qnil;
 }
 
+static Lisp_Object
+defvar (Lisp_Object sym, Lisp_Object initvalue, Lisp_Object docstring, bool eval)
+{
+  Lisp_Object tem;
+
+  CHECK_SYMBOL (sym);
+
+  tem = Fdefault_boundp (sym);
+
+  /* Do it before evaluating the initial value, for self-references.  */
+  Finternal__define_uninitialized_variable (sym, docstring);
+
+  if (NILP (tem))
+    Fset_default (sym, eval ? eval_sub (initvalue) : initvalue);
+  else
+    { /* Check if there is really a global binding rather than just a let
+	     binding that shadows the global unboundness of the var.  */
+      union specbinding *binding = default_toplevel_binding (sym);
+      if (binding && EQ (specpdl_old_value (binding), Qunbound))
+	{
+	  set_specpdl_old_value (binding,
+	                         eval ? eval_sub (initvalue) : initvalue);
+	}
+    }
+  return sym;
+}
+
 DEFUN ("defvar", Fdefvar, Sdefvar, 1, UNEVALLED, 0,
        doc: /* Define SYMBOL as a variable, and return SYMBOL.
 You are not required to define a variable in order to use it, but
@@ -770,12 +797,10 @@ DEFUN ("defvar", Fdefvar, Sdefvar, 1, UNEVALLED, 0,
 buffer-local values are not affected.  If INITVALUE is missing,
 SYMBOL's value is not set.
 
-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 toplevel default binding instead, like
+`set-toplevel-default-binding`.
+(`defcustom' behaves similarly in this respect.)
 
 The optional argument DOCSTRING is a documentation string for the
 variable.
@@ -786,7 +811,7 @@ DEFUN ("defvar", Fdefvar, Sdefvar, 1, UNEVALLED, 0,
 usage: (defvar SYMBOL &optional INITVALUE DOCSTRING)  */)
   (Lisp_Object args)
 {
-  Lisp_Object sym, tem, tail;
+  Lisp_Object sym, tail;
 
   sym = XCAR (args);
   tail = XCDR (args);
@@ -798,24 +823,8 @@ DEFUN ("defvar", Fdefvar, Sdefvar, 1, UNEVALLED, 0,
       if (!NILP (XCDR (tail)) && !NILP (XCDR (XCDR (tail))))
 	error ("Too many arguments");
       Lisp_Object exp = XCAR (tail);
-
-      tem = Fdefault_boundp (sym);
       tail = XCDR (tail);
-
-      /* Do it before evaluating the initial value, for self-references.  */
-      Finternal__define_uninitialized_variable (sym, CAR (tail));
-
-      if (NILP (tem))
-	Fset_default (sym, eval_sub (exp));
-      else
-	{ /* Check if there is really a global binding rather than just a let
-	     binding that shadows the global unboundness of the var.  */
-	  union specbinding *binding = default_toplevel_binding (sym);
-	  if (binding && EQ (specpdl_old_value (binding), Qunbound))
-	    {
-	      set_specpdl_old_value (binding, eval_sub (exp));
-	    }
-	}
+      return defvar (sym, exp, CAR (tail), true);
     }
   else if (!NILP (Vinternal_interpreter_environment)
 	   && (SYMBOLP (sym) && !XSYMBOL (sym)->u.s.declared_special))
@@ -834,6 +843,14 @@ DEFUN ("defvar", Fdefvar, Sdefvar, 1, UNEVALLED, 0,
   return sym;
 }
 
+DEFUN ("defvar-1", Fdefvar_1, Sdefvar_1, 2, 3, 0,
+       doc: /* Like `defvar' but as a function.
+More specifically behaves like (defvar SYM 'INITVALUE DOCSTRING).  */)
+  (Lisp_Object sym, Lisp_Object initvalue, Lisp_Object docstring)
+{
+  return defvar (sym, initvalue, docstring, false);
+}
+
 DEFUN ("defconst", Fdefconst, Sdefconst, 2, UNEVALLED, 0,
        doc: /* Define SYMBOL as a constant variable.
 This declares that neither programs nor users should ever change the
@@ -863,9 +880,18 @@ DEFUN ("defconst", Fdefconst, Sdefconst, 2, UNEVALLED, 0,
 	error ("Too many arguments");
       docstring = XCAR (XCDR (XCDR (args)));
     }
+  tem = eval_sub (XCAR (XCDR (args)));
+  return Fdefconst_1 (sym, tem, docstring);
+}
 
+DEFUN ("defconst-1", Fdefconst_1, Sdefconst_1, 2, 3, 0,
+       doc: /* Like `defconst' but as a function.
+More specifically, behaves like (defconst SYM 'INITVALUE DOCSTRING).  */)
+  (Lisp_Object sym, Lisp_Object initvalue, Lisp_Object docstring)
+{
+  CHECK_SYMBOL (sym);
+  Lisp_Object tem = initvalue;
   Finternal__define_uninitialized_variable (sym, docstring);
-  tem = eval_sub (XCAR (XCDR (args)));
   if (!NILP (Vpurify_flag))
     tem = Fpurecopy (tem);
   Fset_default (sym, tem);      /* FIXME: set-default-toplevel-value? */
@@ -4333,9 +4359,11 @@ syms_of_eval (void)
   defsubr (&Sdefault_toplevel_value);
   defsubr (&Sset_default_toplevel_value);
   defsubr (&Sdefvar);
+  defsubr (&Sdefvar_1);
   defsubr (&Sdefvaralias);
   DEFSYM (Qdefvaralias, "defvaralias");
   defsubr (&Sdefconst);
+  defsubr (&Sdefconst_1);
   defsubr (&Sinternal__define_uninitialized_variable);
   defsubr (&Smake_var_non_special);
   defsubr (&Slet);

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>, Lars Ingebrigtsen <larsi@gnus.org>
> Date: Wed, 25 May 2022 16:38:45 -0400

LGTM, with a minor nit:

> * src/eval.c (defvar, Fdefvar_1, Fdefconst_1): New functions, extracted from
> `Fdef(var|const)`.
       ^^^^^^^^^^^
[...]
> (byte-compile-defvar): Simplify using the new `def(car|const)-1` functions.
                                                    ^^^^^^^^^^^
Can we please not use such "shorthands" in the log messages?  They
make it much harder to search for changes related to some symbol, and
much easier to miss some changes.  I understand the urge to type less,
but M-/ is available to alleviate that, and usually does the job for
me.

Thanks.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> (byte-compile-defvar): Simplify using the new `def(car|const)-1` functions.
>                                                     ^^^^^^^^^^^
> Can we please not use such "shorthands" in the log messages?

Fixed, and then pushed, thanks,


        Stefan