GNU bug report logs - #46407
27.1; Hooks with permanent-local-hook are not cleared of lambdas

Previous Next

Package: emacs;

Reported by: jakanakaevangeli <jakanakaevangeli <at> chiru.no>

Date: Tue, 9 Feb 2021 20:01:01 UTC

Severity: normal

Tags: patch

Found in version 27.1

Fixed in version 28.1

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

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 46407 in the body.
You can then email your comments to 46407 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#46407; Package emacs. (Tue, 09 Feb 2021 20:01:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to jakanakaevangeli <jakanakaevangeli <at> chiru.no>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 09 Feb 2021 20:01:01 GMT) Full text and rfc822 format available.

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

From: jakanakaevangeli <jakanakaevangeli <at> chiru.no>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.1; Hooks with permanent-local-hook are not cleared of lambdas
Date: Tue, 09 Feb 2021 20:29:48 +0100
When kill-all-local-variables encounters a hook variable with its
'permanent-local property set to 'permanent-local-hook, it removes from
its value every element except for t, functions with
'permanent-local-hook property and anything that isn't a symbol
(see the comment at src/buffer.c:1072).

This means that, for the following code

     (defvar   'some-hook nil)
     (add-hook 'some-hook #'some-fun         nil t)
     (add-hook 'some-hook (lambda () (test)) nil t)

whether some-fun is removed depends on some-fun's permanent-local-hook
property, which is expected. As for the anonymous lambda function, it is
not predictable, whether it will be kept or removed. In fact, it depends
on some-fun's permanent-local-hook property.

Perhaps it would make things more predictable to also remove non-symbol
functions such as lambda expressions.

See also info node (elisp) Creating Buffer-Local:
>  -- Function: kill-all-local-variables
>      This function eliminates all the buffer-local variable bindings
>      except for [...] and local hook functions that have a non-‘nil’
>      ‘permanent-local-hook’ property.
Which suggests that functions with a permanent-local-hook property
should be the only exception.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46407; Package emacs. (Tue, 09 Feb 2021 21:36:01 GMT) Full text and rfc822 format available.

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

From: jakanakaevangeli <jakanakaevangeli <at> chiru.no>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: 27.1; Hooks with permanent-local-hook are not cleared of lambdas
Date: Tue, 09 Feb 2021 22:38:49 +0100
Also one more thing:

    (defvar some-hook nil)
    (add-hook 'some-hook #'some-fun nil t)
    (put 'some-fun 'permanent-local-hook t)

If we mark a function as permanent-local-hook only after adding it to a
hook, the hook symbol will not have its permanent-local property set to
'permanent-local-hook, so some-fun will not be kept on removal of local
variables.

This can be a real non-theoretical problem when adding an autoloaded
function to a hook.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46407; Package emacs. (Fri, 21 May 2021 12:52:02 GMT) Full text and rfc822 format available.

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

From: <jakanakaevangeli <at> chiru.no>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: 27.1; Hooks with permanent-local-hook are not cleared of lambdas
Date: Fri, 21 May 2021 14:56:17 +0200
Ping.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46407; Package emacs. (Mon, 24 May 2021 21:56:02 GMT) Full text and rfc822 format available.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: jakanakaevangeli <jakanakaevangeli <at> chiru.no>
Cc: 46407 <at> debbugs.gnu.org
Subject: Re: bug#46407: 27.1; Hooks with permanent-local-hook are not
 cleared of lambdas
Date: Mon, 24 May 2021 23:55:28 +0200
jakanakaevangeli <jakanakaevangeli <at> chiru.no> writes:

> When kill-all-local-variables encounters a hook variable with its
> 'permanent-local property set to 'permanent-local-hook, it removes from
> its value every element except for t, functions with
> 'permanent-local-hook property and anything that isn't a symbol
> (see the comment at src/buffer.c:1072).
>
> This means that, for the following code
>
>      (defvar   'some-hook nil)
>      (add-hook 'some-hook #'some-fun         nil t)
>      (add-hook 'some-hook (lambda () (test)) nil t)
>
> whether some-fun is removed depends on some-fun's permanent-local-hook
> property, which is expected. As for the anonymous lambda function, it is
> not predictable, whether it will be kept or removed. In fact, it depends
> on some-fun's permanent-local-hook property.

Well, it depends on the hook's permanent-local-hook property, but it's
true that add-hook will automatically set that for you if you pass in a
symbol with that property set.

So, yes, that's a strange side effect of this interface, but I'm not
sure anything could be done about it at this stage (it was introduced in
this form almost two decades ago).

jakanakaevangeli <jakanakaevangeli <at> chiru.no> writes:

> Also one more thing:
>
>     (defvar some-hook nil)
>     (add-hook 'some-hook #'some-fun nil t)
>     (put 'some-fun 'permanent-local-hook t)
>
> If we mark a function as permanent-local-hook only after adding it to a
> hook, the hook symbol will not have its permanent-local property set to
> 'permanent-local-hook, so some-fun will not be kept on removal of local
> variables.
>
> This can be a real non-theoretical problem when adding an autoloaded
> function to a hook.

You mean if the property isn't part of the autoloaded signature?  Yes,
that's true.

As far as I can tell, this permanent-local-hook stuff isn't used
anywhere in the Emacs tree, and it seems like a pretty odd and (as this
bug report shows) inconsistent interface.  I'm not sure whether it's
worth trying to fix, or we should just document that it's iffy.

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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46407; Package emacs. (Tue, 25 May 2021 07:06:02 GMT) Full text and rfc822 format available.

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

From: <jakanakaevangeli <at> chiru.no>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 46407 <at> debbugs.gnu.org
Subject: Re: bug#46407: 27.1; Hooks with permanent-local-hook are not
 cleared of lambdas
Date: Tue, 25 May 2021 09:10:43 +0200
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> jakanakaevangeli <jakanakaevangeli <at> chiru.no> writes:
>
>> When kill-all-local-variables encounters a hook variable with its
>> 'permanent-local property set to 'permanent-local-hook, it removes from
>> its value every element except for t, functions with
>> 'permanent-local-hook property and anything that isn't a symbol
>> (see the comment at src/buffer.c:1072).
>>
>> This means that, for the following code
>>
>>      (defvar   'some-hook nil)
>>      (add-hook 'some-hook #'some-fun         nil t)
>>      (add-hook 'some-hook (lambda () (test)) nil t)
>>
>> whether some-fun is removed depends on some-fun's permanent-local-hook
>> property, which is expected. As for the anonymous lambda function, it is
>> not predictable, whether it will be kept or removed. In fact, it depends
>> on some-fun's permanent-local-hook property.
>
> Well, it depends on the hook's permanent-local-hook property, but it's
> true that add-hook will automatically set that for you if you pass in a
> symbol with that property set.

> So, yes, that's a strange side effect of this interface, but I'm not
> sure anything could be done about it at this stage (it was introduced in
> this form almost two decades ago).

On the other hand, this side effect is so undocumented and
undiscoverable (and most probably not really of any use as well), that I
highly doubt there is even a single instance of anyone relying in it
deliberately. I may be wrong though.

>
> jakanakaevangeli <jakanakaevangeli <at> chiru.no> writes:
>
>> Also one more thing:
>>
>>     (defvar some-hook nil)
>>     (add-hook 'some-hook #'some-fun nil t)
>>     (put 'some-fun 'permanent-local-hook t)
>>
>> If we mark a function as permanent-local-hook only after adding it to a
>> hook, the hook symbol will not have its permanent-local property set to
>> 'permanent-local-hook, so some-fun will not be kept on removal of local
>> variables.
>>
>> This can be a real non-theoretical problem when adding an autoloaded
>> function to a hook.
>
> You mean if the property isn't part of the autoloaded signature?  Yes,
> that's true.

Whoops, I forgot that one can simply put an autoload cookie on the
(put ...) form as well. Please ignore my second post.

> As far as I can tell, this permanent-local-hook stuff isn't used
> anywhere in the Emacs tree, and it seems like a pretty odd and (as this
> bug report shows) inconsistent interface.  I'm not sure whether it's
> worth trying to fix, or we should just document that it's iffy.

I'm personally in favour of fixing the `kill-all-local-variables'
inconsistency (patch follows).

If not, then yes, we should document it in add-hook and fix the info
node I mentioned. Though that would leave virtually every local addition
of a lambda to a hook invalid and a possible bug (a quick grep reveals
that there are at least three such usages in Emacs itself).



diff --git a/src/buffer.c b/src/buffer.c
index 565577e75a..6933f89bd4 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1079,9 +1079,9 @@ reset_buffer_local_variables (struct buffer *b, bool permanent_too)
                         /* Preserve element ELT if it's t,
                            if it is a function with a `permanent-local-hook' property,
                            or if it's not a symbol.  */
-                        if (! SYMBOLP (elt)
-                            || EQ (elt, Qt)
-                            || !NILP (Fget (elt, Qpermanent_local_hook)))
+                        if (EQ (elt, Qt)
+                            || (SYMBOLP (elt)
+                                && !NILP (Fget (elt, Qpermanent_local_hook))))
                           newlist = Fcons (elt, newlist);
                       }
                   newlist = Fnreverse (newlist);




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

Best regards.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46407; Package emacs. (Tue, 25 May 2021 19:11:01 GMT) Full text and rfc822 format available.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: <jakanakaevangeli <at> chiru.no>
Cc: 46407 <at> debbugs.gnu.org
Subject: Re: bug#46407: 27.1; Hooks with permanent-local-hook are not
 cleared of lambdas
Date: Tue, 25 May 2021 21:10:04 +0200
<jakanakaevangeli <at> chiru.no> writes:

> On the other hand, this side effect is so undocumented and
> undiscoverable (and most probably not really of any use as well), that I
> highly doubt there is even a single instance of anyone relying in it
> deliberately. I may be wrong though.

Yeah, that's the problem -- it's hard to say whether anybody relies on
this weird behaviour.

There are no in-tree usages of permanent-local-hook at all as far as I
can see, so I wondered whether is was used at all:

https://github.com/search?q=permanent-local-hook+extension%3A.el&type=Code&ref=advsearch&l=&l=

And the answer is yes.  (But that doesn't tell us if anybody relies on
the non-symbol behaviour, though.)

> I'm personally in favour of fixing the `kill-all-local-variables'
> inconsistency (patch follows).
>
> If not, then yes, we should document it in add-hook and fix the info
> node I mentioned. Though that would leave virtually every local addition
> of a lambda to a hook invalid and a possible bug (a quick grep reveals
> that there are at least three such usages in Emacs itself).

I'm leaning towards applying your patch (i.e., changing the behaviour as
you suggest), because it's somewhat difficult to imagine anybody relying
on the current, odd behaviour.

Does anybody object?

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




Added tag(s) patch. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Tue, 25 May 2021 19:11:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46407; Package emacs. (Tue, 20 Jul 2021 14:33:02 GMT) Full text and rfc822 format available.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: <jakanakaevangeli <at> chiru.no>
Cc: 46407 <at> debbugs.gnu.org
Subject: Re: bug#46407: 27.1; Hooks with permanent-local-hook are not
 cleared of lambdas
Date: Tue, 20 Jul 2021 16:31:57 +0200
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> I'm leaning towards applying your patch (i.e., changing the behaviour as
> you suggest), because it's somewhat difficult to imagine anybody relying
> on the current, odd behaviour.
>
> Does anybody object?

There were no objections, so I've now applied your patch.

This change was small enough to apply without assigning copyright to the
FSF, but for future patches you want to submit, it might make sense to
get the paperwork started now, so that subsequent patches can be applied
speedily. Would you be willing to sign such paperwork?

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




bug marked as fixed in version 28.1, send any further explanations to 46407 <at> debbugs.gnu.org and jakanakaevangeli <jakanakaevangeli <at> chiru.no> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Tue, 20 Jul 2021 14:33:02 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. (Wed, 18 Aug 2021 11:24:04 GMT) Full text and rfc822 format available.

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

Previous Next


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