bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Daniel Mendler]

(Follow-up to bug#46326 as suggested by Stefan Monnier)

The functions `add/remove-hook` make use of `equal` to test equality of 
hooks. Using `equal` can lead to excessive memory allocations 
(bug#46326) or hangups (see comment in `set-transient-map`), when large 
closures or cyclic closures are used as hooks.

Right now there are at least three places which have to work around the 
use of `equal` in `add/remove-hook` using a symbol indirection:

* `set-transient-map`
* `minibuffer-with-setup-hook`
* `eval-after-load`

It would be good to change `add/remove-hook` such that it only relies on 
`eq` to test hook equality. Then the symbol indirection workarounds can 
be avoided.

However making such a change directly can lead to subtle breakage. 
Perhaps one could introduce some deprecation behavior first, before 
making the final change to `eq`.  If a hook is added/removed and the 
added/removed object is not found via `eq` but found via `equal`, show a 
deprecation warning?

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Drew Adams]

> (Follow-up to bug#46326 as suggested by Stefan Monnier)
> 
> The functions `add/remove-hook` make use of `equal` to test equality of
> hooks. Using `equal` can lead to excessive memory allocations
> (bug#46326) or hangups (see comment in `set-transient-map`), when large
> closures or cyclic closures are used as hooks.
> 
> Right now there are at least three places which have to work around the
> use of `equal` in `add/remove-hook` using a symbol indirection:
> 
> * `set-transient-map`
> * `minibuffer-with-setup-hook`
> * `eval-after-load`
> 
> It would be good to change `add/remove-hook` such that it only relies
> on `eq` to test hook equality. Then the symbol indirection workarounds
> can be avoided.
> 
> However making such a change directly can lead to subtle breakage.
> Perhaps one could introduce some deprecation behavior first, before
> making the final change to `eq`.  If a hook is added/removed and the
> added/removed object is not found via `eq` but found via `equal`, show
> a deprecation warning?

So instead of just advising users not to use lambda forms
(which makes sense), you'd make it no longer work at all
for interpreted lambda forms (except rare cases where
they might actually be `eq' - e.g., same list structure)?

Perhaps `equal' can be fixed to do something better with
closures?  E.g., if the `eq' test in `equal' fails for a
closure arg then return nil?  (I'm not proposing that.)

Either closure equality needs an `equal' comparison or it
doesn't, no?  It sounds like the effect of what you're
suggesting would be for `eq' to be the closure equality
test - but only for `add|remove-hook' (?).

What's so wrong with the cases you mention using a symbol?
["work around the use of `equal` in `add/remove-hook`
using a symbol indirection"]

Isn't that, in effect, what all uses of `add|remove-hook'
would have to do after your proposal - either that or
make the use amenable to `eq' in some other way?  (Does
byte-compilation of two structurally equivalent lambda
forms generally produce `eq' results?)

I'm no doubt missing something in the motivation for
this change.  It sounds like sacrificing programmatic
flexibility for some performance optimization.  Can you
elaborate on why this is needed (worth it)?

`set-transient-map' not being able to use `letrec',
because of the `add-hook' equality test, doesn't sound
like a good reason to change `add-hook'.

The point of `equal' is to test with `eq' first, then
if nil go beyond that to test for equal structure.  Of
course, real functions don't have structure, and real
function equality is altogether problematic.  But this
is Lisp, and some Lisp representations of "functions",
at least when interpreted, do have structure (list,
string, vector).

When I look at bug #46326, I see this wrt the problem
(motivation):

"The issue can be mitigated by using a modified version
of minibuffer-with-setup-hook, where I am creating a
symbol and fsetting  instead of adding a lambda directly
via add-hook."

and

"It is the add-hook implementation or more precisely
the minibuffer-with-setup-hook implementation which
is responsible for the excessive allocations."

So it sounds like it's not really about `add-hook';
it's about `minibuffer-with-setup-hook'.

And it looks like your `m-w-s-h' replacement does
just what you'd require everything that uses
`add|remove-hook' to do: replace a lambda form by a
symbol (or equivalent workaround to get `eq'-ness).

You also say this in bug #46326, as possible
alternative remedies:

"1. Replace minibuffer-with-setup-hook with my version
    if you think my version is better and an acceptable fix.
 2. Investigate the reasons why add-hook with priorities
    somehow copies large closures during sorting. This
    is unacceptably costly."

And Stefan says, there:

"IOW I think the better fix is to change
`minibuffer-with-setup-hook` to use an indirection
via a symbol."

And that fix was already pushed.  Why instead now
propose changing `add|remove-hook' to use only `eq'?

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Daniel Mendler]

On 4/24/21 10:12 PM, Drew Adams wrote:
> So instead of just advising users not to use lambda forms
> (which makes sense), you'd make it no longer work at all
> for interpreted lambda forms (except rare cases where
> they might actually be `eq' - e.g., same list structure)?

I agree that it makes sense to use symbols in case you want to add a 
plain function as hook. However often you want to add closures; 
`minibuffer-with-setup-hook`, `set-transient-map` and `eval-after-load` 
are examples where this happens. In order to improve the support for 
closures as hooks, this change is necessary.

It is not reasonable to require every `add-hook` user, who wants to add 
a closure, to introduce a symbol indirection. This is neither obvious 
nor easy. Even a very commonly used macro like 
`minibuffer-with-setup-hook` got this wrong.

Furthermore I would argue there are no plausible scenarios where you 
want to add a closure or lambda as hook and then remove or add it again 
afterwards, but not using the identical object, but only an object which 
is `equal`.

This is more than enough motivation for a change to `eq`.

Daniel

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Drew Adams]

> In order to improve the support for closures as hooks,
> this change is necessary.

Necessary?  Why?  There's no other way to do that?

> It is not reasonable to require every `add-hook` user,
> who wants to add a closure, to introduce a symbol
> indirection.

Why not?  "Symbol indirection" just means setting a
symbol's `symbol-function' to the closure, then using
the symbol.  Why is doing that a big deal?

It's what anyone should do when using `add|remove-hook',
at least interpreted, and interactively.

I ask again: If closure equality is inherently a
problem, why limit the "solution" to `add|remove-hook'?

Shouldn't your argument be that closure equality should
_always_ be tested (testable) using just `eq'?  Is this
really about `add|remove-hook'?  Why would they be
special in this regard?

> Furthermore I would argue there are no plausible scenarios where you
> want to add a closure or lambda as hook and then remove or add it again
> afterwards, but not using the identical object, but only an object
> which is `equal`.

 M-: (add-hook 'foo-hook (lambda () (whatever)))

Of course that's generally not advisable, because if
you then want to remove it interactively you'll have
to provide a lambda that's `equal' (with `M-: M-p',
for example).  But it's common enough, I think.

It's better, e.g., to defun or fset the lambda form, 
and then use the symbol.  But I'm guessing that many
users don't always bother, and they're just careful
to respect `equal' (or they soon learn to be).

Emacs's use of Lisp is also interactive, and often ad
hoc.  If we lose sight of that we lose sight of Emacs.

> This is more than enough motivation for a change to `eq`.

It's your motivation; understood.  Thx.

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Daniel Mendler]

On 4/24/21 11:20 PM, Drew Adams wrote:
> Shouldn't your argument be that closure equality should
> _always_ be tested (testable) using just `eq'?  Is this
> really about `add|remove-hook'?  Why would they be
> special in this regard?

This could be discussed. But a change in equality would be much more 
impactful.

There are reasons why one would want to allow structural equality 
testing for closures. I don't see a problem with it if I opt-in 
explicitly by using `equal`. It is still the wrong equality for 
`add/remove-hook` which should be robust. And currently it is not.

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Stefan Monnier]

> So instead of just advising users not to use lambda forms
> (which makes sense), you'd make it no longer work at all
> for interpreted lambda forms (except rare cases where
> they might actually be `eq' - e.g., same list structure)?

It would still work for lambda forms, just differently (arguably, in
a way that's more often right than the current way).

> Perhaps `equal' can be fixed to do something better with closures?

There's no magic: `equal` has to check the structural equality, so it
has to recurse through the whole structure, including all the
closed-over variables to which it refers.

> E.g., if the `eq' test in `equal' fails for a
> closure arg then return nil?  (I'm not proposing that.)

That's what using `eq` would do, so you seem to agree with
Daniel's proposal here.

> And Stefan says, there:
>
> "IOW I think the better fix is to change
> `minibuffer-with-setup-hook` to use an indirection
> via a symbol."

That was written in the context of a fix that needs to work *now*,
whereas changing `add/remove-hook` to use `eq` tests can at best be
a longer term goal.


        Stefan

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Daniel Mendler]

On 4/25/21 12:30 AM, Stefan Monnier wrote:
>> Perhaps `equal' can be fixed to do something better with closures?
> 
> There's no magic: `equal` has to check the structural equality, so it
> has to recurse through the whole structure, including all the
> closed-over variables to which it refers.

Well, structural equality on closures is an arbitrary choice. One could 
simply refuse to compare closures structurally and treat them as opaque 
objects. The structural equality does not even perform alpha conversion. 
This is probably due to how binding works in Elisp?

(equal (lambda (x) x) (lambda (y) y))

Daniel

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Stefan Monnier]

>>> Perhaps `equal' can be fixed to do something better with closures?
>> There's no magic: `equal` has to check the structural equality, so it
>> has to recurse through the whole structure, including all the
>> closed-over variables to which it refers.
> Well, structural equality on closures is an arbitrary choice. One could
> simply refuse to compare closures structurally and treat them as opaque
> objects.

Currently we could do that for byte-compiled closures but not for
interpreted ones.

> The structural equality does not even perform alpha conversion.

It partly does actually, by accident, when the code is byte-compiled,
but only for the variables internal to the function and not for the
formal arguments (because they "escape" into the docstring).

Hopefully this will be "broken" at some point, when we add enough debug
info to bytecode to be able to find the value of (and set) local
variables by name.

> This is probably due to how binding works in Elisp?
>
> (equal (lambda (x) x) (lambda (y) y))

Equality on functions is fundamentally undecidable and it's nigh-on
impossible to provide a sane and well-defined "approximation" of it
either (at least not without significantly restricting the set of
optimizations that the compiler can be allowed to perform).

The upside is that this fundamental problem was the motivation for the
development of type classes in Haskell which are a great feature
(nowadays used in most proof assistants and in several other programming
languages such as Scala and Rust).


        Stefan

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Daniel Mendler]

On 4/25/21 1:04 AM, Stefan Monnier wrote:
>> The structural equality does not even perform alpha conversion.
> 
> It partly does actually, by accident, when the code is byte-compiled,
> but only for the variables internal to the function and not for the
> formal arguments (because they "escape" into the docstring).
> 
> Hopefully this will be "broken" at some point, when we add enough debug
> info to bytecode to be able to find the value of (and set) local
> variables by name.

Hopefully.

> Equality on functions is fundamentally undecidable and it's nigh-on
> impossible to provide a sane and well-defined "approximation" of it
> either (at least not without significantly restricting the set of
> optimizations that the compiler can be allowed to perform).

Yes, for structural equality of functions there seem to be no other sane 
choices than the equality of the representation, maybe with additional 
alpha conversion. It would be okay to use object identity.

> The upside is that this fundamental problem was the motivation for the
> development of type classes in Haskell which are a great feature
> (nowadays used in most proof assistants and in several other programming
> languages such as Scala and Rust).

Indeed. The Eq type class simply forbids equality for functions. But in 
proof assistants the equality problem strikes again, when checking if 
two functions are definitionally equal. And then there is this whole 
equality rabbit hole in type theory.

Daniel

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Drew Adams]

> >>> Perhaps `equal' can be fixed to do something better with closures?
> >>
> >> There's no magic: `equal` has to check the structural equality, so it
> >> has to recurse through the whole structure, including all the
> >> closed-over variables to which it refers.
> >
> > Well, structural equality on closures is an arbitrary choice. One
> > could simply refuse to compare closures structurally and treat
> > them as opaque objects.
> 
> Currently we could do that for byte-compiled closures
> but not for interpreted ones.

Also what I hinted at (I didn't know whether we might
in fact already do that), and why I spoke specifically
of supporting also interpreted code.

Emacs users often use Lisp as part of their interaction
with the editor, so: interpreted code.  Lose that and
we lose Emacs.  IMHO.

And what would we be losing it for?  Some performance
gain for closures used as hooks?  If you're convinced
of the need or desirability of such a change...

To be clear, by lose that I mean the ease of using Lisp
interactively, which today still means interpretation.

You'll say that you'll replace all interpretation by
on-the-fly jitty compilation...  That's also why I wrote
that hint about possibly doing something better with
closures wrt `equal'.  We're not there - not by a long
shot.  And doing what's been proposed here doesn't get
us there. 

> Equality on functions is fundamentally undecidable and it's nigh-on
> impossible to provide a sane and well-defined "approximation" of it
> either (at least not without significantly restricting the set of
> optimizations that the compiler can be allowed to perform).

100% agreement.  And there's no need for it, for Emacs.

> The upside is that this fundamental problem was the motivation for the
> development of type classes in Haskell which are a great feature
> (nowadays used in most proof assistants and in several other
> programming languages such as Scala and Rust).

Meanwhile, back at the Emacs ranch, for actual users...

(Not that a Haskell Emacs wouldn't be an interesting
project.  Please go for it.)

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Stefan Monnier]

Drew, what does this rant have to do with add-hook?
Please stay focused.

The only relevant thing I saw in there was:

>> Equality on functions is fundamentally undecidable [...]
> 100% agreement.  And there's no need for it, for Emacs.

Huh, without it, there's no `remove-hook`.



        Stefan


Drew Adams [2021-04-25 01:16:09] wrote:
> Also what I hinted at (I didn't know whether we might
> in fact already do that), and why I spoke specifically
> of supporting also interpreted code.
>
> Emacs users often use Lisp as part of their interaction
> with the editor, so: interpreted code.  Lose that and
> we lose Emacs.  IMHO.
>
> And what would we be losing it for?  Some performance
> gain for closures used as hooks?  If you're convinced
> of the need or desirability of such a change...
>
> To be clear, by lose that I mean the ease of using Lisp
> interactively, which today still means interpretation.
>
> You'll say that you'll replace all interpretation by
> on-the-fly jitty compilation...  That's also why I wrote
> that hint about possibly doing something better with
> closures wrt `equal'.  We're not there - not by a long
> shot.  And doing what's been proposed here doesn't get
> us there. 
>
>> Equality on functions is fundamentally undecidable and it's nigh-on
>> impossible to provide a sane and well-defined "approximation" of it
>> either (at least not without significantly restricting the set of
>> optimizations that the compiler can be allowed to perform).
>
> 100% agreement.  And there's no need for it, for Emacs.
>
>> The upside is that this fundamental problem was the motivation for the
>> development of type classes in Haskell which are a great feature
>> (nowadays used in most proof assistants and in several other
>> programming languages such as Scala and Rust).
>
> Meanwhile, back at the Emacs ranch, for actual users...
>
> (Not that a Haskell Emacs wouldn't be an interesting
> project.  Please go for it.)

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Drew Adams]

> Drew, what does this rant have to do with add-hook?
> Please stay focused.

Please read what I wrote.  I see no rant.

> The only relevant thing I saw in there was:
> 
> >> Equality on functions is fundamentally undecidable [...]
> >
> > 100% agreement.  And there's no need for it, for Emacs.
> 
> Huh, without it, there's no `remove-hook`.

Read what I said about _real_ functions, which is whose
equality is undecidable.  As contrasted with the equality
of Lisp "functions", represented as symbols, strings, or
lambda forms.  The latter is certainly decidable, and
it's what we use in `remove-hook' to decide.

 "Of course, real functions don't have structure, and real
  function equality is altogether problematic.  But this
  is Lisp, and some Lisp representations of "functions",
  at least when interpreted, do have structure (list,
  string, vector)."

I agreed that equality on functions is undecidable.
I added that (fortunately) we can get by with a lesser
test of just our simple representations of functions.
We can and we do.

Our use of `equal' in `remove-hook' to test Lisp
"function" equality has nothing to do with the
undecidability of (real) function equality.  There's
no need, in Emacs, for "equality on functions", which
"is fundamentally undecidable".

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Stefan Monnier]

> Read what I said about _real_ functions, which is whose
> equality is undecidable.

`add/remove-hook` sadly lives in the real world.

> As contrasted with the equality of Lisp "functions", represented as
> symbols, strings, or lambda forms.

Strings aren't used to represent functions in Lisp, and the case of
lambda-forms is the case that's actually causing real performance
problems in add/remove-hook (both interpreted and compiled).

So, yes, symbols is the only option remaining, to enforce the `eq`
semantics which seems to be at least as often what we want and doesn't
suffer from performance problems.

> The latter is certainly decidable, and it's what we use in
> `remove-hook' to decide.

My comment about decidability was just a side remark that doesn't really
matter for this decision.  I now regret mentioning it.


        Stefan

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Drew Adams]

> The structural equality does not even perform alpha
> conversion.
> This is probably due to how binding works in Elisp?
> 
> (equal (lambda (x) x) (lambda (y) y))

Right.  Which is, again, why we advise users not to
use lambda forms as hook functions.  But some do,
and if they do they soon learn that if they want to
remove the hook they need to provide a sexp that's
`equal' - a lambda form as an `equal' list.

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Drew Adams]

> > So instead of just advising users not to use lambda forms
> > (which makes sense), you'd make it no longer work at all
> > for interpreted lambda forms (except rare cases where
> > they might actually be `eq' - e.g., same list structure)?
> 
> It would still work for lambda forms, just differently (arguably,
> in a way that's more often right than the current way).

Please elaborate.  Comparing lambda forms using `eq'?
Not clear to me how that works in the general case.

 (eq (lambda () foo) (lambda () foo)) ?

I don't see that it works at all, let alone works more
often than the current way:
 (equal (lambda () foo) (lambda () foo))

> > Perhaps `equal' can be fixed to do something better with closures?
> 
> There's no magic: `equal` has to check the structural equality, so it
> has to recurse through the whole structure, including all the
> closed-over variables to which it refers.

That's what I was hinting.  I don't see the magic either.

> > E.g., if the `eq' test in `equal' fails for a
> > closure arg then return nil?  (I'm not proposing that.)
> 
> That's what using `eq` would do, so you seem to agree with
> Daniel's proposal here.

Not at all.  I was saying that that's what I understand
him to be proposing, in the context of `add-hook'.

If that made sense for that case (which it doesn't, to
me) then I should think it would make sense in general
(which I don't think it does - no such magic).

How does comparing closures with `eq' makes sense for
`add-hook' but not in general?  That was the question.
I don't see that it makes sense for either.

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Stefan Monnier]

> Please elaborate.  Comparing lambda forms using `eq'?
> Not clear to me how that works in the general case.
>
>  (eq (lambda () foo) (lambda () foo)) ?
>
> I don't see that it works at all, let alone works more
> often than the current way:
>  (equal (lambda () foo) (lambda () foo))

IOW, you don't have an opinion either way on the proposed change of semantics.


        Stefan

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Drew Adams]

>> > > you'd make it no longer work at all for
>> > > interpreted lambda forms (except rare cases where they
>> > > might actually be `eq' - e.g., same list structure)?
>> > 
>> > It would still work for lambda forms, just differently (arguably,
>> > in a way that's more often right than the current way).
> >
> > Please elaborate.  Comparing lambda forms using `eq'?
> > Not clear to me how that works in the general case.
> >
> >  (eq (lambda () foo) (lambda () foo)) ?
> >
> > I don't see that it works at all, let alone works more
> > often than the current way:
> >  (equal (lambda () foo) (lambda () foo))
> 
> IOW, you don't have an opinion either way on the 
> proposed change of semantics.

Seems to be your favorite way of (not) communicating:
saying that I have nothing to say.

How about actually elaborating: Tell us how using
`eq' would enable the interpreter to test equality
of lambda forms in the general case (not shared list
structure)?

How would using `eq' "still work for lambda forms,
just differently (arguably, in a way that's more
often right than the current way)"?  Forgive me for
not understanding what you mean by that.
___

As for my opinion on the proposed change: I haven't
seen a good argument for using `eq' instead of `equal'
to test for equality in `add|remove-hook' (in the code:
`memq' instead of `member').

I gave good arguments for continuing to use `equal'.
Emacs users use the Elisp interpreter interactively,
and they do use lambda forms with `add|remove-hook',
even though that's not a great idea.  `eq' doesn't
cut the mustard at all, for such use.

Barring a good argument for using `eq', I'm not in
favor of such a change.  Given a good argument, I
might change my mind.  Clear enough?

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Daniel Mendler]

On 4/25/21 6:57 AM, Drew Adams wrote:
> As for my opinion on the proposed change: I haven't
> seen a good argument for using `eq' instead of `equal'
> to test for equality in `add|remove-hook' (in the code:
> `memq' instead of `member').
> 
> I gave good arguments for continuing to use `equal'.
> Emacs users use the Elisp interpreter interactively,
> and they do use lambda forms with `add|remove-hook',
> even though that's not a great idea.  `eq' doesn't
> cut the mustard at all, for such use.
> 
> Barring a good argument for using `eq', I'm not in
> favor of such a change.  Given a good argument, I
> might change my mind.  Clear enough?

Drew, it seems to you don't read the arguments which have been made and 
just stick to your opinion of "not applying changes".

You argue that `equal` is better since the user can
then add/remove literally written lambdas interactively. We both agree 
that this is not a recommended or reasonable usage of the hook 
functionality.

I argue that `eq` is better if you use `add/remove-hook` in a perfectly 
valid way, adding cyclic/large closures programmatically. This is an 
accepted practice, since we add closures in `minibuffer-with-setup-hook` 
and at other places. Furthermore it is a significantly more important 
use case than the interactive use case you put forward as argument.

Now we can stay with the borked semantics of `add/remove-hook` and 
continue to use `equal` to cater for your example and require all the 
reasonable programmatic usages of `add/remove-hook` to go through the 
totally unnecessary symbol indirection.

I don't see how your argument holds up here.

Daniel

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Stefan Monnier]

> Seems to be your favorite way of (not) communicating:
> saying that I have nothing to say.

Because you've written several pages of text in this thread without
giving any concrete example arguing one way or another.  I find this to
be a very unpleasant way of communicating.

Please stay focused.



        Stefan

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Lars Ingebrigtsen]

Daniel Mendler <mail@daniel-mendler.de> writes:

> It would be good to change `add/remove-hook` such that it only relies
> on `eq` to test hook equality. Then the symbol indirection workarounds
> can be avoided.
>
> However making such a change directly can lead to subtle
> breakage. Perhaps one could introduce some deprecation behavior first,
> before making the final change to `eq`.  If a hook is added/removed
> and the added/removed object is not found via `eq` but found via
> `equal`, show a deprecation warning?

There are two issues here:

1) Should `add/remove-hook' even attempt to do uniqueness checks when
adding/removing things that aren't symbols (or more generally, eq-able
things), and

