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

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?

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

[Lars Ingebrigtsen]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

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

I don't think this is supported?  And it's not just with user options --
normal variables will end up being unbound if you do this (unless this
has been changed since I looked at this a decade ago).  And I think it's
documented, too?  Or am I misremembering?  Anybody?

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

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

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
> > 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.
>
> I don't think this is supported?  And it's not just with user options --
> normal variables will end up being unbound if you do this (unless this
> has been changed since I looked at this a decade ago).  And I think it's
> documented, too?  Or am I misremembering?  Anybody?

Hmm - I don't know much about the background, but wasn't
`set-default-toplevel-value' invented to make just that work?  See
commit

a104f656c8 Make defvar affect the default binding outside of any let.
Stefan Monnier <monnier@iro.umontreal.ca> Fri Aug 2 17:16:33 2013 -0400

AFAIU this bug report is a request to (1) correct some docstrings and
(2) use `set-default-toplevel-value' instead of `set-default' at more
places in custom.el to assign values, where appropriate.

Michael.

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

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> a104f656c8 Make defvar affect the default binding outside of any let.
> Stefan Monnier <monnier@iro.umontreal.ca> Fri Aug 2 17:16:33 2013 -0400

And AFAIU this commit also fixed the problem for variables that are not
user options.  Now you can create a local special variable like this:

(defvar variable)
(let ((variable ...))
  (require ...) ...)

That has the desired effect (of binding variable dynamically) but any

(defvar variable ...)

form evaluated when loading still makes the VARIABLE special and sets
the global value.  Right?

Michael.

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

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Hmm - I don't know much about the background, but wasn't
> `set-default-toplevel-value' invented to make just that work?  See
> commit
>
> a104f656c8 Make defvar affect the default binding outside of any let.
> Stefan Monnier <monnier@iro.umontreal.ca> Fri Aug 2 17:16:33 2013 -0400

Ah, newfangled code.  😀

> AFAIU this bug report is a request to (1) correct some docstrings and
> (2) use `set-default-toplevel-value' instead of `set-default' at more
> places in custom.el to assign values, where appropriate.

Right.

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

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

[Ignacio Casso]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> Hmm - I don't know much about the background, but wasn't
>> `set-default-toplevel-value' invented to make just that work?  See
>> commit
>>
>> a104f656c8 Make defvar affect the default binding outside of any let.
>> Stefan Monnier <monnier@iro.umontreal.ca> Fri Aug 2 17:16:33 2013 -0400
>
> Ah, newfangled code.  😀
>
>> AFAIU this bug report is a request to (1) correct some docstrings and
>> (2) use `set-default-toplevel-value' instead of `set-default' at more
>> places in custom.el to assign values, where appropriate.
>
> Right.

I think custom.el already uses `set-default-toplevel-value' where
appropriate by default. So my request is (1) to correct the docstrings
in custom.el to reflect so, so that users know to use it instead of
`set-default', and (2) Add some warnings somewhere, although I'm not
sure where.

I personally can not think of a single case in which someone would want
to use `set-default' instead of `set-default-toplevel-value'. If I
understand them correctly, they both do the same outside a let binding,
and I don't see why someone would want the `set-default' behavior inside
the let binding. In fact, I guess most people assume that `set-default'
behaves like `set-default-toplevel-value' (I did at least).

So I would at least talk about this in the docstrings of `set-default',
and also `default-value' and `default-boundp' which suffer the same
problem. In fact, now that I see it, the docstring of the later is just
wrong. The others just don't mention let bindings and only talk about
buffer-local bindings, but that one explicitly says that the function
can be used to know if a variable has a non-void value outside of a
let-binding, and with dynamic binding that doesn't work (see snippet
below).

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

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

[Ignacio Casso]

> I think custom.el already uses `set-default-toplevel-value' where
> appropriate by default. So my request is (1) to correct the docstrings
> in custom.el to reflect so, so that users know to use it instead of
> `set-default', and (2) Add some warnings somewhere, although I'm not
> sure where.
> 
> I personally can not think of a single case in which someone would want
> to use `set-default' instead of `set-default-toplevel-value'. If I
> understand them correctly, they both do the same outside a let binding,
> and I don't see why someone would want the `set-default' behavior inside
> the let binding. In fact, I guess most people assume that `set-default'
> behaves like `set-default-toplevel-value' (I did at least).
> 
> So I would at least talk about this in the docstrings of `set-default',
> and also `default-value' and `default-boundp' which suffer the same
> problem. In fact, now that I see it, the docstring of the later is just
> wrong. The others just don't mention let bindings and only talk about
> buffer-local bindings, but that one explicitly says that the function
> can be used to know if a variable has a non-void value outside of a
> let-binding, and with dynamic binding that doesn't work (see snippet
> below).
> 
>   (setq lexical-binding nil)
>   (let ((another-fresh-var 1))
>     (default-boundp 'another-fresh-var)) ;; I expect nil, it returns t


Hello,

I was revisiting this bug report and writing a patch to correct and
update some docstrings, both in custom.el and for `default-value',
`set-default' and `default-boundp'. But for the last three, I'm no
longer sure if the errors are in the implementation or the docstrings,
since I have found more strange cases while experimenting. In a few
words, those functions behave differently inside let bindings depending
on whether the current buffer has or not a local value for the variable,
which I find a little bit inconsistent. If it has, they behave as they
"toplevel" counterparts (`default-toplevel-value',
`set-default-toplevel-value'). If they don't, they behave as I explained
in previous emails. I describe those cases below, with code snippets and
comments. Note that the behavior also depends on whether lexical binding
is enabled or not. I use dynamic binding in these examples.

;;;; `default-value'

;; default defined, buffer-local undefined
(defvar var1 "default")
(let ((var1 "inside let")) (default-value 'var1)) ;; returns "inside let"

;; default defined, buffer-local defined
(defvar var2 "default")
(setq-local var2 "buffer-local")
(let ((var2 "inside let")) (default-value 'var2)) ;; returns "default"

;; default undefined, buffer-local undefined
(let ((var3 "inside let")) (default-value 'var3)) ;; returns "inside let"

;; default undefined, buffer-local defined
(setq-local var4 "buffer-local")
(let ((var4 "inside let")) (default-value 'var4)) ;; void-variable error


;;;; `default-boundp'

;; default defined, buffer-local undefined
(defvar var5 "default")
(let ((var5 "inside let")) (default-boundp 'var5)) ;; returns t

;; default defined, buffer-local defined
(defvar var6 "default")
(setq-local var6 "buffer-local")
(let ((var6 "inside let")) (default-boundp 'var6)) ;; returns t

;; default undefined, buffer-local undefined
(let ((var7 "inside let")) (default-boundp 'var7)) ;; returns t

;; default undefined, buffer-local defined
(setq-local var8 "buffer-local")
(let ((var8 "inside let")) (default-boundp 'var8)) ;; returns nil


;;;; `set-default'

;; default defined, buffer-local undefined
(defvar var9 "default")
(let ((var9 "inside let"))
  (set-default 'var9 "new-default")
  var9)                                          ;; returns "new-default"
var9                                             ;; returns "default"
(default-value 'var9)                            ;; returns "default"

;; default defined, buffer-local defined
(defvar var10 "default")
(setq-local var10 "buffer-local")
(let ((var10 "inside let"))
  (set-default 'var10 "new-default")
  var10)                                         ;; returns "inside let"
var10                                            ;; returns "buffer-local"
(default-value 'var10)                           ;; returns "new-default"

;; default undefined, buffer-local undefined
(let ((var11 "inside let"))
  (set-default 'var11 "new-default")
  var11)                                         ;; returns "new-default"
var11                                            ;; void-variable error
(default-value 'var11)                           ;; void-variable error

;; default undefined, buffer-local defined
(setq-local var12 "buffer-local")
(let ((var12 "inside let"))
  (set-default 'var12 "new-default")
  var12)                                         ;; returns "inside let"
var12                                            ;; returns "buffer-local"
(default-value 'var12)                           ;; returns "new-default"


Should I go ahead and just update the docstrings so that they reflect
this behavior, and then close this bug report? Or do you think that the
code should be changed too? For the later I don't think I could help,
since that code is too low level for me.

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

[Eli Zaretskii]

[Adding Stefan to the discussion.]

> From: Ignacio Casso <ignaciocasso@hotmail.com>
> Date: Tue, 12 Apr 2022 11:13:27 +0200
> 
> I was revisiting this bug report and writing a patch to correct and
> update some docstrings, both in custom.el and for `default-value',
> `set-default' and `default-boundp'. But for the last three, I'm no
> longer sure if the errors are in the implementation or the docstrings,
> since I have found more strange cases while experimenting. In a few
> words, those functions behave differently inside let bindings depending
> on whether the current buffer has or not a local value for the variable,
> which I find a little bit inconsistent. If it has, they behave as they
> "toplevel" counterparts (`default-toplevel-value',
> `set-default-toplevel-value'). If they don't, they behave as I explained
> in previous emails. I describe those cases below, with code snippets and
> comments. Note that the behavior also depends on whether lexical binding
> is enabled or not. I use dynamic binding in these examples.

Please tell more in each case where you consider the behavior
"surprising" why did you expect something different.  I think it's
important to make the subsequent discussion focused and efficient.

Thanks.

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

[Ignacio Casso]

> Please tell more in each case where you consider the behavior
> "surprising" why did you expect something different.  I think it's
> important to make the subsequent discussion focused and efficient.
>
> Thanks.

I explained why I considered some of those cases surprising by
themselves in previous emails of the thread. I originally expected
`default-value', `set-default' and `default-boundp' to behave like their
counterparts `default-toplevel-value' and
`set-default-toplevel-value'. But I assumed there was a reason for there
being two versions, so I was just going to update some docstrings to
make the distinction more clear (or in the case `default-boundp',
correct the docstring, which is just wrong).

But now, considering all the cases together, I also consider surprising
that the behavior, whichever the correct one should be, depends on
whether the variable has or not a buffer local binding. So for example,
for the following

> ;; default defined, buffer-local undefined
> (defvar var1 "default")
> (let ((var1 "inside let")) (default-value 'var1)) ;; returns "inside let"
> 
> ;; default defined, buffer-local defined
> (defvar var2 "default")
> (setq-local var2 "buffer-local")
> (let ((var2 "inside let")) (default-value 'var2)) ;; returns "default"

I would expect both snippets to return the same. And the same goes for
every two pair of snippets that only differ in whether the form
(setq-local varX "buffer-local") is present. So I was no longer sure
that the issue was just incomplete or ambiguous documentation, and I
wrote all the cases I could think in the last email to see what you
think about it.


P.S. By the way, I had deleted the previous emails on these thread, so I
could not properly reply to the last one, and had to just write to
54399@debbugs.gnu.org instead. What is the proper way to reply to an
debbugs email thread that is no longer or never was in your inbox? For
the org-mode mail list there is a link on the web archives, but not for
debbugs.gnu.org. Is there a quick way from the debbugs package? Or maybe
using some of the info in the mbox file, which can be downloaded from
the archives in debbugs.gnu.org? Thanks

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

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

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

This means you misunderstand dynamic scoping or the meaning of
"default-" in `default-boundp` (it has nothing to do with let bindings
but is only concerned about buffer-local or not).

If you disregard lexical scoping, there are kinda to dimensions to
locality of variables: there's the "let" locality and there's the
"buffer" locality.  They can be combined.  `default-boundp/set-default`
only differ from `boundp/set` on the "buffer" dimension of locality.

Lexical scoping is yet a different beast because lexical variables have
fundamentally no name, so a lexical binding of variable `foo` has no
relation to what `boundp/set` see when passed `foo` as argument.


        Stefan

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

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

> Should I go ahead and just update the docstrings so that they reflect
> this behavior, and then close this bug report?  Or do you think that the
> code should be changed too?  For the latter I don't think I could help,
> since that code is too low level for me.

If you can send a patch for the docs which addresses your concerns that
would be very helpful.


        Stefan

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

[Ignacio Casso]

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

>>   (setq lexical-binding nil)
>>   (let ((another-fresh-var 1))
>>     (default-boundp 'another-fresh-var)) ;; I expect nil, it returns t
>
> This means you misunderstand dynamic scoping or the meaning of
> "default-" in `default-boundp` (it has nothing to do with let bindings
> but is only concerned about buffer-local or not).

I know, but the docstring for default-boundp in Emacs 29 says:

"A variable may have a buffer-local or a ‘let’-bound local value.  This
function says whether the variable has a non-void value outside of the
current context"

So that docstring at least should be corrected, and either restore the
docstring in Emacs 27, that does not mention let bindings at all, or
clarify the distinction in that docstring an also in the docstrings for
`default-value' and `set-default'

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

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

>> ;; default defined, buffer-local undefined
>> (defvar var1 "default")
>> (let ((var1 "inside let")) (default-value 'var1)) ;; returns "inside let"
>> 
>> ;; default defined, buffer-local defined
>> (defvar var2 "default")
>> (setq-local var2 "buffer-local")
>> (let ((var2 "inside let")) (default-value 'var2)) ;; returns "default"
>
> I would expect both snippets to return the same.

That's because you expect the "default" in `default-value` refers to the
let-nesting dimension rather than the buffer dimension.

BTW `set-default-toplevel-value` has a running time that's proportional
to the stack depth, whereas `default-value`, like `symbol-value` is
constant-time.

> P.S. By the way, I had deleted the previous emails on these thread, so I
> could not properly reply to the last one, and had to just write to
> 54399@debbugs.gnu.org instead. What is the proper way to reply to an
> debbugs email thread that is no longer or never was in your inbox? For
> the org-mode mail list there is a link on the web archives, but not for
> debbugs.gnu.org. Is there a quick way from the debbugs package? Or maybe
> using some of the info in the mbox file, which can be downloaded from
> the archives in debbugs.gnu.org? Thanks

You can M-x gnu-emacs-bug RET 54399 RET and then you should be able to
select the message to which you want to reply.
[ The name is not `gnu-emacs-bug`, but that's the mnemonic I use, and
  the completion does the rest => `gnus-read-ephemeral-emacs-bug-group`.  ]

        Stefan

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

[Ignacio Casso]

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


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

>> Should I go ahead and just update the docstrings so that they reflect
>> this behavior, and then close this bug report?  Or do you think that the
>> code should be changed too?  For the latter I don't think I could help,
>> since that code is too low level for me.
>
> If you can send a patch for the docs which addresses your concerns that
> would be very helpful.
>
>
>         Stefan

Here is a first attempt of a patch. It updates the docstrings for those
functions and makes some other changes regarding the original
`defcustom' issue. For some places were I was not sure if a change was
necessary, I just wrote a comment to mark them and maybe discuss later.

I also would replace in other files `set-default' with
`set-default-toplevel-value', when it is used inside a lambda expression
that is passed as :set argument for `defcustom'.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch for bug#54399 --]
[-- Type: text/x-diff, Size: 7190 bytes --]

From 348f43797e3a126471681e1f5be00bc3fd34914d Mon Sep 17 00:00:00 2001
From: Ignacio <ignaciocasso@hotmail.com>
Date: Tue, 12 Apr 2022 10:48:32 +0200
Subject: [PATCH] updated documentation regarding default toplevel values of
 variables

---
 lisp/custom.el | 26 +++++++++++++-------------
 src/data.c     | 16 +++++++++++-----
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/lisp/custom.el b/lisp/custom.el
index 76c14831ca..e23ca7915a 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -68,7 +68,7 @@ custom-initialize-default
 (defun custom-initialize-set (symbol exp)
   "Initialize SYMBOL based on EXP.
 If the symbol doesn't have a default binding already,
-then set it using its `:set' function (or `set-default' if it has none).
+then set it using its `:set' function (or `set-default-toplevel-value' if it has none).
 The value is either the value in the symbol's `saved-value' property,
 if any, or the value of EXP."
   (condition-case nil
@@ -81,7 +81,7 @@ custom-initialize-set
 
 (defun custom-initialize-reset (symbol exp)
   "Initialize SYMBOL based on EXP.
-Set the symbol, using its `:set' function (or `set-default' if it has none).
+Set the symbol, using its `:set' function (or `set-default-toplevel-value' if it has none).
 The value is either the symbol's current value
  (as obtained using the `:get' function), if any,
 or the value in the symbol's `saved-value' property if any,
@@ -100,7 +100,7 @@ custom-initialize-changed
   "Initialize SYMBOL with EXP.
 Like `custom-initialize-reset', but only use the `:set' function if
 not using the standard setting.
-For the standard setting, use `set-default'."
+For the standard setting, use `set-default-toplevel-value'."
   (condition-case nil
       (let ((def (default-toplevel-value symbol)))
         (funcall (or (get symbol 'custom-set) #'set-default-toplevel-value)
@@ -114,7 +114,7 @@ custom-initialize-changed
                 symbol
                 (eval (car (get symbol 'saved-value)))))
       (t
-       (set-default symbol (eval exp)))))))
+       (set-default-toplevel-value symbol (eval exp)))))))
 
 (defvar custom-delayed-init-variables nil
   "List of variables whose initialization is pending until startup.
@@ -262,11 +262,11 @@ defcustom
 	when using the Customize user interface.  It takes two arguments,
 	the symbol to set and the value to give it.  The function should
 	not modify its value argument destructively.  The default choice
-	of function is `set-default'.
+	of function is `set-default-toplevel-value'.
 :get	VALUE should be a function to extract the value of symbol.
 	The function takes one argument, a symbol, and should return
 	the current value for that symbol.  The default choice of function
-	is `default-value'.
+	is `default-toplevel-value'.
 :require
 	VALUE should be a feature symbol.  If you save a value
 	for this option, then when your init file loads the value,
@@ -717,7 +717,7 @@ custom-set-default
   (if custom-local-buffer
       (with-current-buffer custom-local-buffer
 	(set variable value))
-    (set-default variable value)))
+    (set-default-toplevel-value variable value)))
 
 (defun custom-set-minor-mode (variable value)
   ":set function for minor mode variables.
@@ -752,7 +752,7 @@ customize-mark-to-save
 
 Return non-nil if the `saved-value' property actually changed."
   (custom-load-symbol symbol)
-  (let* ((get (or (get symbol 'custom-get) #'default-value))
+  (let* ((get (or (get symbol 'custom-get) #'default-toplevel-value))
 	 (value (funcall get symbol))
 	 (saved (get symbol 'saved-value))
 	 (standard (get symbol 'standard-value))
@@ -779,7 +779,7 @@ customize-mark-as-set
 
 Return non-nil if the `customized-value' property actually changed."
   (custom-load-symbol symbol)
-  (let* ((get (or (get symbol 'custom-get) #'default-value))
+  (let* ((get (or (get symbol 'custom-get) #'default-toplevel-value))
 	 (value (funcall get symbol))
 	 (customized (get symbol 'customized-value))
 	 (old (or (get symbol 'saved-value) (get symbol 'standard-value))))
@@ -1054,12 +1054,12 @@ custom-theme-set-variables
 		     ;; Rogue variable, set it now.
 		     (put symbol 'force-value t)
 		     (funcall set symbol (eval value)))
-		    ((default-boundp symbol)
+		    ((default-boundp symbol) ; condition-case + default-toplevel-value?
 		     ;; Something already set this, overwrite it.
 		     (funcall set symbol (eval value))))
 	    (error
 	     (message "Error setting %s: %s" symbol data)))
-	  (and (or now (default-boundp symbol))
+	  (and (or now (default-boundp symbol)) ; condition-case + default-toplevel-value?
 	       (put symbol 'variable-comment comment)))))))
 
 (defvar custom--sort-vars-table)
@@ -1608,8 +1608,8 @@ custom-theme-recalc-variable
       (setq valspec (get variable 'standard-value)))
     (if (and valspec
 	     (or (get variable 'force-value)
-		 (default-boundp variable)))
-        (funcall (or (get variable 'custom-set) #'set-default) variable
+		 (default-boundp variable))) ; (condition-case ... default-toplevel-value ...) ?
+        (funcall (or (get variable 'custom-set) #'set-default) variable ; set-default-toplevel-value?
 		 (eval (car valspec))))))
 
 (defun custom-theme-recalc-face (face)
diff --git a/src/data.c b/src/data.c
index f06b561dcc..b73a997341 100644
--- a/src/data.c
+++ b/src/data.c
@@ -1927,9 +1927,10 @@ default_value (Lisp_Object symbol)
 
 DEFUN ("default-boundp", Fdefault_boundp, Sdefault_boundp, 1, 1, 0,
        doc: /* Return t if SYMBOL has a non-void default value.
-A variable may have a buffer-local or a `let'-bound local value.  This
-function says whether the variable has a non-void value outside of the
-current context.  Also see `default-value'.  */)
+This is the value that is seen in buffers that do not have their own
+values for this variable. Let bindings may shadow this default value.
+To take them into account, use `default-toplevel-value' together with
+`condition-case' instead. */)
   (Lisp_Object symbol)
 {
   register Lisp_Object value;
@@ -1942,7 +1943,9 @@ DEFUN ("default-value", Fdefault_value, Sdefault_value, 1, 1, 0,
        doc: /* Return SYMBOL's default value.
 This is the value that is seen in buffers that do not have their own values
 for this variable.  The default value is meaningful for variables with
-local bindings in certain buffers.  */)
+local bindings in certain buffers.  Let bindings may shadow this
+default value.  To take them into account, use
+`default-toplevel-value' instead. */)
   (Lisp_Object symbol)
 {
   Lisp_Object value = default_value (symbol);
@@ -2045,7 +2048,10 @@ set_default_internal (Lisp_Object symbol, Lisp_Object value,
 DEFUN ("set-default", Fset_default, Sset_default, 2, 2, 0,
        doc: /* Set SYMBOL's default value to VALUE.  SYMBOL and VALUE are evaluated.
 The default value is seen in buffers that do not have their own values
-for this variable.  */)
+for this variable. This function may no behave as expected inside let
+bindings of SYMBOL.  To take them into account, use
+`set-default-toplevel-value' instead. */)
+
   (Lisp_Object symbol, Lisp_Object value)
 {
   set_default_internal (symbol, value, SET_INTERNAL_SET);
-- 
2.25.1

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

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

> I know, but the docstring for default-boundp in Emacs 29 says:
>
> "A variable may have a buffer-local or a ‘let’-bound local value.  This
> function says whether the variable has a non-void value outside of the
> current context"
>
> So that docstring at least should be corrected, and either restore the
> docstring in Emacs 27, that does not mention let bindings at all, or
> clarify the distinction in that docstring an also in the docstrings for
> `default-value' and `set-default'

Indeed a patch which fixes these confusions would be great,


        Stefan

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

[Ignacio Casso]

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

>>> ;; default defined, buffer-local undefined
>>> (defvar var1 "default")
>>> (let ((var1 "inside let")) (default-value 'var1)) ;; returns "inside let"
>>> 
>>> ;; default defined, buffer-local defined
>>> (defvar var2 "default")
>>> (setq-local var2 "buffer-local")
>>> (let ((var2 "inside let")) (default-value 'var2)) ;; returns "default"
>>
>> I would expect both snippets to return the same.
>
> That's because you expect the "default" in `default-value` refers to the
> let-nesting dimension rather than the buffer dimension.

No, I just expect to refer to any of those consistently, I don't mind
which one. If it's the buffer dimension, even inside let bindings, both
forms should return "default". If inside let bindings the buffer
dimension is "eclipsed", both should return "inside let". If
`default-value' inside a let binding just has undefined behavior, it
should produce an error, or at least be documented somewhere.

But right now is neither of those, and it just depends of whether the
current buffer actually has a local value for the variable, which I find
inconsistent.

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

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

>>>> ;; default defined, buffer-local undefined
>>>> (defvar var1 "default")
>>>> (let ((var1 "inside let")) (default-value 'var1)) ;; returns "inside let"
>>>> 
>>>> ;; default defined, buffer-local defined
>>>> (defvar var2 "default")
>>>> (setq-local var2 "buffer-local")
>>>> (let ((var2 "inside let")) (default-value 'var2)) ;; returns "default"
>>>
>>> I would expect both snippets to return the same.
>>
>> That's because you expect the "default" in `default-value` refers to the
>> let-nesting dimension rather than the buffer dimension.
>
> No, I just expect to refer to any of those consistently, I don't mind
> which one. If it's the buffer dimension, even inside let bindings, both
> forms should return "default".

If there's no buffer-local value and `default-value` operates in the
buffer-local dimension, why do you expect it to return a different value
from `symbol-value`?


        Stefan

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

[Eli Zaretskii]

> Date: Tue, 12 Apr 2022 15:51:17 +0200
> Cc: Michael Heerdegen <michael_heerdegen@web.de>, larsi@gnus.org,
>  54399@debbugs.gnu.org
> 
>  DEFUN ("set-default", Fset_default, Sset_default, 2, 2, 0,
>         doc: /* Set SYMBOL's default value to VALUE.  SYMBOL and VALUE are evaluated.
>  The default value is seen in buffers that do not have their own values
> -for this variable.  */)
> +for this variable. This function may no behave as expected inside let
                     ^                  ^^
Typo: should be "not".  Also, only one space between sentences.

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

[Eli Zaretskii]

> From: Ignacio Casso <ignaciocasso@hotmail.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 54399@debbugs.gnu.org,
>  larsi@gnus.org, michael_heerdegen@web.de
> Date: Tue, 12 Apr 2022 14:16:34 +0200
> 
> P.S. By the way, I had deleted the previous emails on these thread, so I
> could not properly reply to the last one, and had to just write to
> 54399@debbugs.gnu.org instead. What is the proper way to reply to an
> debbugs email thread that is no longer or never was in your inbox?

You can download each message of the bug discussion as an mbox file,
and then visit it in your email agent.

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

[Ignacio Casso]

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

>>>>> ;; default defined, buffer-local undefined
>>>>> (defvar var1 "default")
>>>>> (let ((var1 "inside let")) (default-value 'var1)) ;; returns "inside let"
>>>>> 
>>>>> ;; default defined, buffer-local defined
>>>>> (defvar var2 "default")
>>>>> (setq-local var2 "buffer-local")
>>>>> (let ((var2 "inside let")) (default-value 'var2)) ;; returns "default"
>>>>
>>>> I would expect both snippets to return the same.
>>>
>>> That's because you expect the "default" in `default-value` refers to the
>>> let-nesting dimension rather than the buffer dimension.
>>
>> No, I just expect to refer to any of those consistently, I don't mind
>> which one. If it's the buffer dimension, even inside let bindings, both
>> forms should return "default".
>
> If there's no buffer-local value and `default-value` operates in the
> buffer-local dimension, why do you expect it to return a different value
> from `symbol-value`?
>
>
>         Stefan

Probably because I don't really understand all the concepts involved
here. But I expect functions that operate on the default value of the
buffer-local dimension to behave the same way regardless of whether the
current buffer happens to have actually a local value. So if
`default-value' returns something different as `symbol-value' when there
is a buffer-local value, I expect the same to occur when there is no
buffer-local value.

But never mind, I just wanted to ensure that the current behavior was
the expected one before updating the docstring for `default-boundp', and
you already confirmed that. And this is an uncommon corner case for
which I can't think of real use cases, aside from the one of autoloading
inside a let binding of a variable a file which defines that (custom)
variable, for which we are already using `default-toplevel-value' and
`set-default-toplevel-value'. So let's leave it at that.

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

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

> Probably because I don't really understand all the concepts involved
> here. But I expect functions that operate on the default value of the
> buffer-local dimension to behave the same way regardless of whether the
> current buffer happens to have actually a local value.

You're looking at the dimensions in the wrong way.  It's like when we
split a window in two.  Some persons will say it's split vertically
(because the two new windows are stacked vertically) while others will
say it's split horizontally (because the line delimiting the two new
windows is horizontal).

When I say that `default-value` operates on the buffer-localness, I mean
that the difference `default-value` and `symbol-value` only differ with
respect to whether they consider a buffer-local value or not.

They're both stuck in the current (i.e. most deeply nested) let-binding
in either case.

IOW, the choice between `default-value` and `symbol-value` lets you walk
along the line between buffer-local and not-buffer-local, but it does
not let you walk up the stack of nested let-bindings.
Only `default-toplevel-value` lets you do that (and it only does that on
the non-buffer-local part of the space: there is nothing like
`symbol-toplevel-value` which would let you find the "top-level
buffer-local value").

> But never mind, I just wanted to ensure that the current behavior was
> the expected one

It is, yes.


        Stefan

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

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> > +for this variable. This function may no behave as expected inside let
>                      ^                  ^^
> Typo: should be "not".  Also, only one space between sentences.

And

> +values for this variable. Let bindings may shadow this default value.
                           ^^^^^^^^^^^^

"Let-bindings" (with a hyphen; appear several times).

But the more interesting question is whether the changes per se are all
correct.

Michael.

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

[Michael Heerdegen]

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

> You can M-x gnu-emacs-bug RET 54399 RET and then you should be able to
> select the message to which you want to reply.
> [ The name is not `gnu-emacs-bug`, but that's the mnemonic I use, and
>   the completion does the rest => `gnus-read-ephemeral-emacs-bug-group`.  ]

That, or the "debbugs-gnu.el" bug browser interface.  But then it's best
to use Gnus or Rmail.

If your mail client allows it you can also read the newsgroup
gnu.emacs.bug (or gmane.emacs.bug), look for your message and answer per
mail "from there".

(AFAIU, there is a "References" named mail header that controls the
associations between messages.  Gnus allows to show this (normally
hidden) header.  When your mail program allows to use given references
you copy from somewhere as part of the mail header that composed mail
should be correctly classified as an answer to that message.)

Michael.

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

[Glenn Morris]

Michael Heerdegen wrote:

> If your mail client allows it you can also read the newsgroup
> gnu.emacs.bug (or gmane.emacs.bug), look for your message and answer per
> mail "from there".

Please don't post via usenet for bug reporting or commenting on issues.
It creates various technical problems.
Please forget that gnu.emacs.bug exists.

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

[Michael Heerdegen]

Glenn Morris <rgm@gnu.org> writes:

> > If your mail client allows it you can also read the newsgroup
> > gnu.emacs.bug (or gmane.emacs.bug), look for your message and answer per
> > mail "from there".
>
> Please don't post via usenet for bug reporting or commenting on issues.
> It creates various technical problems.
> Please forget that gnu.emacs.bug exists.

Thanks for the important hint.  That's why I said "answer _per_mail_".

But with the right setup one can have a convenient workflow.  I more or
less never post via News, but reading the bug threads as Newsgroup is
just too convenient, for reasons like this one (how to respond to the
correct subthread).

If you can't ensure that you never post via usenet, then it's better to
forget gnu.emacs.bug, agreed.

Michael.

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

[Ignacio Casso]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> > +for this variable. This function may no behave as expected inside let
>>                      ^                  ^^
>> Typo: should be "not".  Also, only one space between sentences.
>

Fixed

>
>> +values for this variable. Let bindings may shadow this default value.
>                            ^^^^^^^^^^^^
>
> "Let-bindings" (with a hyphen; appear several times).
>

Fixed

> But the more interesting question is whether the changes per se are all
> correct.
>

Most things are corrections to the docstrings. I explain each change
below, as I should probably have done earlier:


>  (defun custom-initialize-set (symbol exp)
>    "Initialize SYMBOL based on EXP.
>  If the symbol doesn't have a default binding already,
> -then set it using its `:set' function (or `set-default' if it has none).
> +then set it using its `:set' function (or `set-default-toplevel-value' if it has none).
>  The value is either the value in the symbol's `saved-value' property,
>  if any, or the value of EXP."
>    (condition-case nil

docstring correction, the function used is actually `set-default-toplevel-value'


>  (defun custom-initialize-reset (symbol exp)
>    "Initialize SYMBOL based on EXP.
> -Set the symbol, using its `:set' function (or `set-default' if it has none).
> +Set the symbol, using its `:set' function (or `set-default-toplevel-value' if it has none).
>  The value is either the symbol's current value
>   (as obtained using the `:get' function), if any,
>  or the value in the symbol's `saved-value' property if any,

Same

> (defun custom-initialize-changed (symbol exp)
>    "Initialize SYMBOL with EXP.
>  Like `custom-initialize-reset', but only use the `:set' function if
>  not using the standard setting.
> -For the standard setting, use `set-default'."
> +For the standard setting, use `set-default-toplevel-value'."
>    (condition-case nil
>        (let ((def (default-toplevel-value symbol)))
>          (funcall (or (get symbol 'custom-set) #'set-default-toplevel-value)

and

>                  symbol
>                  (eval (car (get symbol 'saved-value)))))
>        (t
> -       (set-default symbol (eval exp)))))))
> +       (set-default-toplevel-value symbol (eval exp)))))))
>  

Everywhere else `set-default-toplevel-value' is used instead, so I have
changed it here too.

> @@ -262,11 +262,11 @@ defcustom
> :set	VALUE should be a function to set the value of the symbol
>  	when using the Customize user interface.  It takes two arguments,
>  	the symbol to set and the value to give it.  The function should
>  	not modify its value argument destructively.  The default choice
> -	of function is `set-default'.
> +	of function is `set-default-toplevel-value'.

Same correction

>  :get	VALUE should be a function to extract the value of symbol.
>  	The function takes one argument, a symbol, and should return
>  	the current value for that symbol.  The default choice of function
> -	is `default-value'.
> +	is `default-toplevel-value'.

Similar correction

> @@ -717,7 +717,7 @@ custom-set-default
>    (if custom-local-buffer
>        (with-current-buffer custom-local-buffer
>  	(set variable value))
> -    (set-default variable value)))
> +    (set-default-toplevel-value variable value)))

Here and in the rest of the changes in custom.el I have made or
suggested one of the following changes:

`default-value' -> `default-toplevel-value'

`set-default' -> `set-default-toplevel-value'

`default-boundp' -> (condition-case nil
                        (or (default-toplevel-value) t)
                      (void-variable nil))

But I'm not really sure of when those functions are used, so I don't
know whether they are necessary. I actually made the changes just to get
the feedback, we can discard them if you don't think that they are an
improvement or that it is worth to invest the time in finding out
whether they are an improvement.

> @@ -752,7 +752,7 @@ customize-mark-to-save
>  
>  Return non-nil if the `saved-value' property actually changed."
>    (custom-load-symbol symbol)
> -  (let* ((get (or (get symbol 'custom-get) #'default-value))
> +  (let* ((get (or (get symbol 'custom-get) #'default-toplevel-value))
>  	 (value (funcall get symbol))
>  	 (saved (get symbol 'saved-value))
>  	 (standard (get symbol 'standard-value))

See last comment


> @@ -779,7 +779,7 @@ customize-mark-as-set
>  
>  Return non-nil if the `customized-value' property actually changed."
>    (custom-load-symbol symbol)
> -  (let* ((get (or (get symbol 'custom-get) #'default-value))
> +  (let* ((get (or (get symbol 'custom-get) #'default-toplevel-value))
>  	 (value (funcall get symbol))
>  	 (customized (get symbol 'customized-value))
>  	 (old (or (get symbol 'saved-value) (get symbol 'standard-value))))

See last comment

> @@ -1054,12 +1054,12 @@ custom-theme-set-variables
>  		     ;; Rogue variable, set it now.
>  		     (put symbol 'force-value t)
>  		     (funcall set symbol (eval value)))
> -		    ((default-boundp symbol)
> +		    ((default-boundp symbol) ; condition-case + default-toplevel-value?
>  		     ;; Something already set this, overwrite it.
>  		     (funcall set symbol (eval value))))
>  	    (error
>  	     (message "Error setting %s: %s" symbol data)))
> -	  (and (or now (default-boundp symbol))
> +	  (and (or now (default-boundp symbol)) ; condition-case + default-toplevel-value?
>  	       (put symbol 'variable-comment comment)))))))

See last comment

>  
>  (defvar custom--sort-vars-table)
> @@ -1608,8 +1608,8 @@ custom-theme-recalc-variable
>        (setq valspec (get variable 'standard-value)))
>      (if (and valspec
>  	     (or (get variable 'force-value)
> -		 (default-boundp variable)))
> -        (funcall (or (get variable 'custom-set) #'set-default) variable
> +		 (default-boundp variable))) ; (condition-case ... default-toplevel-value ...) ?
> +        (funcall (or (get variable 'custom-set) #'set-default) variable ; set-default-toplevel-value?
>  		 (eval (car valspec))))))

See last comment

> diff --git a/src/data.c b/src/data.c
> index f06b561dcc..b73a997341 100644
> --- a/src/data.c
> +++ b/src/data.c
> @@ -1927,9 +1927,10 @@ default_value (Lisp_Object symbol)
>  
>  DEFUN ("default-boundp", Fdefault_boundp, Sdefault_boundp, 1, 1, 0,
>         doc: /* Return t if SYMBOL has a non-void default value.
> -A variable may have a buffer-local or a `let'-bound local value.  This
> -function says whether the variable has a non-void value outside of the
> -current context.  Also see `default-value'.  */)
> +This is the value that is seen in buffers that do not have their own
> +values for this variable. Let-bindings may shadow this default value.
> +To take them into account, use `default-toplevel-value' together with
> +`condition-case' instead. */)
>    (Lisp_Object symbol)
>  {
>    register Lisp_Object value;

Corrected `default-boundp' docstring, which said that it can be used
to check if a variable is non-void outside a let-binding context, which
is false. I just reverted to Emacs 27's version, which was correct.

Also, in this docstring and the ones for `default-value' and
`set-default', I update them to suggest using their "toplevel"
counterparts when let-bindings need to be taken into account. But I
probably use the wrong vocabulary to do so, and that information is
already on the pertinent section of the manual, so we can discard these
changes too if you don't think they belong in the docstrings.


> @@ -1942,7 +1943,9 @@ DEFUN ("default-value", Fdefault_value, Sdefault_value, 1, 1, 0,
>         doc: /* Return SYMBOL's default value.
>  This is the value that is seen in buffers that do not have their own values
>  for this variable.  The default value is meaningful for variables with
> -local bindings in certain buffers.  */)
> +local bindings in certain buffers.  Let-bindings may shadow this
> +default value.  To take them into account, use
> +`default-toplevel-value' instead. */)
>    (Lisp_Object symbol)
>  {
>    Lisp_Object value = default_value (symbol);

See last comment

> @@ -2045,7 +2048,10 @@ set_default_internal (Lisp_Object symbol, Lisp_Object value,
>  DEFUN ("set-default", Fset_default, Sset_default, 2, 2, 0,
>         doc: /* Set SYMBOL's default value to VALUE.  SYMBOL and VALUE are evaluated.
>  The default value is seen in buffers that do not have their own values
> -for this variable.  */)
> +for this variable. This function may not behave as expected inside let-bindings
> +of SYMBOL.  To take them into account, use
> +`set-default-toplevel-value' instead. */)
> +

See last comment


I've also made some new changes in customize.texi after I sent this
patch. They are equivalent to those in the docstring of
`defcustom'.

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

[Ignacio Casso]

> When I say that `default-value` operates on the buffer-localness, I mean
> that the difference `default-value` and `symbol-value` only differ with
> respect to whether they consider a buffer-local value or not.
>
> They're both stuck in the current (i.e. most deeply nested) let-binding
> in either case.
>
> IOW, the choice between `default-value` and `symbol-value` lets you walk
> along the line between buffer-local and not-buffer-local, but it does
> not let you walk up the stack of nested let-bindings.
> Only `default-toplevel-value` lets you do that (and it only does that on
> the non-buffer-local part of the space: there is nothing like
> `symbol-toplevel-value` which would let you find the "top-level
> buffer-local value").

Thanks, I get it now. I didn't know that there were different stacks of
let-bindings for a variable, one for the default value, and another one
for each buffer-local value. I though that there were only the default
value, the buffer-local value for some buffers, and a common stack of
let-bindings that shadowed all of them.

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

[Ignacio Casso]

Thanks! It seems mu4e can't visit mbox files,  but I'm trying it right 
now with Thunderbird. Let's hope it works.

On 12/4/22 17:24, Eli Zaretskii wrote:
>> From: Ignacio Casso <ignaciocasso@hotmail.com>
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 54399@debbugs.gnu.org,
>>   larsi@gnus.org, michael_heerdegen@web.de
>> Date: Tue, 12 Apr 2022 14:16:34 +0200
>>
>> P.S. By the way, I had deleted the previous emails on these thread, so I
>> could not properly reply to the last one, and had to just write to
>> 54399@debbugs.gnu.org instead. What is the proper way to reply to an
>> debbugs email thread that is no longer or never was in your inbox?
> You can download each message of the bug discussion as an mbox file,
> and then visit it in your email agent.
>
>
>

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

[Lars Ingebrigtsen]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> I've also made some new changes in customize.texi after I sent this
> patch. They are equivalent to those in the docstring of
> `defcustom'.

I didn't see any new version of the proposed patch in this bug report?

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

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

[Ignacio Casso]

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


>> I've also made some new changes in customize.texi after I sent this
>> patch. They are equivalent to those in the docstring of
>> `defcustom'.
>
> I didn't see any new version of the proposed patch in this bug report?

Sorry, I was waiting for more feedback in other parts of the patch
before sending it. I send a new version now with the typos you pointed
out fixed, and the changes in customize.texi that I said. I've also
dropped some of the changes in the previous path that I don't think that
were actually necessary, to keep the patch and review simpler.

--Ignacio


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch 54399 --]
[-- Type: text/x-diff, Size: 6727 bytes --]

From 9ebf1065c2970f688fdcafd6278ae5e239265065 Mon Sep 17 00:00:00 2001
From: Ignacio Casso <ignaciocasso@hotmail.com>
Date: Thu, 12 May 2022 08:41:44 +0200
Subject: [PATCH] updated some documentation regarding customize and default
 values

---
 doc/lispref/customize.texi |  8 ++++----
 lisp/custom.el             | 18 +++++++++---------
 src/data.c                 |  4 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/doc/lispref/customize.texi b/doc/lispref/customize.texi
index 54059d7b6e..06a2f5365d 100644
--- a/doc/lispref/customize.texi
+++ b/doc/lispref/customize.texi
@@ -376,7 +376,7 @@ Variable Definitions
 the value properly for this option (which may not mean simply setting
 the option as a Lisp variable); preferably, though, it should not
 modify its value argument destructively.  The default for
-@var{setfunction} is @code{set-default}.
+@var{setfunction} is @code{set-default-toplevel-value}.
 
 If you specify this keyword, the variable's documentation string
 should describe how to do the same job in hand-written Lisp code.
@@ -387,7 +387,7 @@ Variable Definitions
 option.  The function @var{getfunction} should take one argument, a
 symbol, and should return whatever customize should use as the
 current value for that symbol (which need not be the symbol's Lisp
-value).  The default is @code{default-value}.
+value).  The default is @code{default-toplevel-value}.
 
 You have to really understand the workings of Custom to use
 @code{:get} correctly.  It is meant for values that are treated in
@@ -409,7 +409,7 @@ Variable Definitions
 
 @item custom-initialize-default
 Like @code{custom-initialize-set}, but use the function
-@code{set-default} to set the variable, instead of the variable's
+@code{set-default-toplevel-value} to set the variable, instead of the variable's
 @code{:set} function.  This is the usual choice for a variable whose
 @code{:set} function enables or disables a minor mode; with this choice,
 defining the variable will not call the minor mode function, but
@@ -424,7 +424,7 @@ Variable Definitions
 @item custom-initialize-changed
 Use the @code{:set} function to initialize the variable, if it is
 already set or has been customized; otherwise, just use
-@code{set-default}.
+@code{set-default-toplevel-value}.
 
 @item custom-initialize-delay
 This function behaves like @code{custom-initialize-set}, but it
diff --git a/lisp/custom.el b/lisp/custom.el
index 76c14831ca..2ab7c69d00 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -68,7 +68,7 @@ custom-initialize-default
 (defun custom-initialize-set (symbol exp)
   "Initialize SYMBOL based on EXP.
 If the symbol doesn't have a default binding already,
-then set it using its `:set' function (or `set-default' if it has none).
+then set it using its `:set' function (or `set-default-toplevel-value' if it has none).
 The value is either the value in the symbol's `saved-value' property,
 if any, or the value of EXP."
   (condition-case nil
@@ -81,7 +81,7 @@ custom-initialize-set
 
 (defun custom-initialize-reset (symbol exp)
   "Initialize SYMBOL based on EXP.
-Set the symbol, using its `:set' function (or `set-default' if it has none).
+Set the symbol, using its `:set' function (or `set-default-toplevel-value' if it has none).
 The value is either the symbol's current value
  (as obtained using the `:get' function), if any,
 or the value in the symbol's `saved-value' property if any,
@@ -100,7 +100,7 @@ custom-initialize-changed
   "Initialize SYMBOL with EXP.
 Like `custom-initialize-reset', but only use the `:set' function if
 not using the standard setting.
-For the standard setting, use `set-default'."
+For the standard setting, use `set-default-toplevel-value'."
   (condition-case nil
       (let ((def (default-toplevel-value symbol)))
         (funcall (or (get symbol 'custom-set) #'set-default-toplevel-value)
@@ -114,7 +114,7 @@ custom-initialize-changed
                 symbol
                 (eval (car (get symbol 'saved-value)))))
       (t
-       (set-default symbol (eval exp)))))))
+       (set-default-toplevel-value symbol (eval exp)))))))
 
 (defvar custom-delayed-init-variables nil
   "List of variables whose initialization is pending until startup.
@@ -262,11 +262,11 @@ defcustom
 	when using the Customize user interface.  It takes two arguments,
 	the symbol to set and the value to give it.  The function should
 	not modify its value argument destructively.  The default choice
-	of function is `set-default'.
+	of function is `set-default-toplevel-value'.
 :get	VALUE should be a function to extract the value of symbol.
 	The function takes one argument, a symbol, and should return
 	the current value for that symbol.  The default choice of function
-	is `default-value'.
+	is `default-toplevel-value'.
 :require
 	VALUE should be a feature symbol.  If you save a value
 	for this option, then when your init file loads the value,
@@ -717,7 +717,7 @@ custom-set-default
   (if custom-local-buffer
       (with-current-buffer custom-local-buffer
 	(set variable value))
-    (set-default variable value)))
+    (set-default-toplevel-value variable value)))
 
 (defun custom-set-minor-mode (variable value)
   ":set function for minor mode variables.
@@ -752,7 +752,7 @@ customize-mark-to-save
 
 Return non-nil if the `saved-value' property actually changed."
   (custom-load-symbol symbol)
-  (let* ((get (or (get symbol 'custom-get) #'default-value))
+  (let* ((get (or (get symbol 'custom-get) #'default-toplevel-value))
 	 (value (funcall get symbol))
 	 (saved (get symbol 'saved-value))
 	 (standard (get symbol 'standard-value))
@@ -779,7 +779,7 @@ customize-mark-as-set
 
 Return non-nil if the `customized-value' property actually changed."
   (custom-load-symbol symbol)
-  (let* ((get (or (get symbol 'custom-get) #'default-value))
+  (let* ((get (or (get symbol 'custom-get) #'default-toplevel-value))
 	 (value (funcall get symbol))
 	 (customized (get symbol 'customized-value))
 	 (old (or (get symbol 'saved-value) (get symbol 'standard-value))))
diff --git a/src/data.c b/src/data.c
index 72dcf6f878..9b36ecc1b2 100644
--- a/src/data.c
+++ b/src/data.c
@@ -1939,9 +1939,9 @@ default_value (Lisp_Object symbol)
 
 DEFUN ("default-boundp", Fdefault_boundp, Sdefault_boundp, 1, 1, 0,
        doc: /* Return t if SYMBOL has a non-void default value.
-A variable may have a buffer-local or a `let'-bound local value.  This
+A variable may have a buffer-local value.  This
 function says whether the variable has a non-void value outside of the
-current context.  Also see `default-value'.  */)
+current buffer context.  Also see `default-value'.  */)
   (Lisp_Object symbol)
 {
   register Lisp_Object value;
-- 
2.25.1

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

[Eli Zaretskii]

> From: Ignacio Casso <ignaciocasso@hotmail.com>
> Cc: Michael Heerdegen <michael_heerdegen@web.de>, Eli Zaretskii
>  <eliz@gnu.org>, 54399@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Thu, 12 May 2022 08:58:09 +0200
> 
> >> I've also made some new changes in customize.texi after I sent this
> >> patch. They are equivalent to those in the docstring of
> >> `defcustom'.
> >
> > I didn't see any new version of the proposed patch in this bug report?
> 
> Sorry, I was waiting for more feedback in other parts of the patch
> before sending it. I send a new version now with the typos you pointed
> out fixed, and the changes in customize.texi that I said. I've also
> dropped some of the changes in the previous path that I don't think that
> were actually necessary, to keep the patch and review simpler.

This doesn't just change documentation, it also changes some code.

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

[Ignacio Casso]

>> >> I've also made some new changes in customize.texi after I sent this
>> >> patch. They are equivalent to those in the docstring of
>> >> `defcustom'.
>> >
>> > I didn't see any new version of the proposed patch in this bug report?
>> 
>> Sorry, I was waiting for more feedback in other parts of the patch
>> before sending it. I send a new version now with the typos you pointed
>> out fixed, and the changes in customize.texi that I said. I've also
>> dropped some of the changes in the previous path that I don't think that
>> were actually necessary, to keep the patch and review simpler.
>
> This doesn't just change documentation, it also changes some code.

Do you say it because of the commit message? I was waiting until you
agreed on the patch to write a proper one following the guidelines.

Or if you want a patch with only documentation changes, I can
update it, but the changes are just using the toplevel versions in a
few places that were missing.

--Ignacio

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

[Eli Zaretskii]

> From: Ignacio Casso <ignaciocasso@hotmail.com>
> Cc: larsi@gnus.org, michael_heerdegen@web.de, 54399@debbugs.gnu.org,
>  monnier@iro.umontreal.ca
> Date: Thu, 12 May 2022 10:14:13 +0200
> 
> > This doesn't just change documentation, it also changes some code.
> 
> Do you say it because of the commit message?

No, because of this:

> -  (let* ((get (or (get symbol 'custom-get) #'default-value))
> +  (let* ((get (or (get symbol 'custom-get) #'default-toplevel-value))

and this:

> @@ -114,7 +114,7 @@ custom-initialize-changed
>                  symbol
>                  (eval (car (get symbol 'saved-value)))))
>        (t
> -       (set-default symbol (eval exp)))))))
> +       (set-default-toplevel-value symbol (eval exp)))))))
 

and several similar changes.

> I was waiting until you agreed on the patch to write a proper one
> following the guidelines.
> 
> Or if you want a patch with only documentation changes, I can
> update it, but the changes are just using the toplevel versions in a
> few places that were missing.

My point is that you seemed to be sating that you suggest a
documentation change, but the changeset actually changes some code.
So I'm not sure anymore what is this changeset about.  Perhaps I
forgot what we were discussing at the beginning, since that was quite
some time ago.  Can you remind?

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

[Ignacio Casso]

> My point is that you seemed to be sating that you suggest a
> documentation change, but the changeset actually changes some code.
> So I'm not sure anymore what is this changeset about.  Perhaps I
> forgot what we were discussing at the beginning, since that was quite
> some time ago.  Can you remind?

Sure. It began as a bug report, but after you explained things to me,
all that was left were some mismatches between the docstrings of some
functions and their actual behavior: they use `default-toplevel-value'
and `set-default-toplevel-value' and their docstring said they use
`default-value' and `set-default'. And that is mainly what this patch
fixes, aside from another error in the docstring of
`default-boundp'.

The few non-documentation changes are just using the toplevel versions
of those functions in some places that were still missing, in functions
and places completely equivalent to those that are already using
them. This makes the use of those functions more consistent across
custom.el, and fixes in those new places the potential bug that those
functions were introduced to solve in the first place: calling
`defcustom' or customize functions inside a let-binding of the variable
in question (e.g., because a function inside the let body autoloads,
calling `defcustom')

A few further comments on each change:

> @@ -114,7 +114,7 @@ custom-initialize-changed
>                  symbol
>                  (eval (car (get symbol 'saved-value)))))
>        (t
> -       (set-default symbol (eval exp)))))))
> +       (set-default-toplevel-value symbol (eval exp)))))))
>  
>  (defvar custom-delayed-init-variables nil
>    "List of variables whose initialization is pending until startup.

The docstring of this function actually says that it behaves like
`custom-initialize-reset', which is already using
`set-default-toplevel-value'.


> @@ -717,7 +717,7 @@ custom-set-default
>    (if custom-local-buffer
>        (with-current-buffer custom-local-buffer
>  	(set variable value))
> -    (set-default variable value)))
> +    (set-default-toplevel-value variable value)))
>  
>  (defun custom-set-minor-mode (variable value)
>    ":set function for minor mode variables.

This would be the only setter function that does not use
`set-default-toplevel-value'.

> @@ -752,7 +752,7 @@ customize-mark-to-save
> @@ -779,7 +779,7 @@ customize-mark-as-set

These two I thought that I had remove them. Ignore them, they will not
be there in the next version of the patch

--Ignacio

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

[Eli Zaretskii]

> From: Ignacio Casso <ignaciocasso@hotmail.com>
> Cc: larsi@gnus.org, michael_heerdegen@web.de, 54399@debbugs.gnu.org,
>  monnier@iro.umontreal.ca
> Date: Thu, 12 May 2022 10:41:56 +0200
> 
> The few non-documentation changes are just using the toplevel versions
> of those functions in some places that were still missing, in functions
> and places completely equivalent to those that are already using
> them. This makes the use of those functions more consistent across
> custom.el, and fixes in those new places the potential bug that those
> functions were introduced to solve in the first place: calling
> `defcustom' or customize functions inside a let-binding of the variable
> in question (e.g., because a function inside the let body autoloads,
> calling `defcustom')

If those changes are fine with Stefan and Lars, I don't mind.

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

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> If those changes are fine with Stefan and Lars, I don't mind.

They make sense to me, but perhaps Stefan has some comments...

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

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

[Lars Ingebrigtsen]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> These two I thought that I had remove them. Ignore them, they will not
> be there in the next version of the patch

Ignacio, did you have a new version of the patch that we missed?

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

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

[Ignacio Casso]

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


Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
>> These two I thought that I had remove them. Ignore them, they will not
>> be there in the next version of the patch
>
> Ignacio, did you have a new version of the patch that we missed?

Not really, just the last one but without those two changes. I guess I was
waiting for all of you to agree before sending it (you and Eli did, but
Stefan did not answer) and I forgot eventually. Here it is:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch for bug#54399 --]
[-- Type: text/x-diff, Size: 5910 bytes --]

From 6f0791ce7d5189af29362e8570659881776d7748 Mon Sep 17 00:00:00 2001
From: Ignacio Casso <ignaciocasso@hotmail.com>
Date: Thu, 12 May 2022 10:37:10 +0200
Subject: [PATCH] updated some documentation regarding customize and default
 values

---
 doc/lispref/customize.texi |  8 ++++----
 lisp/custom.el             | 14 +++++++-------
 src/data.c                 |  4 ++--
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/doc/lispref/customize.texi b/doc/lispref/customize.texi
index 54059d7b6e..06a2f5365d 100644
--- a/doc/lispref/customize.texi
+++ b/doc/lispref/customize.texi
@@ -376,7 +376,7 @@ Variable Definitions
 the value properly for this option (which may not mean simply setting
 the option as a Lisp variable); preferably, though, it should not
 modify its value argument destructively.  The default for
-@var{setfunction} is @code{set-default}.
+@var{setfunction} is @code{set-default-toplevel-value}.
 
 If you specify this keyword, the variable's documentation string
 should describe how to do the same job in hand-written Lisp code.
@@ -387,7 +387,7 @@ Variable Definitions
 option.  The function @var{getfunction} should take one argument, a
 symbol, and should return whatever customize should use as the
 current value for that symbol (which need not be the symbol's Lisp
-value).  The default is @code{default-value}.
+value).  The default is @code{default-toplevel-value}.
 
 You have to really understand the workings of Custom to use
 @code{:get} correctly.  It is meant for values that are treated in
@@ -409,7 +409,7 @@ Variable Definitions
 
 @item custom-initialize-default
 Like @code{custom-initialize-set}, but use the function
-@code{set-default} to set the variable, instead of the variable's
+@code{set-default-toplevel-value} to set the variable, instead of the variable's
 @code{:set} function.  This is the usual choice for a variable whose
 @code{:set} function enables or disables a minor mode; with this choice,
 defining the variable will not call the minor mode function, but
@@ -424,7 +424,7 @@ Variable Definitions
 @item custom-initialize-changed
 Use the @code{:set} function to initialize the variable, if it is
 already set or has been customized; otherwise, just use
-@code{set-default}.
+@code{set-default-toplevel-value}.
 
 @item custom-initialize-delay
 This function behaves like @code{custom-initialize-set}, but it
diff --git a/lisp/custom.el b/lisp/custom.el
index a084304ff8..84c740f73b 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -68,7 +68,7 @@ custom-initialize-default
 (defun custom-initialize-set (symbol exp)
   "Initialize SYMBOL based on EXP.
 If the symbol doesn't have a default binding already,
-then set it using its `:set' function (or `set-default' if it has none).
+then set it using its `:set' function (or `set-default-toplevel-value' if it has none).
 The value is either the value in the symbol's `saved-value' property,
 if any, or the value of EXP."
   (condition-case nil
@@ -81,7 +81,7 @@ custom-initialize-set
 
 (defun custom-initialize-reset (symbol exp)
   "Initialize SYMBOL based on EXP.
-Set the symbol, using its `:set' function (or `set-default' if it has none).
+Set the symbol, using its `:set' function (or `set-default-toplevel-value' if it has none).
 The value is either the symbol's current value
  (as obtained using the `:get' function), if any,
 or the value in the symbol's `saved-value' property if any,
@@ -100,7 +100,7 @@ custom-initialize-changed
   "Initialize SYMBOL with EXP.
 Like `custom-initialize-reset', but only use the `:set' function if
 not using the standard setting.
-For the standard setting, use `set-default'."
+For the standard setting, use `set-default-toplevel-value'."
   (condition-case nil
       (let ((def (default-toplevel-value symbol)))
         (funcall (or (get symbol 'custom-set) #'set-default-toplevel-value)
@@ -114,7 +114,7 @@ custom-initialize-changed
                 symbol
                 (eval (car (get symbol 'saved-value)))))
       (t
-       (set-default symbol (eval exp)))))))
+       (set-default-toplevel-value symbol (eval exp)))))))
 
 (defvar custom-delayed-init-variables nil
   "List of variables whose initialization is pending until startup.
@@ -262,11 +262,11 @@ defcustom
 	when using the Customize user interface.  It takes two arguments,
 	the symbol to set and the value to give it.  The function should
 	not modify its value argument destructively.  The default choice
-	of function is `set-default'.
+	of function is `set-default-toplevel-value'.
 :get	VALUE should be a function to extract the value of symbol.
 	The function takes one argument, a symbol, and should return
 	the current value for that symbol.  The default choice of function
-	is `default-value'.
+	is `default-toplevel-value'.
 :require
 	VALUE should be a feature symbol.  If you save a value
 	for this option, then when your init file loads the value,
@@ -717,7 +717,7 @@ custom-set-default
   (if custom-local-buffer
       (with-current-buffer custom-local-buffer
 	(set variable value))
-    (set-default variable value)))
+    (set-default-toplevel-value variable value)))
 
 (defun custom-set-minor-mode (variable value)
   ":set function for minor mode variables.
diff --git a/src/data.c b/src/data.c
index 72dcf6f878..9b36ecc1b2 100644
--- a/src/data.c
+++ b/src/data.c
@@ -1939,9 +1939,9 @@ default_value (Lisp_Object symbol)
 
 DEFUN ("default-boundp", Fdefault_boundp, Sdefault_boundp, 1, 1, 0,
        doc: /* Return t if SYMBOL has a non-void default value.
-A variable may have a buffer-local or a `let'-bound local value.  This
+A variable may have a buffer-local value.  This
 function says whether the variable has a non-void value outside of the
-current context.  Also see `default-value'.  */)
+current buffer context.  Also see `default-value'.  */)
   (Lisp_Object symbol)
 {
   register Lisp_Object value;
-- 
2.25.1

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

[Lars Ingebrigtsen]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> Not really, just the last one but without those two changes. I guess I was
> waiting for all of you to agree before sending it (you and Eli did, but
> Stefan did not answer) and I forgot eventually. Here it is:

Thanks; pushed to Emacs 29 (with some whitespace changes -- a couple of
the doc strings were too wide).

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

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

[dick]

This may fall under "If it hurts, don't do it?", but things like:

(let (parens-require-spaces defun-prompt-regexp)
  (require 'cl)
  (custom-set-variables
   '(parens-require-spaces t)
   '(defun-prompt-regexp (cl-assert parens-require-spaces))))

used to work before commit 071722e.

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

[Lars Ingebrigtsen]

dick <dick.r.chiang@gmail.com> writes:

> This may fall under "If it hurts, don't do it?", but things like:
>
> (let (parens-require-spaces defun-prompt-regexp)
>   (require 'cl)
>   (custom-set-variables
>    '(parens-require-spaces t)
>    '(defun-prompt-regexp (cl-assert parens-require-spaces))))
>
> used to work before commit 071722e.

Hm, yes.  That should be fixed, I think.  Should we call both
set-default-toplevel-value and set-default here?

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