GNU bug report logs - #26624
26.0.50; Generalized variable `buffer-local-value' does't restore local flag

Previous Next

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

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

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

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Sun, 23 Apr 2017 19:13:40 +0200

In *scratch*, evaluate:

(defvar foo-test-var nil)
(with-temp-buffer
  (list (list (buffer-local-value 'foo-test-var (current-buffer))
              (local-variable-p 'foo-test-var)
              (local-variable-if-set-p 'foo-test-var))
        (cl-letf (((buffer-local-value 'foo-test-var (current-buffer)) 123))
          (list (buffer-local-value 'foo-test-var (current-buffer))
                (local-variable-p 'foo-test-var)
                (local-variable-if-set-p 'foo-test-var)))
        (list (buffer-local-value 'foo-test-var (current-buffer))
              (local-variable-p 'foo-test-var)
              (local-variable-if-set-p 'foo-test-var))))

The result is:

((nil nil nil) (123 t t) (nil t t))

But expected is:

((nil nil nil) (123 t t) (nil nil nil))

i.e. the local flag of the variable should be reset.



In GNU Emacs 26.0.50 (build 19, x86_64-apple-darwin16.5.0, NS appkit-1504.82 Version 10.12.4 (Build 16E195))
 of 2017-04-23 built on p
Repository revision: a1f93c1dfa53dbe007faa09ab0c6e913e86e3ffe
Windowing system distributor 'Apple', version 10.3.1504
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --without-xml2 --with-modules --without-pop --with-mailutils
 --enable-checking --enable-check-lisp-object-type 'CFLAGS=-ggdb3 -O0'
 MAKEINFO=/usr/local/opt/texinfo/bin/makeinfo'

Configured features:
RSVG DBUS NOTIFY ACL GNUTLS ZLIB TOOLKIT_SCROLL_BARS NS MODULES

Important settings:
  value of $LANG: de_DE.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message subr-x puny seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils time-date tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/ns-win
ns-win ucs-normalize mule-util term/common-win tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow
isearch timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind kqueue cocoa ns multi-tty make-network-process emacs)

Memory information:
((conses 16 203864 9733)
 (symbols 48 19974 1)
 (miscs 40 57 190)
 (strings 32 18048 4861)
 (string-bytes 1 589547)
 (vectors 16 34976)
 (vector-slots 8 701022 3277)
 (floats 8 52 66)
 (intervals 56 199 0)
 (buffers 976 11))

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

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Sat, 17 Jun 2017 13:10:20 +0000

[Message part 1 (text/plain, inline)]

Philipp Stephani <p.stephani2 <at> gmail.com> schrieb am So., 23. Apr. 2017 um
19:14 Uhr:

>
> In *scratch*, evaluate:
>
> (defvar foo-test-var nil)
> (with-temp-buffer
>   (list (list (buffer-local-value 'foo-test-var (current-buffer))
>               (local-variable-p 'foo-test-var)
>               (local-variable-if-set-p 'foo-test-var))
>         (cl-letf (((buffer-local-value 'foo-test-var (current-buffer))
> 123))
>           (list (buffer-local-value 'foo-test-var (current-buffer))
>                 (local-variable-p 'foo-test-var)
>                 (local-variable-if-set-p 'foo-test-var)))
>         (list (buffer-local-value 'foo-test-var (current-buffer))
>               (local-variable-p 'foo-test-var)
>               (local-variable-if-set-p 'foo-test-var))))
>
> The result is:
>
> ((nil nil nil) (123 t t) (nil t t))
>
> But expected is:
>
> ((nil nil nil) (123 t t) (nil nil nil))
>
> i.e. the local flag of the variable should be reset.
>
>
It's possible to fix this (see attached patch), but at the expense of
breaking other valid use cases such as (cl-incf (buffer-local-value ...)).
Not sure whether the bug can be fixed at all without breaking other stuff.

[Message part 2 (text/html, inline)]

[0001-Have-cl-letf-restore-buffer-local-status-Bug-26624.txt (text/plain, attachment)]

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

From: npostavs <at> users.sourceforge.net
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Sat, 17 Jun 2017 14:48:02 -0400

Philipp Stephani <p.stephani2 <at> gmail.com> writes:

> It's possible to fix this (see attached patch), but at the expense of
> breaking other valid use cases such as (cl-incf (buffer-local-value ...)).
> Not sure whether the bug can be fixed at all without breaking other
> stuff.

I don't really understand the internals of the gv expander stuff, but
maybe we could special case the fix to cl-letf?

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Sun, 18 Jun 2017 06:17:15 +0200

Philipp Stephani <p.stephani2 <at> gmail.com> writes:

> It's possible to fix this (see attached patch), but at the expense of
> breaking other valid use cases such as (cl-incf (buffer-local-value
> ...)). Not sure whether the bug can be fixed at all without breaking
> other stuff.

I have no solution, but some thoughts.

The more I think about it, the more I come to the conclusion that
`buffer-local-value' does not have a well defined according place.

The function `buffer-local-value' is not injective: it maps different
states to the same value because it can't express whether the VARIABLE's
binding is buffer-local or not.  But we need this information because we
need to undo creating a buffer local binding in the setter when closing
the `letf'.

And the setter, accepting only a value for the binding, isn't
surjective, because the argument doesn't hold any information of
buffer-localness.  Moreover, we want the setter to always create a
buffer-local binding in one situation (setf), but this isn't true for
the setter we need to use for `cl-letf'.

We could widen the semantics of `cl-letf' to do what we want in this
case, but I'm not sure if it's worth the trouble.  Not if there are more
cases like this.


BTW, a completely different point of view would be to argument like
this: the manual describes generalized variables as "one of the many
places in Lisp memory where values can be stored".  If variable X has no
buffer local binding in some buffer B, that place in Lisp memory is the
one that holds the global binding of X.  That means that the place expression

  (buffer-local-binding X B)

is equivalent to the place expression X.

OTOH, If X has a buffer local binding in B, we can use X as getter
expression and `setq' as setter, so the place expression
(buffer-local-binding X B) is also equivalent to X.  So it is always
equivalent to just X and of no use.


Michael.

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

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Sun, 02 Jul 2017 16:53:33 +0000

[Message part 1 (text/plain, inline)]

Michael Heerdegen <michael_heerdegen <at> web.de> schrieb am So., 18. Juni 2017
um 06:17 Uhr:

> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>
> > It's possible to fix this (see attached patch), but at the expense of
> > breaking other valid use cases such as (cl-incf (buffer-local-value
> > ...)). Not sure whether the bug can be fixed at all without breaking
> > other stuff.
>
> I have no solution, but some thoughts.
>
> The more I think about it, the more I come to the conclusion that
> `buffer-local-value' does not have a well defined according place.
>
> The function `buffer-local-value' is not injective: it maps different
> states to the same value because it can't express whether the VARIABLE's
> binding is buffer-local or not.  But we need this information because we
> need to undo creating a buffer local binding in the setter when closing
> the `letf'.
>
> And the setter, accepting only a value for the binding, isn't
> surjective, because the argument doesn't hold any information of
> buffer-localness.  Moreover, we want the setter to always create a
> buffer-local binding in one situation (setf), but this isn't true for
> the setter we need to use for `cl-letf'.
>
> We could widen the semantics of `cl-letf' to do what we want in this
> case, but I'm not sure if it's worth the trouble.  Not if there are more
> cases like this.
>
>
Thanks for this great analysis. Given this, it seems that the place
definition for `buffer-local-value' should be removed from gv.el.

[Message part 2 (text/html, inline)]

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

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Sun, 24 Sep 2017 15:19:48 +0000

[Message part 1 (text/plain, inline)]

Philipp Stephani <p.stephani2 <at> gmail.com> schrieb am So., 2. Juli 2017 um
18:53 Uhr:

> Michael Heerdegen <michael_heerdegen <at> web.de> schrieb am So., 18. Juni
> 2017 um 06:17 Uhr:
>
>> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>>
>> > It's possible to fix this (see attached patch), but at the expense of
>> > breaking other valid use cases such as (cl-incf (buffer-local-value
>> > ...)). Not sure whether the bug can be fixed at all without breaking
>> > other stuff.
>>
>> I have no solution, but some thoughts.
>>
>> The more I think about it, the more I come to the conclusion that
>> `buffer-local-value' does not have a well defined according place.
>>
>> The function `buffer-local-value' is not injective: it maps different
>> states to the same value because it can't express whether the VARIABLE's
>> binding is buffer-local or not.  But we need this information because we
>> need to undo creating a buffer local binding in the setter when closing
>> the `letf'.
>>
>> And the setter, accepting only a value for the binding, isn't
>> surjective, because the argument doesn't hold any information of
>> buffer-localness.  Moreover, we want the setter to always create a
>> buffer-local binding in one situation (setf), but this isn't true for
>> the setter we need to use for `cl-letf'.
>>
>> We could widen the semantics of `cl-letf' to do what we want in this
>> case, but I'm not sure if it's worth the trouble.  Not if there are more
>> cases like this.
>>
>>
> Thanks for this great analysis. Given this, it seems that the place
> definition for `buffer-local-value' should be removed from gv.el.
>

Here's a patch that does just that.

[Message part 2 (text/html, inline)]

[0001-Remove-buffer-local-value-as-generalized-variable-Bug-.txt (text/plain, attachment)]

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Sun, 24 Sep 2017 17:36:10 +0200

Philipp Stephani <p.stephani2 <at> gmail.com> writes:

> Here's a patch that does just that. 

Thanks.  Looks good to me.


Michael.

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

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Sun, 24 Sep 2017 11:44:53 -0400

Philipp Stephani <p.stephani2 <at> gmail.com> writes:

> * lisp/emacs-lisp/gv.el (buffer-local-value): Remove.

Is it possible to just give an obsolete warning first?

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

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Sun, 24 Sep 2017 16:42:12 +0000

[Message part 1 (text/plain, inline)]

Noam Postavsky <npostavs <at> users.sourceforge.net> schrieb am So., 24. Sep.
2017 um 17:44 Uhr:

> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>
> > * lisp/emacs-lisp/gv.el (buffer-local-value): Remove.
>
> Is it possible to just give an obsolete warning first?
>

I don't think it's possible in the sense of `make-obsolete' because the
expander is not a named function.
It would be possible to use `display-warning' within the body of the
setter, but that would only annoy users.
If necessary, we might add additional code to the `setf' macro to warn
about this form in particular during byte compilation.

