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" <bug-gnu-emacs@gnu.org>]

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);