Fix problems resulting from modification of the undo list

* doc/lispref/text.texi (Atomic Changes): Describe what not to
do inside an atomic change group.
* lisp/elec-pair.el (electric-pair-inhibit-if-helps-balance):
Don't call `delete-char'; that edits the undo list by removing
boundary markers.
* lisp/subr.el (atomic-change-group, prepare-change-group): Warn
against modifying the undo list inside.

3 files changed, 23 insertions, 1 deletions

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index f15b3c33e0c..dac8d0d8ce9 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -6181,6 +6181,17 @@ would expect.  Non-nested use of change groups for the same buffer
 will get Emacs confused, so don't let it happen; the first change
 group you start for any given buffer should be the last one finished.
 
+  Emacs keeps track of change groups by assuming that by following
+each cdr in @code{buffer-undo-list}, it will eventually arrive at the
+cons it was set to at the time @code{prepare-change-group} was called.
+
+  If @code{buffer-undo-list} no longer contains that cons, Emacs will
+lose track of any change groups, resulting in an error when the change
+group is cancelled.  To avoid this, do not call any functions which
+may edit the undo list in such a manner, when a change group is
+active: notably, ``amalgamating'' commands such as @code{delete-char},
+which call @code{undo-auto-amalgamate}.
+
 @node Change Hooks
 @section Change Hooks
 @cindex change hooks
diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el
index b894965eae4..416c95e7a34 100644
--- a/lisp/elec-pair.el
+++ b/lisp/elec-pair.el
@@ -439,7 +439,9 @@ happened."
        ;; position some markers.  The real fix would be to compute the
        ;; result without having to modify the buffer at all.
        (atomic-change-group
-         (delete-char -1)
+         ;; Don't use `delete-char'; that may modify the head of the
+         ;; undo list.
+         (delete-region (point) (1- (point)))
          (throw
           'done
           (cond ((eq ?\( syntax)
diff --git a/lisp/subr.el b/lisp/subr.el
index cef631a69c3..1384215498b 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3773,6 +3773,9 @@ This means that if BODY exits abnormally,
 all of its changes to the current buffer are undone.
 This works regardless of whether undo is enabled in the buffer.
 
+Do not call functions which edit the undo list within BODY; see
+`prepare-change-group'.
+
 This mechanism is transparent to ordinary use of undo;
 if undo is enabled in the buffer and BODY succeeds, the
 user can undo the change normally."
@@ -3839,6 +3842,12 @@ Once you finish the group, don't use the handle again--don't try to
 finish the same group twice.  For a simple example of correct use, see
 the source code of `atomic-change-group'.
 
+As long as this handle is still in use, do not call functions
+which edit the undo list: if it no longer contains its current
+value, Emacs will not be able to cancel the change group.  This
+includes any \"amalgamating\" commands, such as `delete-char',
+which call `undo-auto-amalgamate'.
+
 The handle records only the specified buffer.  To make a multibuffer
 change group, call this function once for each buffer you want to
 cover, then use `nconc' to combine the returned values, like this: