Possible defvar bug

Possible defvar bug

[Tassilo Horn]

Hi all,

I have a problem in AUCTeX where I get some void-variable error although
I require the file defining the variable.  After some experimenting,
I've isolated the problem to this minimal example:

    (let ((crm-separator ","))
      (require 'crm))

crm.el defines `crm-separator' with some default value.  However, when
evaluating that snipped, I get:

,----[ C-h v crm-separator ]
| crm-separator is a variable defined in `crm.el'.
| It is void as a variable.
| 
| Documentation:
| Separator regexp used for separating strings in `completing-read-multiple'.
| It should be a regexp that does not match the list of completion candidates.
| Modify this value to make `completing-read-multiple' use a separator other
| than `crm-default-separator'.
`----

I understand that defvar doesn't override a variable's value if the
variable is already defined, but is that TRT with dynamic bindings, too?

Bye,
Tassilo

Re: Possible defvar bug

[Jambunathan K]

The documentation of `defvar' has the following paragraphs.  I am
certain I don't understand what these paragraphs mean, particularly the
second one.  Just throwing my hat, to see where it lands.

,----  C-h f
| The optional argument INITVALUE is evaluated, and used to set SYMBOL,
| only if SYMBOL's value is void.  If SYMBOL is buffer-local, its
| default value is what is set; buffer-local values are not affected.
| If INITVALUE is missing, SYMBOL's value is not set.
| 
| If SYMBOL has a local binding, then this form affects the local
| binding.  This is usually not what you want.  Thus, if you need to
| load a file defining variables, with this form or with `defconst' or
| `defcustom', you should always load that file _outside_ any bindings
| for these variables.  (`defconst' and `defcustom' behave similarly in
| this respect.)
`----


Tassilo Horn <tsdh@gnu.org> writes:

> Hi all,
>
> I have a problem in AUCTeX where I get some void-variable error although
> I require the file defining the variable.  After some experimenting,
> I've isolated the problem to this minimal example:
>
>     (let ((crm-separator ","))
>       (require 'crm))
>
> crm.el defines `crm-separator' with some default value.  However, when
> evaluating that snipped, I get:
>
> ,----[ C-h v crm-separator ]
> | crm-separator is a variable defined in `crm.el'.
> | It is void as a variable.
> | 
> | Documentation:
> | Separator regexp used for separating strings in `completing-read-multiple'.
> | It should be a regexp that does not match the list of completion candidates.
> | Modify this value to make `completing-read-multiple' use a separator other
> | than `crm-default-separator'.
> `----
>
> I understand that defvar doesn't override a variable's value if the
> variable is already defined, but is that TRT with dynamic bindings, too?
>
> Bye,
> Tassilo
>
>

-- 

Re: Possible defvar bug

[Tassilo Horn]

Jambunathan K <kjambunathan@gmail.com> writes:

Hi Jambunathan,

> ,----  C-h f
> | The optional argument INITVALUE is evaluated, and used to set SYMBOL,
> | only if SYMBOL's value is void.  If SYMBOL is buffer-local, its
> | default value is what is set; buffer-local values are not affected.
> | If INITVALUE is missing, SYMBOL's value is not set.
> | 
> | If SYMBOL has a local binding, then this form affects the local
> | binding.  This is usually not what you want.  Thus, if you need to
> | load a file defining variables, with this form or with `defconst' or
> | `defcustom', you should always load that file _outside_ any bindings
> | for these variables.  (`defconst' and `defcustom' behave similarly in
> | this respect.)
> `----

Oh, I'd say the "Thus, ..." sentence makes it pretty clear that I
shouldn't be doing what I've done.  Thanks for the hint.

I've fixed the problem now in AUCTeX.

Bye,
Tassilo

Re: Possible defvar bug

[Didier Verna]

Jambunathan K <kjambunathan@gmail.com> wrote:

> I am certain I don't understand what these paragraphs mean,
> particularly the second one.
> lands.

> ,----  C-h f
> | 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.)
> `----

  This means that:

ELISP> (let ((foo 3))
	 (defvar foo 1)
	 (print foo))
