bug#78042: Improper sequence of `before/after-change-functions` calls

[Stefan Monnier]

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