[Message part 2 (text/html, inline)]

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

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Sun, 24 Sep 2017 13:43:20 -0400

Philipp Stephani <p.stephani2 <at> gmail.com> writes:

> Noam Postavsky <npostavs <at> users.sourceforge.net> schrieb am So., 24.
> Sep. 2017 um 17:44 Uhr:
>
>     Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>    
>     > * lisp/emacs-lisp/gv.el (buffer-local-value): Remove.
>    
>     Is it possible to just give an obsolete warning first?
>
>
> I don't think it's possible in the sense of `make-obsolete' because
> the expander is not a named function.
> It would be possible to use `display-warning' within the body of the
> setter, but that would only annoy users.
> If necessary, we might add additional code to the `setf' macro to
> warn about this form in particular during byte compilation.

IMO, a compilation warning would be appropriate.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: michael_heerdegen <at> web.de, p.stephani2 <at> gmail.com, 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Fri, 29 Sep 2017 10:50:36 +0300

> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> Date: Sun, 24 Sep 2017 13:43:20 -0400
> Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 26624 <at> debbugs.gnu.org
> 
> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
> 
> > Noam Postavsky <npostavs <at> users.sourceforge.net> schrieb am So., 24.
> > Sep. 2017 um 17:44 Uhr:
> >
> >     Philipp Stephani <p.stephani2 <at> gmail.com> writes:
> >    
> >     > * lisp/emacs-lisp/gv.el (buffer-local-value): Remove.
> >    
> >     Is it possible to just give an obsolete warning first?
> >
> >
> > I don't think it's possible in the sense of `make-obsolete' because
> > the expander is not a named function.
> > It would be possible to use `display-warning' within the body of the
> > setter, but that would only annoy users.
> > If necessary, we might add additional code to the `setf' macro to
> > warn about this form in particular during byte compilation.
> 
> IMO, a compilation warning would be appropriate.