3
ELISP> foo
*** Eval error ***  Symbol's value as variable is void: foo
ELISP> 


And, no, this is almost never what you want :-) Just like Tassilo
noticed by accident.

-- 
Resistance is futile. You will be jazzimilated.

Scientific site:   http://www.lrde.epita.fr/~didier
Music (Jazz) site: http://www.didierverna.com

Re: Possible defvar bug

[Alan Mackenzie]

On Mon, Feb 18, 2013 at 04:00:28PM +0100, Didier Verna wrote:
> Jambunathan K <kjambunathan@gmail.com> wrote:

> > I am certain I don't understand what these paragraphs mean,
> > particularly the second one.
> > lands.

> > ,----  C-h f
> > | 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.)
> > `----

>   This means that:

> ELISP> (let ((foo 3))
> 	 (defvar foo 1)
> 	 (print foo))
> 3
> ELISP> foo
> *** Eval error ***  Symbol's value as variable is void: foo
> ELISP> 


> And, no, this is almost never what you want :-) Just like Tassilo
> noticed by accident.

Why would anybody want to defvar a variable inside a let binding which
also defines it, anyway?  It may not be totally invalid, but it's
certainly in breach of good taste.  I would argue that the current
behaviour _is_ what is wanted, since it alerts one to a probable error.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Possible defvar bug

[Thierry Volpiatto]

Alan Mackenzie <acm@muc.de> writes:

> On Mon, Feb 18, 2013 at 04:00:28PM +0100, Didier Verna wrote:
>> Jambunathan K <kjambunathan@gmail.com> wrote:
>
>> > I am certain I don't understand what these paragraphs mean,
>> > particularly the second one.
>> > lands.
>
>> > ,----  C-h f
>> > | 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.)
>> > `----
>
>>   This means that:
>
>> ELISP> (let ((foo 3))
>> 	 (defvar foo 1)
>> 	 (print foo))
>> 3
>> ELISP> foo
>> *** Eval error ***  Symbol's value as variable is void: foo
>> ELISP> 
>
>
>> And, no, this is almost never what you want :-) Just like Tassilo
>> noticed by accident.
>
> Why would anybody want to defvar a variable inside a let binding which
> also defines it, anyway?  It may not be totally invalid, but it's
> certainly in breach of good taste.  I would argue that the current
> behaviour _is_ what is wanted, since it alerts one to a probable error.

See also documentation of `require' which should not be used inside
`let' bindings for same reasons.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

RE: Possible defvar bug

[Drew Adams]

> See also documentation of `require' which should not be used inside
> `let' bindings for same reasons.

Hm.  What doc of `require' mentions this?  I looked at the doc string and
(elisp) `Named Features', but I didn't notice anything about this.

I hope that the doc for this, wherever it might be, is more specific than what
you said, because I cannot imagine that such a blanket statement is correct.
Presumably, any potential problem would depend on what variables the `let'
binds?

Re: Possible defvar bug

[Thierry Volpiatto]

"Drew Adams" <drew.adams@oracle.com> writes:

>> See also documentation of `require' which should not be used inside
>> `let' bindings for same reasons.
>
> Hm.  What doc of `require' mentions this?

,----
| The expression `(require 'comint)' loads the file `comint.el' if it has
| not yet been loaded, ensuring that `comint-dynamic-complete-filename'
| is defined.  Features are normally named after the files that provide
| them, so that `require' need not be given the file name.  (Note that it
| is important that the `require' statement be outside the body of the
| `let'.  Loading a library while its variables are let-bound can have
| unintended consequences, namely the variables becoming unbound after
| the let exits.)
`----


> I looked at the doc string and (elisp) `Named Features', but I didn't
> notice anything about this.
>
> I hope that the doc for this, wherever it might be, is more specific than what
> you said, because I cannot imagine that such a blanket statement is correct.
> Presumably, any potential problem would depend on what variables the `let'
> binds?
Of course.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

RE: Possible defvar bug

[Drew Adams]

> >> See also documentation of `require' which should not be used inside
> >> `let' bindings for same reasons.
> >
> > Hm.  What doc of `require' mentions this?
> 
> ,---- 
> | (Note that it
> | is important that the `require' statement be outside the body of the
> | `let'.  Loading a library while its variables are let-bound can have
> | unintended consequences, namely the variables becoming unbound after
> | the let exits.)
> `----

Right, thanks - read right past that without noticing, somehow.

> > I hope that the doc for this, wherever it might be, is more specific
> > than what you said, because I cannot imagine that such a blanket
> > statement is correct.  Presumably, any potential problem would depend
> > on what variables the `let' binds?
> Of course.

