bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Eli Zaretskii
Subject: bug#55156: [PATCH] eval.c: New functions `defvar-f` and `defconst-f`
Date: Thu, 28 Apr 2022 08:34:23 +0300

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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]