unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
@ 2021-04-24 12:11 Daniel Mendler
  2021-04-24 20:12 ` bug#47992: [External] : " Drew Adams
  2021-05-02  9:09 ` Lars Ingebrigtsen
  0 siblings, 2 replies; 22+ messages in thread
From: Daniel Mendler @ 2021-04-24 12:11 UTC (permalink / raw)
  To: 47992; +Cc: monnier, jakanakaevangeli

(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?





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 12:11 bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook` Daniel Mendler
@ 2021-04-24 20:12 ` Drew Adams
  2021-04-24 20:23   ` Daniel Mendler
  2021-04-24 22:30   ` Stefan Monnier
  2021-05-02  9:09 ` Lars Ingebrigtsen
  1 sibling, 2 replies; 22+ messages in thread
From: Drew Adams @ 2021-04-24 20:12 UTC (permalink / raw)
  To: Daniel Mendler, 47992; +Cc: monnier, jakanakaevangeli

> (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'?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 20:12 ` bug#47992: [External] : " Drew Adams
@ 2021-04-24 20:23   ` Daniel Mendler
  2021-04-24 21:20     ` Drew Adams
  2021-04-24 22:30   ` Stefan Monnier
  1 sibling, 1 reply; 22+ messages in thread
From: Daniel Mendler @ 2021-04-24 20:23 UTC (permalink / raw)
  To: Drew Adams, 47992; +Cc: monnier, jakanakaevangeli

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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 20:23   ` Daniel Mendler
@ 2021-04-24 21:20     ` Drew Adams
  2021-04-24 21:34       ` Daniel Mendler
  0 siblings, 1 reply; 22+ messages in thread
From: Drew Adams @ 2021-04-24 21:20 UTC (permalink / raw)
  To: Daniel Mendler, 47992; +Cc: monnier, jakanakaevangeli

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 21:20     ` Drew Adams
@ 2021-04-24 21:34       ` Daniel Mendler
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Mendler @ 2021-04-24 21:34 UTC (permalink / raw)
  To: Drew Adams, 47992; +Cc: monnier, jakanakaevangeli

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.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 20:12 ` bug#47992: [External] : " Drew Adams
  2021-04-24 20:23   ` Daniel Mendler
@ 2021-04-24 22:30   ` Stefan Monnier
  2021-04-24 22:38     ` Daniel Mendler
  2021-04-25  1:23     ` Drew Adams
  1 sibling, 2 replies; 22+ messages in thread
From: Stefan Monnier @ 2021-04-24 22:30 UTC (permalink / raw)
  To: Drew Adams; +Cc: Daniel Mendler, 47992, jakanakaevangeli

> 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






^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 22:30   ` Stefan Monnier
@ 2021-04-24 22:38     ` Daniel Mendler
  2021-04-24 23:04       ` Stefan Monnier
  2021-04-25  1:16       ` Drew Adams
  2021-04-25  1:23     ` Drew Adams
  1 sibling, 2 replies; 22+ messages in thread
From: Daniel Mendler @ 2021-04-24 22:38 UTC (permalink / raw)
  To: Stefan Monnier, Drew Adams; +Cc: 47992, jakanakaevangeli

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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 22:38     ` Daniel Mendler
@ 2021-04-24 23:04       ` Stefan Monnier
  2021-04-24 23:38         ` Daniel Mendler
  2021-04-25  1:16         ` Drew Adams
  2021-04-25  1:16       ` Drew Adams
  1 sibling, 2 replies; 22+ messages in thread
From: Stefan Monnier @ 2021-04-24 23:04 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: 47992, jakanakaevangeli

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






^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 23:04       ` Stefan Monnier
@ 2021-04-24 23:38         ` Daniel Mendler
  2021-04-25  1:16         ` Drew Adams
  1 sibling, 0 replies; 22+ messages in thread
From: Daniel Mendler @ 2021-04-24 23:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 47992, jakanakaevangeli

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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 23:04       ` Stefan Monnier
  2021-04-24 23:38         ` Daniel Mendler
@ 2021-04-25  1:16         ` Drew Adams
  2021-04-25  3:08           ` Stefan Monnier
  1 sibling, 1 reply; 22+ messages in thread
From: Drew Adams @ 2021-04-25  1:16 UTC (permalink / raw)
  To: Stefan Monnier, Daniel Mendler; +Cc: 47992, jakanakaevangeli

> >>> 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.)





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 22:38     ` Daniel Mendler
  2021-04-24 23:04       ` Stefan Monnier
@ 2021-04-25  1:16       ` Drew Adams
  1 sibling, 0 replies; 22+ messages in thread
From: Drew Adams @ 2021-04-25  1:16 UTC (permalink / raw)
  To: Daniel Mendler, Stefan Monnier; +Cc: 47992, jakanakaevangeli

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 22:30   ` Stefan Monnier
  2021-04-24 22:38     ` Daniel Mendler
@ 2021-04-25  1:23     ` Drew Adams
  2021-04-25  3:10       ` Stefan Monnier
  1 sibling, 1 reply; 22+ messages in thread
From: Drew Adams @ 2021-04-25  1:23 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Daniel Mendler, 47992, jakanakaevangeli

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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-25  1:16         ` Drew Adams
@ 2021-04-25  3:08           ` Stefan Monnier
  2021-04-25  4:57             ` Drew Adams
  0 siblings, 1 reply; 22+ messages in thread