And the passage is clear enough about that: "the `let'" and "its variables"
refer to a specific `let' that binds variables defined in the required file.
That's the point.

It's worth being careful about this - a general statement that "`require' should
not be used inside `let' bindings" is quite misleading.  You had me worried
there, for a moment.  Thanks for the doc reference.

Re: Possible defvar bug

[Lawrence Mitchell]

Drew Adams wrote:
>> See also documentation of `require' which should not be used inside
>> `let' bindings for same reasons.

> Hm.  What doc of `require' mentions this?  I looked at the doc string and
> (elisp) `Named Features', but I didn't notice anything about this.

It's in the discussion, rather than in the documentation of the
individual functions on that page but:

| (Note that it is important that the `require' statement be
| outside the body of the `let'.  Loading a library while its
| variables are let-bound can have unintended consequences,
| namely the variables becoming unbound after the let exits.)


> I hope that the doc for this, wherever it might be, is more
> specific than what you said, because I cannot imagine that such
> a blanket statement is correct.  Presumably, any potential
> problem would depend on what variables the `let' binds?

Indeed.  I think that is what the paragraph is warning of.

Lawrence
-- 
Lawrence Mitchell <wence@gmx.li>

Re: Possible defvar bug

[Tassilo Horn]

Alan Mackenzie <acm@muc.de> writes:

>> ELISP> (let ((foo 3))
>> 	 (defvar foo 1)
>> 	 (print foo))
>> 3
>> ELISP> foo
>> *** Eval error ***  Symbol's value as variable is void: foo
>> ELISP> 
>
>> And, no, this is almost never what you want :-) Just like Tassilo
>> noticed by accident.
>
> Why would anybody want to defvar a variable inside a let binding which
> also defines it, anyway?

Yes, these minimal examples look weired.  The real code was a bit more
like

  (let* ((crm-separator ",") ...)
    ...
    (function-that-might-read-multiple-values-using-crm))

and `function-that-might-read-multiple-values-using-crm' dispatches if
one or multiple separated values are to be read, and then delegates to a
read-single-value or read-multiple-values-with-crm function.  The latter
required crm when being called.

The let-binding of `crm-separator' was just meant as a default value in
case crm was going to be used, because all LaTeX keyval-options use ","
to separate key-value pairs.

Bye,
Tassilo

Re: Possible defvar bug

[Didier Verna]

Alan Mackenzie <acm@muc.de> wrote:

> Why would anybody want to defvar a variable inside a let binding which
> also defines it, anyway?

  Because you're not responsible for all the code you use; only for the
  code you write. Consider this for instance:

