GNU bug report logs -
#44733
Nested let bindings for non-local DEFVAR_PER_BUFFER variables unwind wrong
Previous Next
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.
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):
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: 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):
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):
>> (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):
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):
>> 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):
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):
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):
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.