I agree.  Removing some feature without due warning is not something
we should do, except in very rare cases (which this one isn't).

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

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>,
 Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: michael_heerdegen <at> web.de, 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Fri, 29 Sep 2017 20:55:41 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> schrieb am Fr., 29. Sep. 2017 um 09:51 Uhr:

> > From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> > Date: Sun, 24 Sep 2017 13:43:20 -0400
> > Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 26624 <at> debbugs.gnu.org
> >
> > Philipp Stephani <p.stephani2 <at> gmail.com> writes:
> >
> > > Noam Postavsky <npostavs <at> users.sourceforge.net> schrieb am So., 24.
> > > Sep. 2017 um 17:44 Uhr:
> > >
> > >     Philipp Stephani <p.stephani2 <at> gmail.com> writes:
> > >
> > >     > * lisp/emacs-lisp/gv.el (buffer-local-value): Remove.
> > >
> > >     Is it possible to just give an obsolete warning first?
> > >
> > >
> > > I don't think it's possible in the sense of `make-obsolete' because
> > > the expander is not a named function.
> > > It would be possible to use `display-warning' within the body of the
> > > setter, but that would only annoy users.
> > > If necessary, we might add additional code to the `setf' macro to
> > > warn about this form in particular during byte compilation.
> >
> > IMO, a compilation warning would be appropriate.
>
> I agree.  Removing some feature without due warning is not something
> we should do, except in very rare cases (which this one isn't).
>

I fully agree, but I don't know how to correctly deprecate a generalized
variable. Should I add code to the byte compiler to deal with this case
explicitly?

[Message part 2 (text/html, inline)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: michael_heerdegen <at> web.de, 26624 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Sat, 30 Sep 2017 09:46:22 +0300

> From: Philipp Stephani <p.stephani2 <at> gmail.com>
> Date: Fri, 29 Sep 2017 20:55:41 +0000
> Cc: michael_heerdegen <at> web.de, 26624 <at> debbugs.gnu.org
> 
>  I agree. Removing some feature without due warning is not something
>  we should do, except in very rare cases (which this one isn't).
> 
> I fully agree, but I don't know how to correctly deprecate a generalized variable. Should I add code to the byte
> compiler to deal with this case explicitly? 

If no other idea comes up, yes, I think so.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Philipp Stephani <p.stephani2 <at> gmail.com>, 26624 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Tue, 26 Dec 2017 23:19:33 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> >  I agree. Removing some feature without due warning is not something
> >  we should do, except in very rare cases (which this one isn't).
> > 
> > I fully agree, but I don't know how to correctly deprecate a
> > generalized variable. Should I add code to the byte
> > compiler to deal with this case explicitly? 
>
> If no other idea comes up, yes, I think so.

I guess it's not really worth the time to implement an infrastructure
for gv-expander obsoletion, because we will probably make use of it only
every 150 years (estimation).  So it could be that nobody wants to do
this for quite a while.

Would it be acceptable if the gv setter of `buffer-local-value' would
just print a warning (i.e., solve it "by hand")?  Not perfect, admitted,
but still much better than leaving this unfixed.


Thanks,

Michael.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: p.stephani2 <at> gmail.com, 26624 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Wed, 27 Dec 2017 18:10:49 +0200

> From: Michael Heerdegen <michael_heerdegen <at> web.de>
> Cc: Philipp Stephani <p.stephani2 <at> gmail.com>,  26624 <at> debbugs.gnu.org,  npostavs <at> users.sourceforge.net
> Date: Tue, 26 Dec 2017 23:19:33 +0100
> 
> I guess it's not really worth the time to implement an infrastructure
> for gv-expander obsoletion, because we will probably make use of it only
> every 150 years (estimation).  So it could be that nobody wants to do
> this for quite a while.
> 
> Would it be acceptable if the gv setter of `buffer-local-value' would
> just print a warning (i.e., solve it "by hand")?  Not perfect, admitted,
> but still much better than leaving this unfixed.

Is it (better than leaving this unfixed)?

In any case, I don't think I understand the suggestion in detail.  Can
you show a patch or an idea of a patch?

Thanks.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: p.stephani2 <at> gmail.com, 26624 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Wed, 27 Dec 2017 20:54:28 +0100

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

> In any case, I don't think I understand the suggestion in detail.  Can
> you show a patch or an idea of a patch?

Maybe simply something like this?

[0001-Obsolete-gv-setter-of-buffer-local-value.patch (text/x-diff, inline)]

From 5bc85fadc201eb7d061fe585283f0f4e44f0d910 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen <at> web.de>
Date: Wed, 27 Dec 2017 20:46:20 +0100
Subject: [PATCH] Obsolete gv-setter of `buffer-local-value'

This solves Bug#26624.
---
 lisp/emacs-lisp/gv.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
index 777b955d90..f0aad6689d 100644
--- a/lisp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -370,6 +370,7 @@ setf
 (gv-define-setter window-start (v &optional w) `(set-window-start ,w ,v))
 
 (gv-define-setter buffer-local-value (val var buf)
+  (byte-compile-warn "Warning: obsolete gv-setter: `buffer-local-value'")
   (macroexp-let2 nil v val
     `(with-current-buffer ,buf (set (make-local-variable ,var) ,v))))
 
-- 
2.15.1

[Message part 3 (text/plain, inline)]

Michael.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: p.stephani2 <at> gmail.com, 26624 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Wed, 27 Dec 2017 22:27:14 +0200

> From: Michael Heerdegen <michael_heerdegen <at> web.de>
> Cc: p.stephani2 <at> gmail.com,  26624 <at> debbugs.gnu.org,  npostavs <at> users.sourceforge.net
> Date: Wed, 27 Dec 2017 20:54:28 +0100
> 
> diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
> index 777b955d90..f0aad6689d 100644
> --- a/lisp/emacs-lisp/gv.el
> +++ b/lisp/emacs-lisp/gv.el
> @@ -370,6 +370,7 @@ setf
>  (gv-define-setter window-start (v &optional w) `(set-window-start ,w ,v))
>  
>  (gv-define-setter buffer-local-value (val var buf)
> +  (byte-compile-warn "Warning: obsolete gv-setter: `buffer-local-value'")
>    (macroexp-let2 nil v val
>      `(with-current-buffer ,buf (set (make-local-variable ,var) ,v))))

Does this warning pop up during bootstrap, and if so, how many times?

Thanks.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: p.stephani2 <at> gmail.com, 26624 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Fri, 29 Dec 2017 15:08:41 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> Does this warning pop up during bootstrap, and if so, how many times?

It currently would pop up five times:

| ../lisp/electric.el:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’
| --------------------
| ../lisp/electric.el:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’
| --------------------
| electric.el:350:40:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’
| --------------------
| electric.el:580:39:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’
| --------------------
| elec-pair.el:608:38:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’


Yes, these would need to be treated.


Michael.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: p.stephani2 <at> gmail.com, 26624 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Fri, 29 Dec 2017 18:14:23 +0200

> From: Michael Heerdegen <michael_heerdegen <at> web.de>
> Cc: p.stephani2 <at> gmail.com,  26624 <at> debbugs.gnu.org,  npostavs <at> users.sourceforge.net
> Date: Fri, 29 Dec 2017 15:08:41 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Does this warning pop up during bootstrap, and if so, how many times?
> 
> It currently would pop up five times:
> 
> | ../lisp/electric.el:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’
> | --------------------
> | ../lisp/electric.el:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’
> | --------------------
> | electric.el:350:40:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’
> | --------------------
> | electric.el:580:39:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’
> | --------------------
> | elec-pair.el:608:38:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’
> 
> 
> Yes, these would need to be treated.

Can we treat them as part of fixing this issue?

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

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 26624 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Fri, 29 Dec 2017 16:20:49 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> schrieb am Fr., 29. Dez. 2017 um 17:15 Uhr:

> > From: Michael Heerdegen <michael_heerdegen <at> web.de>
> > Cc: p.stephani2 <at> gmail.com,  26624 <at> debbugs.gnu.org,
> npostavs <at> users.sourceforge.net
> > Date: Fri, 29 Dec 2017 15:08:41 +0100
> >
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> > > Does this warning pop up during bootstrap, and if so, how many times?
> >
> > It currently would pop up five times:
> >
> > | ../lisp/electric.el:Warning: Warning: obsolete gv-setter:
> ‘buffer-local-value’
> > | --------------------
> > | ../lisp/electric.el:Warning: Warning: obsolete gv-setter:
> ‘buffer-local-value’
> > | --------------------
> > | electric.el:350:40:Warning: Warning: obsolete gv-setter:
> ‘buffer-local-value’
> > | --------------------
> > | electric.el:580:39:Warning: Warning: obsolete gv-setter:
> ‘buffer-local-value’
> > | --------------------
> > | elec-pair.el:608:38:Warning: Warning: obsolete gv-setter:
> ‘buffer-local-value’
> >
> >
> > Yes, these would need to be treated.
>
> Can we treat them as part of fixing this issue?
>

Yes, but I think changing them should be a separate commit because it's not
straightforward.
These are all modes that can be locally or globally enabled. I think
typically such modes would be defined using `define-minor-mode` and
`define-globalized-minor-mode`, but the electric modes are defined the
other way round, i.e. the main modes are global, and then there are local
modes that use `buffer-local-value` as mode variable. I'd suggest to turn
this around to use `define-minor-mode` for the local modes and
`define-globalized-minor-mode` for the global ones. Would that have any
downsides?

[Message part 2 (text/html, inline)]

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Wed, 24 Jan 2018 15:33:38 +0100

Philipp Stephani <p.stephani2 <at> gmail.com> writes:

> Thanks for this great analysis. Given this, it seems that the place
> definition for `buffer-local-value' should be removed from gv.el.

But hmm - surely there are other functions where `cl-letf' doesn't
exactly restore the previous state - `alist-get', for example:

#+begin_src emacs-lisp
(setq my-alist '((x . 1)))
(ignore (cl-letf (((alist-get 'y my-alist) 17)) my-alist))
my-alist
==> ((y) (x . 1))
#+end_src

(admittedly, this is not as serious as the `buffer-local-value' case).
Also, as another, different problematic case, the manual warns about
`point' to be used with `cl-letf'.

So, I wonder if, instead of removing the gv-setter definition for
`buffer-local-value', we should instead add some more text about how
`cl-letf' can have surprising effects to the manual.


Michael.

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

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Sun, 04 Feb 2018 19:01:15 +0000

[Message part 1 (text/plain, inline)]

Michael Heerdegen <michael_heerdegen <at> web.de> schrieb am Mi., 24. Jan. 2018
um 15:33 Uhr:

> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>
> > Thanks for this great analysis. Given this, it seems that the place
> > definition for `buffer-local-value' should be removed from gv.el.
>
> But hmm - surely there are other functions where `cl-letf' doesn't
> exactly restore the previous state - `alist-get', for example:
>
> #+begin_src emacs-lisp
> (setq my-alist '((x . 1)))
> (ignore (cl-letf (((alist-get 'y my-alist) 17)) my-alist))
> my-alist
> ==> ((y) (x . 1))
> #+end_src
>
> (admittedly, this is not as serious as the `buffer-local-value' case).
> Also, as another, different problematic case, the manual warns about
> `point' to be used with `cl-letf'.
>
> So, I wonder if, instead of removing the gv-setter definition for
> `buffer-local-value', we should instead add some more text about how
> `cl-letf' can have surprising effects to the manual.
>
>
>
I think we should spend significant efforts to avoid surprises. In this
case, if it means we should remove `alist-get' as well from the forms
supported by `cl-letf', then I think that's what we should do. The
documentation for `cl-letf' clearly states: "On exit, either normally or
because of a ‘throw’ or error, the PLACEs are set back to their original
values." If it can't do that for some place form, it shouldn't be allowed.

[Message part 2 (text/html, inline)]

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Sun, 04 Feb 2018 22:02:37 +0100

Philipp Stephani <p.stephani2 <at> gmail.com> writes:

>  #+begin_src emacs-lisp
>  (setq my-alist '((x . 1)))
>  (ignore (cl-letf (((alist-get 'y my-alist) 17)) my-alist))
>  my-alist
>  ==> ((y) (x . 1))
>  #+end_src

> I think we should spend significant efforts to avoid surprises. In
> this case, if it means we should remove `alist-get' as well from the
> forms supported by `cl-letf', then I think that's what we should
> do. The documentation for `cl-letf' clearly states: "On exit, either
> normally or because of a ‘throw’ or error, the PLACEs are set back to
> their original values." If it can't do that for some place form, it
> shouldn't be allowed.

But

  (alist-get value my-alist)

doesn't change for any value (especially for y), so the alist, or the
`alist-get' place expressions, aren't effectively changed.  The object
that represents the alist changes, however.  Is that a problem or an
internal implementation detail?


Michael.

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

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Sun, 11 Feb 2018 17:54:02 +0000

[Message part 1 (text/plain, inline)]

Michael Heerdegen <michael_heerdegen <at> web.de> schrieb am So., 4. Feb. 2018
um 22:02 Uhr:

> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>
> >  #+begin_src emacs-lisp
> >  (setq my-alist '((x . 1)))
> >  (ignore (cl-letf (((alist-get 'y my-alist) 17)) my-alist))
> >  my-alist
> >  ==> ((y) (x . 1))
> >  #+end_src
>
> > I think we should spend significant efforts to avoid surprises. In
> > this case, if it means we should remove `alist-get' as well from the
> > forms supported by `cl-letf', then I think that's what we should
> > do. The documentation for `cl-letf' clearly states: "On exit, either
> > normally or because of a ‘throw’ or error, the PLACEs are set back to
> > their original values." If it can't do that for some place form, it
> > shouldn't be allowed.
>
> But
>
>   (alist-get value my-alist)
>
> doesn't change for any value (especially for y), so the alist, or the
> `alist-get' place expressions, aren't effectively changed.  The object
> that represents the alist changes, however.  Is that a problem or an
> internal implementation detail?
>
>
>
Since it affects user-visible behavior, I wouldn't classify it as internal
implementation detail.
It seems to me that the approach that `cl-letf` takes is too naive: binding
a generalized variable is never the same as setting it and later resetting
it to the previous value, not even for simple dynamic symbols (consider
unbound variables). Rather than having `(cl-letf ((place val)) body)`
expand to

(let ((oldval place))
  (setf place val)
  (unwind-protect body
    (setf place oldval)))

it should rather expand to

(let ((old-state (internal-get-state place)))
  (setf place val)
  (unwind-protect body
    (internal-reset-state place old-state)))

with suitably defined `internal-get-state` and `internal-reset-state`. For
most use cases `internal-get-state` and `internal-reset-state` could just
be `identity` and `setf`, but for the cases discussed here they would
contain additional information.

[Message part 2 (text/html, inline)]

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50;
 Generalized variable `buffer-local-value' does't restore local flag
Date: Sun, 11 Feb 2018 21:56:47 +0100

Philipp Stephani <p.stephani2 <at> gmail.com> writes:

> it should rather expand to
>
> (let ((old-state (internal-get-state place)))
>   (setf place val)
>   (unwind-protect body
>     (internal-reset-state place old-state)))
>
> with suitably defined `internal-get-state` and
> `internal-reset-state`. For most use cases `internal-get-state` and
> `internal-reset-state` could just be `identity` and `setf `, but for
> the cases discussed here they would contain additional information.

Is that even well-defined?  What happens when the code inside `letf'
also alters this state?

For example, code like

#+begin_src emacs-lisp
(let ((my-alist '((x 1))))
  (cl-letf (((alist-get 'y my-alist) 2))
    (push (cons 'y 17) my-alist))
  my-alist)
#+end_src

or

#+begin_src emacs-lisp
(cl-letf (((buffer-local-value 'x my-buffer) 20))
  ...
  (with-current-buffer my-buffer
    (set (make-local-variable 'x) 0))
  ...)
#+end_src

what would "reset the state" mean?


Michael.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Philipp Stephani <p.stephani2 <at> gmail.com>, 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Sun, 21 Aug 2022 23:38:34 +0200

Noam Postavsky <npostavs <at> users.sourceforge.net> writes:

> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>
>> * lisp/emacs-lisp/gv.el (buffer-local-value): Remove.
>
> Is it possible to just give an obsolete warning first?

I've added a mechanism for obsoletion, and I've now followed Michael's
recommendation about buffer-local-value not being well-defined as a
generalized variable, and obsoleted it in Emacs 29.

I'm therefore closing this bug report.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Philipp Stephani <p.stephani2 <at> gmail.com>, 26624 <at> debbugs.gnu.org
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Mon, 22 Aug 2022 00:21:33 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> I've added a mechanism for obsoletion, and I've now followed Michael's
> recommendation about buffer-local-value not being well-defined as a
> generalized variable, and obsoleted it in Emacs 29.

It turns out that while not well-defined, it's useful here:

(define-minor-mode electric-indent-local-mode
  "Toggle `electric-indent-mode' only in this buffer."
  :variable (buffer-local-value 'electric-indent-mode (current-buffer))

Rewriting this to avoid this is slightly cumbersome, it turns out.  So
I'm not sure it's worth obsoleting the form, and we just have to live
with the somewhat odd semantics.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Philipp Stephani <p.stephani2 <at> gmail.com>, 26624 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> users.sourceforge.net>
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Mon, 22 Aug 2022 08:53:53 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> It turns out that while not well-defined, it's useful here:
>
> (define-minor-mode electric-indent-local-mode
>   "Toggle `electric-indent-mode' only in this buffer."
>   :variable (buffer-local-value 'electric-indent-mode (current-buffer))
>
> Rewriting this to avoid this is slightly cumbersome, it turns out.

Why not use the (GET . SET) syntax for :variable?

Michael.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Philipp Stephani <p.stephani2 <at> gmail.com>, 26624 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> users.sourceforge.net>
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Mon, 22 Aug 2022 12:10:43 +0200

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

>> (define-minor-mode electric-indent-local-mode
>>   "Toggle `electric-indent-mode' only in this buffer."
>>   :variable (buffer-local-value 'electric-indent-mode (current-buffer))
>>
>> Rewriting this to avoid this is slightly cumbersome, it turns out.
>
> Why not use the (GET . SET) syntax for :variable?

Let's see...  would this work?

:variable (electric-indent-mode .
           (lambda (val) (setq-local electric-indent-mode val)))
           

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Philipp Stephani <p.stephani2 <at> gmail.com>, 26624 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> users.sourceforge.net>
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Mon, 22 Aug 2022 23:43:30 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Let's see...  would this work?
>
> :variable (electric-indent-mode .
>            (lambda (val) (setq-local electric-indent-mode val)))

Looks good - similar specs are already used in other places.

BTW: I find the definition of `electric-indent-local-mode' inelegant:
the handling of the variable is split between the :variable spec and the
body.  The body enables the global mode and sets the global variable
back to nil - quite hackish.

Anyway, if this is needed in more places it cries for a
`define-localized-mode'.

Michael.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Philipp Stephani <p.stephani2 <at> gmail.com>, 26624 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> users.sourceforge.net>
Subject: Re: bug#26624: 26.0.50; Generalized variable `buffer-local-value'
 does't restore local flag
Date: Tue, 23 Aug 2022 12:28:47 +0200

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

>> :variable (electric-indent-mode .
>>            (lambda (val) (setq-local electric-indent-mode val)))
>
> Looks good - similar specs are already used in other places.

Now pushed to Emacs 29 (and I've made buffer-local-value obsolete again
as a generalized variable).

> BTW: I find the definition of `electric-indent-local-mode' inelegant:
> the handling of the variable is split between the :variable spec and the
> body.  The body enables the global mode and sets the global variable
> back to nil - quite hackish.

Yes, it's not ideal at all.

> Anyway, if this is needed in more places it cries for a
> `define-localized-mode'.

I hope it's not used a lot -- we'd rather have modes work in the
opposite direction.  That is, the mode is local, and then we have
globalized versions of it.

Previous Next

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