2) Should `add-hook' disallow adding such things?

Today, it's super common for people to say

(add-hook 'some-hook (lambda () ...))

in their .emacs files.  This isn't because they have any expectation
that add-hook does this uniquely, or that remove-hook will work, but
because that's just what they think they should do.

So I think 2) is out of the question -- we can't deprecate this, and we
can't issue any warnings about doing it this way.  (Even if it's
"wrong" -- it's just not feasible to disallow this.)

And since 2) isn't possible, I don't really think 1) is possible
either.  People do `M-x eval-buffer' their .emacs files, and since we're
using `equal' here, this happens to work -- almost by accident.

(If they change the lambda, then they get two instances of the lambda in
the hook, so it's "wrong", but it's so common.)

So I'm not sure I see any way forward with this.  Would adding a new
pair of functions (that are `eq' only) help in any way?

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

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Daniel Mendler]

On 5/2/21 11:09 AM, Lars Ingebrigtsen wrote:> And since 2) isn't
possible, I don't really think 1) is possible
> either.  People do `M-x eval-buffer' their .emacs files, and since we're
> using `equal' here, this happens to work -- almost by accident.

I understand. This argument is pretty similar to Drew's argument, but
realistic. The problem is that people expect idempotence when evaluating
their ".emacs". This property breaks as soon as we require `eq` and if
we want to retain this behavior we have no alternative than keeping the
current behavior.

From my experience, relying on this accidental idempotence when
evaluating buffers almost ever breaks in more intricate scenarios, so I
am usually not relying on this behavior. But making it break more often
is not a good idea, in particular in this important use case of the
".emacs", which is one of the points where people get first contact with
Elisp.

There are the following resolutions from my perspective:

1. Do almost nothing, but document the behavior of `add/remove-hook`
more precisely. Add a warning, that the functions should not be used
with large/cyclic closures and an symbol indirection should be used in
such scenarios. Generally it should be recommended to add symbols.
2. As you propose we could either add new `add/remove-hook-eq` variants.
3. Add an argument to the `add/remove-hook` functions, which allows to
turn on `eq` equality.

I assume the problem is mostly gone now due to the patches applied
previously by Stefan to `add/remove-hook`, which removed the depth
information to avoid leaks. In particular the problem with the original
`minibuffer-with-setup-hook` I observed was gone after the changes to
`add/remove-hook`. So my favorite resolution is 1.

Daniel

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Lars Ingebrigtsen]

Daniel Mendler <mail@daniel-mendler.de> writes:

> 1. Do almost nothing, but document the behavior of `add/remove-hook`
> more precisely. Add a warning, that the functions should not be used
> with large/cyclic closures and an symbol indirection should be used in
> such scenarios. Generally it should be recommended to add symbols.

