Subject: bug#78221: 31.0.50; Improving *-change-functions notifications

From: Alan Mackenzie <[email-masked]>
Date: Sat, 3 May 2025 14:13:40 +0000

Hello, Stefan.

On Fri, May 02, 2025 at 17:48:11 -0400, Stefan Monnier wrote:
> Package: Emacs
> Version: 31.0.50

> After a long discussion with Eli over bug#78042, we agreed there's
> a need to improve the C side handling of
> *-change-functions notifications.

I've glanced at bug#78042, but haven't (as yet) followed the discussions
in detail.

> Alan Mackenzie made significant improvements in the past to the C code
> that runs these hooks, so that we now rarely break our promises in terms
> of how/when those notifications are sent, but there are still
> cases lurking.

By "those promises", I think you mean being that we invoke
before-change-functions and after-change-functions strictly in matched
pairs.  We only actually do this most of the time.

On 2018-01-06, we formally agreed that these hooks will be invoked as
follows:
  o - Every buffer changing primitive starts off with exactly one call
    to b-c-f,
  o - after which there will be zero, one, or several calls to a-c-f.
  o - The b-c-f's region will enclose those of all the a-c-f regions,
    but need not be the minimal such region.

I actually think a good strategy would be to invoke these hooks strictly
in matching pairs in all cases, but I'm sure I said that back in 2018
too.

> The main problem is that it's very hard to find the places where this
> occurs and it's not always obvious that the corresponding fix is
> harmless.  A fix is likely to involve changes visible to the Elisp side,
> such as new hooks, so it seemed out of scope for bug#78042.

I don't think it's all that hard.  In 2020, I made a list of which
non-static functions in insdel.c call before/after-change-functions.
The list, which is likely still valid, or very nearly so, looked like
this:

insert                             B/A
insert_and_inherit                 B/A
insert_char                        B/A
insert_string                      B/A
insert_before_markers              B/A

insert_before_markers_and_inherit  B/A
insert_1_both                      ?/-
insert_from_string                 B/A
insert_from_string_before_markers  B/A
insert_from_gap_1                  -/-

insert_from_gap                    -/-
insert_from_buffer                 B/A
del_range                          B/A
del_range_1                        ?/A
del_range_byte                     B/A

del_range_both                     ?/A
del_range_2                        -/-
replace_range                      ?/A
replace_range_2                    -/-
modify_text                        B/-

(where B/A means both before- and after-change-functions are called,
etc.).

It would be comparatively easy to modify insdel.c so that _all_ buffer
changing functions invoked the two hooks in matched pairs.  That would
require a willingness to make substantial changes in insdel.c, something
that hasn't been forthcoming in years gone bye.

But if an alternative is to allow nested calls to the hooks (something
we don't allow at the moment, even though the rule isn't explicitly
formulated anywhere), this is going to involve modifying insdel.c
anyway.

> Some of the possible avenues that came up or that I have considered and
> not yet discarded (and they're not mutually exclusive either):

> - Don't notify changes to text-properties.
>   Currently, things like `put-text-property` run the *-change-functions
>   like any other buffer modification.  In practice this is of dubious value:
>   - It's already the case that most calls to `put-text-property` and
>     friends don't cause any notification because they are wrapped within
>     a `with-silent-modifications` or otherwise take place from code
>     which always runs with `inhibit-modification-hooks` set
>     (e.g. because it's run from a *-change-function).
>     So *-change-functions can't reliably track changes to text-properties.
>   - Most *-change-functions are *not* interested in text-property
>     changes anyway.
>   The above two points suggest maybe we could just refrain from
>   running *-change-functions when changing the text-properties (just
>   like we do for changes to overlay properties).
>   Then again, this would be a backward-incompatible change, so there's
>   a chance some packages out there would be negatively affected.
>   Another option would be to allow packages to choose whether to receive
>   notifications like now or only for changes to the text.
>   E.g. just like we have `buffer-modified-tick` and
>   `buffer-chars-modified-tick`, we could have `*-change-functions`
>   and `*-chars-change-functions` (with some questions remaining about
>   `first-change-hook` and the `modification-hooks` property).
>   Or we could rely on a (symbol) property being set on the functions
>   added to `*-change-functions` to tell whether they want to know about
>   changes to text-properties or not.
>   Or maybe a more crude way would be a buffer-local variable controlling
>   whether `*-change-functions` are called for text-property changes.

I think it was a mistake in the beginning to have text property
modifications trigger before/after-c-f.  But seeing it's been that way
for so long, I don't think it would be a good idea to change it now.

> - Allow *-change-functions notifications to be nested.
>   E.g. allow a C function to call

>       BEFORE 10 100
>         BEFORE 20 30
>         AFTER 20 25 10
>       AFTER 10 90 90
>       AFTER 10 60 80

>   or something like that.  This would require some way for the C code to
>   indicate which BEGIN corresponds to which AFTER.  This could happen
>   for example by adding a "change-ID" to every notification, so we'd
>   get:
  
>       BEFORE nnn 10 100
>         BEFORE mmm 20 30
>         AFTER mmm 20 25 10
>       AFTER nnn 10 90 90
>       AFTER nnn 10 60 80

>   I don't know how we could do that in a backward compatible way, and
>   I can't think of any `*-change-functions` which would benefit from
>   this nesting (all the ones I can think of would either ignore the
>   extra info or use the change-ID to distinguish inner notifications
>   from outer ones and then ignore the inner ones).

> - Maybe the last point can be turned into an internal change with no
>   ELisp-side changes: use an approach like the one above but filter out
>   the inner changes directly in the C code.  This would provide
>   to ELisp the same behavior that was brought by the fix to bug#78042,
>   but without resorting to the "hammer" of `inhibit-modification-hooks`
>   which risks silencing legitimate notifications.

I don't think nested b/a-c-f are a good idea at all.  It's one of these
ideas that would introduce a very great deal of complexity for rather
limited benefit.  We would be paying a penalty in increased bug
complexity for the forseeable future.

before/after-change-functions functions SHOULDN'T themselves make
changes to the buffer text (apart from text properties).  I don't think
it would be all that hard to instrument insdel.c to signal an error when
such changes do get made, just tedious.  Running such an instrumented
Emacs on the test suite would catch a very great number of violations.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).