| ;;; foo.el
| 
| (defvar foo-var 42)
| 
| (defun foo-init ()
|     ;; do a lot of stuff using foo-var
|   )
| 
| (foo-init)
| 
| (provide 'foo)

  BTW, I'm not saying this is good. This is actually pretty bad but I've
  seen code like this. If for some reason you're not happy with the
  initialization value of foo-var, it makes sense to write:

| (let ((foo-var 51))
|   (load "foo.el"))

and although this is not true, I can even understand why some people
would /intuitively/ think it would also change the global init value of
foo-var.


> I would argue that the current behaviour _is_ what is wanted

  Yes, I agree. I was referring to the docstring of defvar saying "this
  is not what you want", but rather in the sense that "this does not do
  what you think it does" (typically: change globally the initial value
  of a defvar'ed variable).

Common Lisp behaves that way too:

| * (let ((foo 3))
|     (declare (special foo))
|     (defvar foo 1)
|     (print foo))
| 
| 3 
| 3
| * foo
| 
| debugger invoked on a UNBOUND-VARIABLE in thread
| #<THREAD "main thread" RUNNING {AAFF761}>:
|   The variable FOO is unbound.
| 
| Type HELP for debugger help, or (SB-EXT:QUIT) to exit from SBCL.
| 
| restarts (invokable by number or by possibly-abbreviated name):
|   0: [ABORT] Exit debugger, returning to top level.
| 
| (SB-INT:SIMPLE-EVAL-IN-LEXENV FOO #<NULL-LEXENV>)
| 0] 


-- 
ELS 2013: deadline approaching!         http://www.european-lisp-symposium.org

Scientific site:   http://www.lrde.epita.fr/~didier
Music (Jazz) site: http://www.didierverna.com

Re: Possible defvar bug

[Glenn Morris]

Tassilo Horn wrote:

>     (let ((crm-separator ","))
>       (require 'crm))

This is a long-standing issue. The only solution at the moment is "don't
do that".

Re: Possible defvar bug

[Richard Stallman]

    >     (let ((crm-separator ","))
    >       (require 'crm))

    This is a long-standing issue. The only solution at the moment is "don't
    do that".

We could make it give an error.  defvar is a primitive and could
check the specpdl to see if the variable has a local binding.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call

Re: Possible defvar bug

[Stefan Monnier]

>> (let ((crm-separator ","))
>>   (require 'crm))
>     This is a long-standing issue. The only solution at the moment is "don't
>     do that".
> We could make it give an error.  defvar is a primitive and could
> check the specpdl to see if the variable has a local binding.

defvar already gives a message for it (but defcustom doesn't).


        Stefan

Re: Possible defvar bug

[Richard Stallman]

    defvar already gives a message for it (but defcustom doesn't).

1. defcustom could handle it like defvar.

2. It could be an error rather than a mere message.

3. If we want to do a bit of work, we could arrange for the error
to occur at the start of loading of the file, so the file
would not be partly loaded when the error happens.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call

Re: Possible defvar bug

[Stefan Monnier]

>     defvar already gives a message for it (but defcustom doesn't).
> 1. defcustom could handle it like defvar.

IIRC it's not easy to do.

> 2. It could be an error rather than a mere message.

Not sure if it would be better.

Another option is to change defvar so that it changes the specpdl entry
when the var is let-bound (i.e. so that the default is set when we
leave the let, as if it had been set before).


        Stefan

Re: Possible defvar bug

[Tassilo Horn]

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

> Another option is to change defvar so that it changes the specpdl
> entry when the var is let-bound (i.e. so that the default is set when
> we leave the let, as if it had been set before).

That would be fine with me if it would apply not only to `defvar' but
also to `defcustom'.

The reason for the problematic code in AUCTeX was that the loading of
crm should be deferred up to the point when it's actually used, e.g.,
when just viewing some LaTeX file there's no need to have crm loaded.
Thus, it was required only in some lower-level completion function
that's actually using it, but some of its higher-level (indirect)
callers let-bound crm-separator as they needed.

The alternative is to require crm at any caller binding `crm-separator',
or to eagerly require it top-level.  I do the latter now, and with crm
which is rather small and doesn't require anything else, that's no big
deal, but there are probably other packages where let-binding a variable
makes sense and requiring the package triggers a cascading load of other
packages.

Bye,
Tassilo

Re: Possible defvar bug

[Stefan Monnier]

> That would be fine with me if it would apply not only to `defvar' but
> also to `defcustom'.

Agreed (or at least to those defcustom where it makes sense, since
a defcustom might not even define a variable, depending on its :getter
and :setter).

> The reason for the problematic code in AUCTeX was that the loading of
> crm should be deferred up to the point when it's actually used, e.g.,
> when just viewing some LaTeX file there's no need to have crm loaded.

Agreed.  That's a general problem with autoloading functions.


        Stefan

Re: Possible defvar bug

[Richard Stallman]

    Another option is to change defvar so that it changes the specpdl entry
    when the var is let-bound (i.e. so that the default is set when we
    leave the let, as if it had been set before).

That is a good idea.  Maybe set-default is the place to do this.


-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call