I've now added such a recommendation to the add-hook doc string in Emacs
28.

> 2. As you propose we could either add new `add/remove-hook-eq` variants.
> 3. Add an argument to the `add/remove-hook` functions, which allows to
> turn on `eq` equality.

Adding another argument to the functions doesn't seem all that
attractive...  but...  neither does adding a new pair of functions,
really...

I guess the doc change might have to suffice here.

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

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Olivier Certner]

Hi,

> > either.  People do `M-x eval-buffer' their .emacs files, and since we're
> > using `equal' here, this happens to work -- almost by accident.
> 
> I understand. This argument is pretty similar to Drew's argument, but
> realistic. The problem is that people expect idempotence when evaluating
> their ".emacs". This property breaks as soon as we require `eq` and if
> we want to retain this behavior we have no alternative than keeping the
> current behavior.

I agree with all this. We should aim for idempotence, as much as practical.

Just wanted to add that there is really no other simple general way out than 
naming lambdas (by whichever mechanism this is done) and matching on names. 
Indeed, even with `equal', and no changes in some lambda's code, idempotence 
is not guaranteed: Just use in the code some macro that calls `gensym'. Each 
evaluation of `lambda' will then raise different functions by `equal', since 
their code differs.

-- 
Olivier Certner

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> If you ask me: that problem exists mainly for end users and their init
> files, it's not that we have a problem in Emacs.  Instead we should try
> to provide a better tool for users to add stuff to hooks conveniently.
>
> Since the problem of identification is not trivial - how about forcing
> the user to specify a name?  How about a macro like
>
> (user-add-hook foo-hook my-configure-foo-keybindings
>    code...) ?

