Re: Lisp primitives and their calling of the change hooks

[Alan Mackenzie <acm@muc.de>]

Hello, Eli.

On Mon, Jan 08, 2018 at 23:15:11 +0200, Eli Zaretskii wrote:
> > Date: Mon, 8 Jan 2018 19:24:15 +0000
> > Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > > There is also no a-c-f call if the decompression exits with an error.

> > > You mean, if the user quits?  That throws to top level, so it would be
> > > wrong to invoke any after-change hooks, and unwind_decompress will
> > > call the hooks for the partially uncompressed data.  Do we need more?

> > I was thinking more of when the compressed text is corrupt and the
> > decompression routines report an error.

> What do other primitives do when there's an error?  That was never
> reported in this discussion nor discussed, AFAICT.  Up front, I see no
> reason to keep any promises when that happens.

base64-decode-region first decodes into a mallocked area of store, and
if that has worked, inserts that area, then deletes the original.  This
gives two balanced pairs of b/a-c-f.  If there's an error in decoding,
there is neither buffer manipulation nor change hook calls.

base64-encode-region does the same, using a mallocked area, and with two
balanced pairs of b/a-c-f on success.

> > The (1 22016) b-c-f is thus unbalanced when this happens.

> If this is really important (and I don't see why it would be), you can
> add a call to after-change-hooks before unbind_to of the error return.

I think b/a-c-f calls should follow the rules whether a primitive
operation succeeds or fails.  This will enable operations partly done by
before-change-functions to be backed out of without leaving a buffer in
an inconsistent state.

> > I'm asking you to consider again having two pairs of hook calls in this
> > primitive (as, for example, base64-decode-region does).  That way we need
> > only signal the b-c-f for the deletion after the decompression has
> > worked, and we know we are going to follow through with the deleteion.  I
> > think an aborted decompression operation would also be easier to close
> > off with an a-c-f with this strategy.

> Is implementation convenience the only argument for Stefan's variant?
> If so, it doesn't convince me, as the difference is barely tangible.

The point is, that zlib-decompress-region is NOT an atomic change, and
trying to treat it like one will tie us in knots.

base64-de/encode-region ARE atomic from the buffer manipulation point of
view - the replacement is either wholly done or not done at all, and this
is known in advance.

zlib-decompress-region, by contrast, always writes the decompressed bit
into the buffer, then deletes a portion of buffer, but does not know in
advance which bit of buffer will be deleted.  (It depends on the success
of the decompression operation.)

The way I see it, to announce in advance that the original region will be
deleted (by calling b-c-f for it) is suboptimal.  If the decompression
fails, we need to balance that b-c-f with a "fake" a-c-f call.

There are several primitives which have more than one balanced b/a-c-f
call in execution: examples are the base64- ones, move-to-column (when it
replaces tabs), replace-buffer-contents, insert.  (If you are interested
enough, I can send you my test file.)

However, I think we'll agree that we've already spent enough time
debating this issue.  If you still say one b/a-c-f pair for a
(successful) zlib-decompress-region call, I will accept it, and fix the
b/a-c-f call of the unsuccessful case.

-- 
Alan Mackenzie (Nuremberg, Germany).