GNU bug report logs - #44733
Nested let bindings for non-local DEFVAR_PER_BUFFER variables unwind wrong

Previous Next

Package: emacs;

Reported by: Spencer Baugh <sbaugh <at> catern.com>

Date: Thu, 19 Nov 2020 03:12:02 UTC

Severity: normal

Fixed in version 28.1

Done: Stefan Kangas <stefan <at> marxist.se>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 44733 in the body.
You can then email your comments to 44733 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#44733; Package emacs. (Thu, 19 Nov 2020 03:12:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Spencer Baugh <sbaugh <at> catern.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 19 Nov 2020 03:12:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> catern.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Nested let bindings for non-local DEFVAR_PER_BUFFER variables
 unwind wrong
Date: Wed, 18 Nov 2020 22:11:00 -0500
The problem is with variables defined in C by DEFVAR_PER_BUFFER.  These
are Lisp variables with special Lisp_Object slots in struct buffer.

These variables can be let-bound.  When these variable are let-bound
when the variable is not buffer-local in the current buffer, the default
value for the variable is changed (which affects all buffers which don't
have a buffer-local value for the variable). In the C code, this is a
SPECPDL_LET_DEFAULT binding.

If a DEFVAR_PER_BUFFER variable is set with setq inside such a
SPECPDL_LET_DEFAULT binding, the resulting situation is somewhat
unusual: The variable is set to the specified value only for the current
buffer, and other buffers keep their old values for the variable, but
the variable does not become buffer-local - e.g. local-variable-p
returns nil. This situation is unusual and undocumented, but not
necessarily buggy. This is somewhat normal.

However, more buggy is what happens when these let bindings are nested.
If we do first SPECPDL_LET_DEFAULT, then setq, then a second nested
SPECPDL_LET_DEFAULT, when the second nested let binding is unwound, the
default value for variable is set to the pseudo-buffer-local value that
was active in (current-buffer) when the nested let was entered.

See the below code example (left-margin is chosen as an arbitrary
DEFVAR_PER_BUFFER variable):

(let ((left-margin 1))
  ;; Set this variable "pseudo-locally", inside a SPECPDL_LET_DEFAULT binding.
  (setq left-margin 123)
  (assert (eq left-margin 123))
  ;; Note, it's not a local variable.
  (assert (not (local-variable-p 'left-margin)))
  ;; The default value doesn't change.
  (assert (eq (default-value 'left-margin) 1))
  (with-temp-buffer (assert (eq left-margin 1)))
  ;; Perform a seemingly unrelated do-nothing let-binding of left-margin.
  (let ((left-margin 2)))
  ;; !! The default value of left-margin has changed to 123.
  (assert (eq (default-value 'left-margin) 123))
  (with-temp-buffer (assert (eq left-margin 123)))
  ;; Emacs used (current-buffer)'s value for left-margin, 123, instead of
  ;; the actual default value, 1, when storing the old value for left-margin.
  ;; So when it unwound the let, it set the default value to 123!
)

This seems unexpected.

I ran into this while working on a patch-set to optimize
DEFVAR_PER_BUFFER.  This current unwinding behavior is pretty clearly a
bug in C, so maybe we don't need to preserve it, which hopefully might
allow for an easier implementation of an optimized DEFVAR_PER_BUFFER, if
behavior will change anyway.  (Although I can't say yet exactly what
might be the best change...)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#44733; Package emacs. (Thu, 19 Nov 2020 14:11:02 GMT) Full text and rfc822 format available.

Message #8 received at 44733 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> catern.com>
Cc: 44733 <at> debbugs.gnu.org
Subject: Re: bug#44733: Nested let bindings for non-local DEFVAR_PER_BUFFER
 variables unwind wrong
Date: Thu, 19 Nov 2020 16:10:27 +0200
> From: Spencer Baugh <sbaugh <at> catern.com>
> Date: Wed, 18 Nov 2020 22:11:00 -0500
> 
> However, more buggy is what happens when these let bindings are nested.
> If we do first SPECPDL_LET_DEFAULT, then setq, then a second nested
> SPECPDL_LET_DEFAULT, when the second nested let binding is unwound, the
> default value for variable is set to the pseudo-buffer-local value that
> was active in (current-buffer) when the nested let was entered.
> 
> See the below code example (left-margin is chosen as an arbitrary
> DEFVAR_PER_BUFFER variable):
> 
> (let ((left-margin 1))
>   ;; Set this variable "pseudo-locally", inside a SPECPDL_LET_DEFAULT binding.
>   (setq left-margin 123)
>   (assert (eq left-margin 123))
>   ;; Note, it's not a local variable.
>   (assert (not (local-variable-p 'left-margin)))
>   ;; The default value doesn't change.
>   (assert (eq (default-value 'left-margin) 1))
>   (with-temp-buffer (assert (eq left-margin 1)))
>   ;; Perform a seemingly unrelated do-nothing let-binding of left-margin.
>   (let ((left-margin 2)))
>   ;; !! The default value of left-margin has changed to 123.
>   (assert (eq (default-value 'left-margin) 123))
>   (with-temp-buffer (assert (eq left-margin 123)))
>   ;; Emacs used (current-buffer)'s value for left-margin, 123, instead of
>   ;; the actual default value, 1, when storing the old value for left-margin.
>   ;; So when it unwound the let, it set the default value to 123!
> )
> 
> This seems unexpected.

Why did you think this is a bug?  The ELisp manual seems to document
what you see:

     A variable can have more than one local binding at a time (e.g., if
  there are nested ‘let’ forms that bind the variable).  The “current
  binding” is the local binding that is actually in effect.  It determines
  the value returned by evaluating the variable symbol, and it is the
  binding acted on by ‘setq’.

Or did I misunderstand what you found unexpected?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#44733; Package emacs. (Thu, 19 Nov 2020 14:23:02 GMT) Full text and rfc822 format available.

Message #11 received at 44733 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> catern.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 44733 <at> debbugs.gnu.org
Subject: Re: bug#44733: Nested let bindings for non-local DEFVAR_PER_BUFFER
 variables unwind wrong
Date: Thu, 19 Nov 2020 09:22:07 -0500
Eli Zaretskii <eliz <at> gnu.org> writes:
> Why did you think this is a bug?  The ELisp manual seems to document
> what you see:
>
>      A variable can have more than one local binding at a time (e.g., if
>   there are nested ‘let’ forms that bind the variable).  The “current
>   binding” is the local binding that is actually in effect.  It determines
>   the value returned by evaluating the variable symbol, and it is the
>   binding acted on by ‘setq’.
>
> Or did I misunderstand what you found unexpected?

I mentioned two unexpected things, but I don't think that parapgrah
describes either of them.

First was the behavior that when you setq within a let_default binding,
the binding does not appear local when queried with local-variable-p.
That's relatively minor.  It doesn't seem to me that that paragraph says
anything about "You can have local bindings which don't appear local
when queried with local-variable-p".

Second, and the more important bug, which my code example was about, is
"the default value is set to whatever your current binding is, if you
enter and then exit a nested let-binding".  That seems definitely
uncovered by that paragraph, or any documentation.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#44733; Package emacs. (Thu, 19 Nov 2020 18:19:01 GMT) Full text and rfc822 format available.

Message #14 received at 44733 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Spencer Baugh <sbaugh <at> catern.com>, 44733 <at> debbugs.gnu.org
Subject: Re: bug#44733: Nested let bindings for non-local DEFVAR_PER_BUFFER
 variables unwind wrong
Date: Thu, 19 Nov 2020 13:17:51 -0500
>> (let ((left-margin 1))
>>   ;; Set this variable "pseudo-locally", inside a SPECPDL_LET_DEFAULT binding.
>>   (setq left-margin 123)
>>   (assert (eq left-margin 123))
>>   ;; Note, it's not a local variable.
>>   (assert (not (local-variable-p 'left-margin)))
>>   ;; The default value doesn't change.
>>   (assert (eq (default-value 'left-margin) 1))

This is a bug, indeed.  It should be 123 at this point.

>>   (with-temp-buffer (assert (eq left-margin 1)))

Same here, it should be 123.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#44733; Package emacs. (Thu, 19 Nov 2020 19:22:02 GMT) Full text and rfc822 format available.

Message #17 received at 44733 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> catern.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 44733 <at> debbugs.gnu.org
Subject: Re: bug#44733: Nested let bindings for non-local DEFVAR_PER_BUFFER
 variables unwind wrong
Date: Thu, 19 Nov 2020 14:21:16 -0500
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>> (let ((left-margin 1))
>>>   ;; Set this variable "pseudo-locally", inside a SPECPDL_LET_DEFAULT binding.
>>>   (setq left-margin 123)
>>>   (assert (eq left-margin 123))
>>>   ;; Note, it's not a local variable.
>>>   (assert (not (local-variable-p 'left-margin)))
>>>   ;; The default value doesn't change.
>>>   (assert (eq (default-value 'left-margin) 1))
>
> This is a bug, indeed.  It should be 123 at this point.

That's one perspective, but it seems less consistent with the
documentation and with expected behavior.  The documentation for these
variables says:

  Automatically becomes buffer-local when set.

and here, we are setting it, with setq.  It would seem that it should
become buffer-local, then.  Indeed, that's the current behavior, that it
becomes "pseudo-buffer-local", in that the value is different in this
buffer from every other buffer.  (But local-variable-p returns nil,
which is the only indication that something weird is going on.)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#44733; Package emacs. (Thu, 19 Nov 2020 20:24:01 GMT) Full text and rfc822 format available.

Message #20 received at 44733 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> catern.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 44733 <at> debbugs.gnu.org
Subject: Re: bug#44733: Nested let bindings for non-local DEFVAR_PER_BUFFER
 variables unwind wrong
Date: Thu, 19 Nov 2020 15:23:45 -0500
>> This is a bug, indeed.  It should be 123 at this point.
> That's one perspective, but it seems less consistent with the
> documentation and with expected behavior.

That's the way all other variables behave:

    (defvar-local sm-foo 1)
    (let ((sm-foo 23))
      (setq sm-foo 45)
      (list sm-foo
            (with-temp-buffer sm-foo)))

and I think it's asking for trouble if

    (let ((sm-foo 23))
      ...)

behaves differently from

    (let (sm-foo)
      (setq sm-foo 23)
      ...)

> and here, we are setting it, with setq.  It would seem that it should
> become buffer-local, then.  Indeed, that's the current behavior, that it
> becomes "pseudo-buffer-local", in that the value is different in this
> buffer from every other buffer.  (But local-variable-p returns nil,
> which is the only indication that something weird is going on.)

Indeed the current behavior is clearly buggy.  Historically, the
behavior of PER_BUFFER variables has been even more unlike that of
`make-variable-buffer-local` but over the years, I've made efforts to
make them behave the same.  Clearly, I missed this spot.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#44733; Package emacs. (Thu, 19 Nov 2020 20:57:01 GMT) Full text and rfc822 format available.

Message #23 received at 44733 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> catern.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 44733 <at> debbugs.gnu.org
Subject: Re: bug#44733: Nested let bindings for non-local DEFVAR_PER_BUFFER
 variables unwind wrong
Date: Thu, 19 Nov 2020 15:56:54 -0500
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>> This is a bug, indeed.  It should be 123 at this point.
>> That's one perspective, but it seems less consistent with the
>> documentation and with expected behavior.
>
> That's the way all other variables behave:
>
>     (defvar-local sm-foo 1)
>     (let ((sm-foo 23))
>       (setq sm-foo 45)
>       (list sm-foo
>             (with-temp-buffer sm-foo)))

Aha, okay, I certainly can't argue with that.  DEFVAR_PER_BUFFER
variables should match that behavior.  I'll update my patch series to
match.

> and I think it's asking for trouble if
>
>     (let ((sm-foo 23))
>       ...)
>
> behaves differently from
>
>     (let (sm-foo)
>       (setq sm-foo 23)
>       ...)

Ah, this example is persuasive to me.

I see also that there is at least some documentation of this behavior,
in the make-variable-buffer-local documentation in variables.texi, which
I missed:

  A peculiar wrinkle of this feature is that binding the variable (with
  @code{let} or other binding constructs) does not create a buffer-local
  binding for it.  Only setting the variable (with @code{set} or
  @code{setq}), while the variable does not have a @code{let}-style
  binding that was made in the current buffer, does so.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#44733; Package emacs. (Thu, 19 Nov 2020 22:15:02 GMT) Full text and rfc822 format available.

Message #26 received at 44733 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> catern.com>
Cc: 44733 <at> debbugs.gnu.org
Subject: Re: bug#44733: Nested let bindings for non-local DEFVAR_PER_BUFFER
 variables unwind wrong
Date: Thu, 19 Nov 2020 17:14:01 -0500
I just pushed a fix for that, along with the corresponding test case,


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#44733; Package emacs. (Sat, 23 Oct 2021 10:20:02 GMT) Full text and rfc822 format available.

Message #29 received at 44733 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Spencer Baugh <sbaugh <at> catern.com>, 44733 <at> debbugs.gnu.org
Subject: Re: bug#44733: Nested let bindings for non-local DEFVAR_PER_BUFFER
 variables unwind wrong
Date: Sat, 23 Oct 2021 03:19:42 -0700
close 44733 28.1
thanks

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

> I just pushed a fix for that, along with the corresponding test case,

It seems like this was fixed but never closed, so I'm closing it now.

commit 8fac2444641567b10f4c38b599636aeae0478e68
Author: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date:   Thu Nov 19 17:13:04 2020 -0500

    * src/data.c (set_internal): Fix bug#44733

    Set the default value when `set` encounters a PER_BUFFER variable
    which has been let-bound globally, to match the behavior seen with
    `make-variable-buffer-local`.

    * test/src/data-tests.el (binding-test--let-buffer-local):
    Add corresponding test.
    (data-tests--set-default-per-buffer): Add tentative test for the
    performance problem encountered in bug#41029.




bug marked as fixed in version 28.1, send any further explanations to 44733 <at> debbugs.gnu.org and Spencer Baugh <sbaugh <at> catern.com> Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Sat, 23 Oct 2021 10:20:03 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 20 Nov 2021 12:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 129 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.