Some other languages have the ability to add names to anonymous
functions (yes, a contradiction in terms) to aid debugging and the like.
We could also extend our `lambda' with an optional name -- it seems like
that could be generally useful beyond this particular use case -- and
then this new add-hook function could refuse to accept name-less
lambdas...

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

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> [...]
> and then this new add-hook function could refuse to accept name-less
> lambdas...

I must say I'm skeptical: in the case of `add-hook', this would be not
much different from forcing the use of symbols (aka named normal
functions).  Why add another name space (and the complexity this
implies) - where is one too limiting?


Michael.

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> [...]
>> and then this new add-hook function could refuse to accept name-less
>> lambdas...
>
> I must say I'm skeptical: in the case of `add-hook', this would be not
> much different from forcing the use of symbols (aka named normal
> functions).  Why add another name space (and the complexity this
> implies) - where is one too limiting?

These would not be normal functions, but lambdas with an id.

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

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> > I must say I'm skeptical: in the case of `add-hook', this would be not
> > much different from forcing the use of symbols (aka named normal
> > functions).  Why add another name space (and the complexity this
> > implies) - where is one too limiting?
>
> These would not be normal functions, but lambdas with an id.

That's the part I understood.  But what advantage would these offer,
compared to standard naming?

Michael.

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> That's the part I understood.  But what advantage would these offer,
> compared to standard naming?

