bug#36190: 27.0.50; `put-text-property' etc. with buffer argument calls current buffer's `after-change-functions'

[Eli Zaretskii <eliz@gnu.org>]

> From: Pip Cet <pipcet@gmail.com>
> Date: Thu, 13 Jun 2019 19:42:29 +0000
> Cc: 36190@debbugs.gnu.org
> 
> > Not sure I have a clear idea of how you intend to use that additional
> > argument.  Are you suggesting that we switch to that buffer?
> 
> Yes:
> 
> @@ -2183,6 +2184,9 @@ signal_after_change (ptrdiff_t charpos,
> ptrdiff_t lendel, ptrdiff_t lenins)
>    if (inhibit_modification_hooks)
>      return;
> 
> +  record_unwind_current_buffer ();
> +  set_buffer_internal (buffer);

Ugh! switching buffers just to run a hook!  This will kill performance
in some cases.  We had something similar with JSON parsing a few
months ago.  I wish we had a better alternative.  Maybe we should warn
in the documentation that calling these functions with BUFFER being
other than the current buffer might hurt performance when
after-change-functions is non-nil.

> > Also, passing current_buffer sounds redundant to me anyway, because in
> > that case signal_after_change will not need to do anything that it
> > doesn't already do.  I would pass NULL instead.
> 
> May I ask why?

To make the code speak for itself.  With passing current_buffer, you
now rely on subroutines of set_buffer_internal two or 3 levels down to
test whether we are already in that buffer and do nothing.  Meanwhile,
you wasted cycles on 2 or 3 function calls, and forced someone who
reads the code to go down that rabbit hole if they want to understand
what happens in that particular case.

> I think passing current_buffer is the clearest signal we can send to
> someone reusing the code that they might have to change this if
> they're dealing with more than one buffer.

Each function has commentary, where you can say that NULL means not to
switch buffers because we are already there.  That is a more clear
signal, IME.

> As a practical matter, it's hard to change the text property functions
> to use NULL when passed a nil argument

How is it harder than passing current_buffer?

> so we'd have functions using current_buffer and others using NULL,
> and that seems needlessly inconsistent.

Sorry, I don't see any inconsistency.  We do such things (for other
kinds of arguments) all over the place.

It's really a matter of stylistic preferences, but you did ask why...