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

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

[jakanakaevangeli]

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.

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

[jakanakaevangeli]

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.

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

[jakanakaevangeli]

Ping.

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

[Lars Ingebrigtsen]

jakanakaevangeli <jakanakaevangeli@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@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

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

[jakanakaevangeli]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> jakanakaevangeli <jakanakaevangeli@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@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.

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

[Lars Ingebrigtsen]

<jakanakaevangeli@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

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

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@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