From: Stefan Monnier @ 2021-04-25  3:08 UTC (permalink / raw)
  To: Drew Adams; +Cc: Daniel Mendler, 47992, jakanakaevangeli

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.)






^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-25  1:23     ` Drew Adams
@ 2021-04-25  3:10       ` Stefan Monnier
  2021-04-25  4:57         ` Drew Adams
  0 siblings, 1 reply; 22+ messages in thread
From: Stefan Monnier @ 2021-04-25  3:10 UTC (permalink / raw)
  To: Drew Adams; +Cc: Daniel Mendler, 47992, jakanakaevangeli

> 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






^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-25  3:08           ` Stefan Monnier
@ 2021-04-25  4:57             ` Drew Adams
  2021-04-25 13:52               ` Stefan Monnier
  0 siblings, 1 reply; 22+ messages in thread
From: Drew Adams @ 2021-04-25  4:57 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Daniel Mendler, 47992, jakanakaevangeli

> 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".





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-25  3:10       ` Stefan Monnier
@ 2021-04-25  4:57         ` Drew Adams
  2021-04-25 10:33           ` Daniel Mendler
  2021-04-25 13:56           ` Stefan Monnier
  0 siblings, 2 replies; 22+ messages in thread
From: Drew Adams @ 2021-04-25  4:57 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Daniel Mendler, 47992, jakanakaevangeli

>> > > 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?





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-25  4:57         ` Drew Adams
@ 2021-04-25 10:33           ` Daniel Mendler
  2021-04-25 13:56           ` Stefan Monnier
  1 sibling, 0 replies; 22+ messages in thread
From: Daniel Mendler @ 2021-04-25 10:33 UTC (permalink / raw)
  To: Drew Adams, Stefan Monnier; +Cc: 47992, jakanakaevangeli

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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-25  4:57             ` Drew Adams
@ 2021-04-25 13:52               ` Stefan Monnier
  0 siblings, 0 replies; 22+ messages in thread
From: Stefan Monnier @ 2021-04-25 13:52 UTC (permalink / raw)
  To: Drew Adams; +Cc: Daniel Mendler, 47992, jakanakaevangeli

> 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






^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: [External] : bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-25  4:57         ` Drew Adams
  2021-04-25 10:33           ` Daniel Mendler
@ 2021-04-25 13:56           ` Stefan Monnier
  1 sibling, 0 replies; 22+ messages in thread
From: Stefan Monnier @ 2021-04-25 13:56 UTC (permalink / raw)
  To: Drew Adams; +Cc: Daniel Mendler, 47992, jakanakaevangeli

> 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






^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-04-24 12:11 bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook` Daniel Mendler
  2021-04-24 20:12 ` bug#47992: [External] : " Drew Adams
@ 2021-05-02  9:09 ` Lars Ingebrigtsen
  2021-05-02 10:37   ` Daniel Mendler
  1 sibling, 1 reply; 22+ messages in thread
From: Lars Ingebrigtsen @ 2021-05-02  9:09 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: 47992, monnier, jakanakaevangeli

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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-05-02  9:09 ` Lars Ingebrigtsen
@ 2021-05-02 10:37   ` Daniel Mendler
  2021-05-03  8:50     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Mendler @ 2021-05-02 10:37 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 47992, monnier, jakanakaevangeli

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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook`
  2021-05-02 10:37   ` Daniel Mendler
@ 2021-05-03  8:50     ` Lars Ingebrigtsen
  0 siblings, 0 replies; 22+ messages in thread
From: Lars Ingebrigtsen @ 2021-05-03  8:50 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: 47992, monnier, jakanakaevangeli

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





^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-05-03  8:50 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-24 12:11 bug#47992: 27; 28; Phase out use of `equal` in `add-hook`, `remove-hook` Daniel Mendler
2021-04-24 20:12 ` bug#47992: [External] : " Drew Adams
2021-04-24 20:23   ` Daniel Mendler
2021-04-24 21:20     ` Drew Adams
2021-04-24 21:34       ` Daniel Mendler
2021-04-24 22:30   ` Stefan Monnier
2021-04-24 22:38     ` Daniel Mendler
2021-04-24 23:04       ` Stefan Monnier
2021-04-24 23:38         ` Daniel Mendler
2021-04-25  1:16         ` Drew Adams
2021-04-25  3:08           ` Stefan Monnier
2021-04-25  4:57             ` Drew Adams
2021-04-25 13:52               ` Stefan Monnier
2021-04-25  1:16       ` Drew Adams
2021-04-25  1:23     ` Drew Adams
2021-04-25  3:10       ` Stefan Monnier
2021-04-25  4:57         ` Drew Adams
2021-04-25 10:33           ` Daniel Mendler
2021-04-25 13:56           ` Stefan Monnier
2021-05-02  9:09 ` Lars Ingebrigtsen
2021-05-02 10:37   ` Daniel Mendler
2021-05-03  8:50     ` Lars Ingebrigtsen

unofficial mirror of bug-gnu-emacs@gnu.org 

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/emacs-bugs/0 emacs-bugs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 emacs-bugs emacs-bugs/ https://yhetil.org/emacs-bugs \
		bug-gnu-emacs@gnu.org
	public-inbox-index emacs-bugs

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.emacs.bugs
	nntp://news.gmane.io/gmane.emacs.bugs


code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs.git

AGPL code for this site: git clone http://ou63pmih66umazou.onion/public-inbox.git