By "standard naming" you mean "define a function, and then use it in
`add-hook'?"  Experience shows that people avoid doing so (for various
reason), so creating a new form to cater to this use case makes sense.

Especially now when people are using lexical binding more, when it would
make sense to allow people to add/remove/change these closures in hooks
in a convenient manner.

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

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Drew Adams]

> > But what advantage would these offer,
> > compared to standard naming?
> 
> By "standard naming" you mean "define a function,
> and then use it in `add-hook'?"  Experience shows
> that people avoid doing so (for various reason),
> so creating a new form to cater to this use case
> makes sense.

FWIW, I disagree.  There's no real need for such
a "new form".

I think the use case you've imagined for it is a
s t r e t c h .  Users will just as likely avoid
making any of your new forms as they avoid named
functions now.

The "various reasons" amount to laziness (which
is often the right approach), and that will be
the same reason for not naming lambdas in the
new way you envision.  There's a reason lambdas
are anonymous functions.

Here's maybe another hint that it isn't needed:
Has anyone ever requested such a feature?

And if there really were such a need, you could
just check for non-nil `eq'uality of a lambda's
doc string, instead of adding yet another "new
form" that will likely ~never be used.

And even that feature - using a doc string with
a lambda - is rarely used.  As the doc string
of `lambda' itself says:

   But documentation strings are usually not
   useful in nameless functions.

Let's not add another ~useless "new form" for
lambdas.  They already have a doc string, which
I'm guessing is as much a name as what you have
in mind.

If you really want to test with `eq', just use
`eq' together with function `documentation'.

That'll save you implementing the same kind of
thing for yet another "name".  Same test: (1)
are both "names" present (non-nil), and (2) if
if so, are they `eq'?

And the same problem will persist: few will
ever bother to provide such a "name", even if
it might help them with `add|remove-hook'.

You have a solution in search of a problem, I
think.  Users just learn the hard way (but the
doc of `add|remove-hook' could be improved to
help them learn) that they don't want to use
lambdas as hook functions.

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Michael Heerdegen]

Drew,

these are exactly my thoughts so far, thanks for writing these down.  I
only see disadvantages, and that's why I asked for any concrete
advantages I might have missed.

Michael.

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

> FWIW, I disagree.  There's no real need for such
> a "new form".
>
> I think the use case you've imagined for it is a
> s t r e t c h .  Users will just as likely avoid
> making any of your new forms as they avoid named
> functions now.

Languages that have added naming to their lambdas mostly do so because
it eases debugging and the like.  We're going to see more and more code
in Emacs that's based on closures, which means that our backtraces will
just contain a bunch of unnamed lambda forms, called from asynchronous
contexts, with no easy way for xref to jump to the location(s) where
they were defined, and profiling will have the same problem.

It's true that users will commonly avoid naming their lambdas, but
library makers (in Javascript, which has named lambdas) often do put
them in -- to aid debugging and profiling.

My suggestion, though, for the particular usage in hooks we were
discussing here, was that the new `push-hook' (or whatever) function
would disallow using anonymous lambdas, thereby forcing users to name
their lambdas (in this context).

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

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

People are proposing a lot of work on a little wrinkle that isn't worth
much effort.  There are other changes we could make that would
really help users do editing.

Here's an easy way to help users avoid such collisions: to say
they should include a name in each anonymous lambda that they put on a
hook in their init files -- a name based on the user's name.

Adding a name to a lambda is easy.  Just write the symbol after the
argument list (and doc string and interactive spec, if any), like
this:

   (lambda () rms-init-3 (glum-mode 1))

The mame will distinguish it from any otherwise-identical function
such as

   (lambda () j-r-gensym (glum-mode 1))

because they will not be equal.

Thus, the only change needed is in documentation.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Drew Adams]

> People are proposing a lot of work on a little wrinkle that isn't worth
> much effort.  There are other changes we could make that would
> really help users do editing.

Agreed.

> Here's an easy way to help users avoid such collisions: to say
> they should include a name in each anonymous lambda that they put on a
> hook in their init files -- a name based on the user's name.
> 
> Adding a name to a lambda is easy.  Just write the symbol after the
> argument list (and doc string and interactive spec, if any), like
> this: (lambda () rms-init-3 (glum-mode 1))
> 
> The mame will distinguish it from any otherwise-identical function
> such as (lambda () j-r-gensym (glum-mode 1))
> because they will not be equal.
> 
> Thus, the only change needed is in documentation.

I don't think that solves the problem ("little
wrinkle") raised by the bug report.  (But I agree
that the only change needed is documentation - just
advise users not to use anonymous functions as hook
functions.)

IIUC, the problem is not to _distinguish_ the
lambda.  It's kinda the opposite.  It's to let you
just type a lambda expression and have
`add|remove-hook' recognize it as being the _same_
function you gave it before.

Currently the test used for a lambda is `equal',
meaning that to remove a lambda you've added with
`add-hook' you need to provide a lambda (a list)
that's `equal' to the one you added originally.
(For a named function the function symbol is
simply tested using `eq'.)

Those who proposed solving this looked for a way
to enable an `eq' test for a lambda.  Lars
proposed to have users "name" anonymous functions
they put on hooks, and (presumably) to update the
`add|remove-hook' code to test for such a name
(if present) using `eq'.  (If a given lambda is
unnamed then the wrinkle is still a wrinkle.)

