GNU bug report logs - #78042
31.0.50; Improper sequence of `before/after-change-functions` calls

Previous Next

Full log

View this message in rfc822 format

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78042 <at> debbugs.gnu.org
Subject: bug#78042: Improper sequence of `before/after-change-functions` calls
Date: Sat, 26 Apr 2025 13:27:00 -0400

>> The function where I made the modification inserts bytes into
>> the buffer but without running the hooks.
>> So it is *already* the case that the callers must run the hooks
>> themselves.
>
> I don't understand: if you made the changes (by removing the calls to
> modification hooks), then the function originally _was_ calling them,
> no?

Here's what currently happens:

- the caller calls `before-change-functions`.
- it calls `decode_coding`:
  - in all cases, if dst_object is a buffer, `decode_coding` inserts
    bytes into that buffer.
  - in no case does `decode_coding` take charge to run
    the `before/after-change-functions` needed for those insertions.
  - in some relatively rare cases, `decode_coding` adds some
    text-properties causing nested `before/after-change-functions` calls.
    This is always done on the just-inserted text, so it's *always*
    nested (aka redundant, and harmful).
- the caller then calls `after-change-functions`.

The patch I sent binds `Qinhibit_modification_hooks` to t during the
execution of `decode_coding`, so it affects only nested calls to
`before/after-change-functions` (which apply only to some cases and only
to some parts of the inserted text), not the "normal" calls which are
always performed outside of that function.

> The problem with such ad-hoc evidence is that it relies on the
> assumption that your code and the "usual" cases everyone runs do
> exercise all of the relevant code paths.  Such assumptions are not
> very reliable in Emacs, IME.

No, the evidence is simply that `decode_coding` does not itself do
anything to run the change functions to reflect the text insertion.
So the inhibition that my patch does can affect only those redundant
nested runs.

> But the "usual-case" notification is about the entire decoded text,
> whereas the notifications your patch blocks are about smaller chunks
> of text for specific kinds of changes, and thus more fine-grained.  So
> the hooks will need to do a more complex job now: they cannot rely on
> the fact that notifications for decoding are separate from
> notifications about text-property changes, so they will need to
> examine the entire decoded region of text and decide what to do with
> each chunk of it.

No, any function placed on those hooks has to deal with a wild array of
other cases and simply can never know whether they're called for
text-property changes or anything else.

Instead, (the coders of) those functions always have to expend a great
effort to try and make as few assumptions as possible because those
assumptions *always* end up being broken in one case or another.

> Or maybe we should not install this at all?  What are the problems
> these "nested" notifications cause, again?

I've seen a backtrace in CC-mode (tho I haven't yet been able to figure
out how to reproduce it: it depends on the state of the CC-mode cache),
it causes Eglot to throw its hands up and say "how well, Emacs messed
up, let's start over", etc...

I guess next time I'll just push my patch silently to avoid having to
argue why we should fix our bugs,


        Stefan

Previous Next

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