If your suggested way of naming a lambda were
used then the same approach would presumably
apply: change the `add|remove-hook' code to test
for such a name (if present) using `eq'.  And it
has the same problem: no good if not used.
___

I mentioned that if someone really wanted to go
down such a road (not something I'd encourage),
then there's no need to add a "name" possibility
- just use the doc-string possibility for lambdas.
The test (when a doc string is present) could
then be to use `eq' plus function `documentation'.

(That's no different from the test needed for a
lambda with a name - whether named your way or
another way: need to test for the presence of a
name, and if present test it with `eq'.  The test
for a name is analogous to the test for a doc
string with function `documentation'.)

But I and some others also think the whole attempt
to provide more or less `eq' testing for lambdas
is maybe misguided, and that users who today don't
bother to use a named function will likely also
not bother to name the lambdas they use on hooks.
(That goes also for "naming" using a doc string.)

We should, however (IMO) advise users not to use
anonymous functions as hook functions, in general,
and let them know why.  As you said, "the only
change needed is in documentation."

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> But I and some others also think the whole attempt
> to provide more or less `eq' testing for lambdas
> is maybe misguided

The original complaint was about `equal' testing consuming a lot of
time.  That might be a valid complaint.  `eq' testing doesn't create
problems unless for those not adding to hooks correctly.  So moving to
`eq' and providing better help would make sense to me.

Michael.

bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

It would not be hard to write a function to remove, from a hook,
any lambda expression that has a certain symbol as its name.
(memq SYMBOL LAMBDAEXP) can test whether LAMBDAEXP has the name SYMBOL.

We could make remove-hook do this if the supplied argument is a symbol
with no function definition, or whatever other interface is desirable.

It wouldn't be hard to do this with doc strings either.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Arthur Miller]

Richard Stallman <rms@gnu.org> writes:

> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
> People are proposing a lot of work on a little wrinkle that isn't worth
> much effort.  There are other changes we could make that would
> really help users do editing.
>
> Here's an easy way to help users avoid such collisions: to say
> they should include a name in each anonymous lambda that they put on a
> hook in their init files -- a name based on the user's name.
>
> Adding a name to a lambda is easy.  Just write the symbol after the
> argument list (and doc string and interactive spec, if any), like
> this:
>
>    (lambda () rms-init-3 (glum-mode 1))
>
> The mame will distinguish it from any otherwise-identical function
> such as
>
>    (lambda () j-r-gensym (glum-mode 1))
>
> because they will not be equal.
>
> Thus, the only change needed is in documentation.

If they are equal, why do they need to be distinguished?

Also, if we name lambdas, arent they becoming just as functions? What is
the advantage of named lambda over named function?

Sorry I am not so fluent with elisp yet, so I am trying to understand.

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > If they are equal, why do they need to be distinguished?

If file A adds such a hook, then file B adds one that is the same,
then you unload file B, testing for "the same hook" would
leave none.  But A won't work right without its hook.

My scheme would put on two distinct but equivalent hooks,
then delete one of them, leaving the other in place.
In some cases, that's what you want.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Arthur Miller]

Richard Stallman <rms@gnu.org> writes:

> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
>   > If they are equal, why do they need to be distinguished?
>
> If file A adds such a hook, then file B adds one that is the same,
> then you unload file B, testing for "the same hook" would
> leave none.  But A won't work right without its hook.
>
> My scheme would put on two distinct but equivalent hooks,
> then delete one of them, leaving the other in place.
> In some cases, that's what you want.

Ok, I understand now. Yes, indeed, put name tag would be one solution to
the problem. It trades memory for cpu time.

bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Some other languages have the ability to add names to anonymous
> functions (yes, a contradiction in terms) to aid debugging and the like.
> We could also extend our `lambda' with an optional name -- it seems like
> that could be generally useful beyond this particular use case -- and
> then this new add-hook function could refuse to accept name-less
> lambdas...

BTW, I think you can have something like that by misusing advice,
e.g. (example at the end):

#+begin_src emacs-lisp
(defalias 'function-name-advice #'identity) ;just a tag

(defun named-function (f name)
  (unless (and (functionp f)
               (not (symbolp f)))
    (error "Not an anonymous function"))
  (when (advice-function-member-p 'function-name-advice f)
    (error "Already has a name"))
  (add-function :filter-return (var f) 'function-name-advice `((name . ,name)))
  f)

(defun function-name (f)
  (and (advice-function-member-p 'function-name-advice f)
       (cdr (assoc 'name (advice--props f)))))

;; Example:
(setq my-test-lambda (lambda (n) (+ 2 n)))
(setq my-named-test-lambda
      (named-function my-test-lambda "Moon Child")) ;; => [bytecode lambda]

(funcall my-named-test-lambda 3) ;; => 5 (works)

(function-name my-named-test-lambda) ;; => "Moon Child"
#+end_src


Michael.