bug#12426: 24.2.50; Emacs is closed unexpectedly after query-replace

[Eli Zaretskii <eliz@gnu.org>]

> Date: Thu, 13 Sep 2012 07:01:48 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: dmoncayo@gmail.com, 12426@debbugs.gnu.org
> 
> IIUC marker can have out-of-buffer-range position, but only somewhere in the
> middle of an insertion/deletion/replace operation; out-of-range position should
> be detected and immediately fixed by adjust_markers_for_{delete,insert,replace}.

But marker_position and marker_byte_position are simple getters of
these two attributes of a marker.  If these attributes can be out of
range for some window of time, then the getters shouldn't enforce this
limitation.  Otherwise, they are getters that cannot be used in some
situations, which is IMO bad SE.  At the very least that should be
documented.

adjust_markers_for_* functions access the marker positions directly,
bypassing these two getters.  If we want to enforce the assertions you
added, we should change OVERLAY_POSITION not to use the getter, but
access the struct directly, too.

> BTW, look at this code from replace_range:
> 
>   /* Adjust the overlay center as needed.  This must be done after
>       adjusting the markers that bound the overlays.  */
>    adjust_overlays_for_delete (from, nchars_del);
>    adjust_overlays_for_insert (from, inschars);
> 
>    /* Adjust markers for the deletion and the insertion.  */
>    if (markers)
>      adjust_markers_for_replace (from, from_byte, nchars_del, nbytes_del,
> 				inschars, outgoing_insbytes);
> 
> The comment explicitly says that overlays should be adjusted _after_ markers,
> but the code adjusts overlays and then markers :-(. Since an overlays are
> bounded by markers, the comment looks correct but the code isn't. I suppose
> that the code snippets above should be swapped.

This was introduced in revision 40908 by Richard, and looks quite
deliberate.  So switching the order might not be TRT, especially since
adjust_overlays_for_insert recenters the overlays around the buffer
position in question, which makes access to overlays faster.

I tried to look in the various related mailing lists for the reason of
the change in revision 40908, but couldn't find anything appropriate.
Richard, do you happen to remember the reason?  The change and its log
entry are below; it looks like this was accompanied by some changes in
Lisp as well.

2001-11-11  Richard M. Stallman  <rms@gnu.org>

	* insdel.c (replace_range): Use adjust_markers_for_replace
	instead of adjust_markers_for_delete and adjust_markers_for_insert.

	* intervals.h: Declare set_text_properties and set_text_properties_1.

	* textprop.c (set_text_properties_1): New subroutine
	broken out of set_text_properties.
	(set_text_properties): Use set_text_properties_1.

	* intervals.c (graft_intervals_into_buffer):
	Use set_text_properties_1 to clear out properties.

	* search.c (Freplace_match): Use replace_range to insert
	and delete.  Don't request property inheritance from
	surrounding text.

=== modified file 'src/insdel.c'
--- src/insdel.c	2001-10-26 12:02:21 +0000
+++ src/insdel.c	2001-11-11 20:04:45 +0000
@@ -1423,13 +1423,6 @@ replace_range (from, to, new, prepare, i
   if (! EQ (current_buffer->undo_list, Qt))
     deletion = make_buffer_string_both (from, from_byte, to, to_byte, 1);
 
-  if (markers)
-    /* Relocate all markers pointing into the new, larger gap
-       to point at the end of the text before the gap.
-       Do this before recording the deletion,
-       so that undo handles this after reinserting the text.  */
-    adjust_markers_for_delete (from, from_byte, to, to_byte);
-
   GAP_SIZE += nbytes_del;
   ZV -= nchars_del;
   Z -= nchars_del;
@@ -1489,10 +1482,11 @@ replace_range (from, to, new, prepare, i
      adjusting the markers that bound the overlays.  */
   adjust_overlays_for_delete (from, nchars_del);
   adjust_overlays_for_insert (from, inschars);
+
+  /* Adjust markers for the deletion and the insertion.  */
   if (markers)
-    adjust_markers_for_insert (from, from_byte,
-			       from + inschars, from_byte + outgoing_insbytes,
-			       0);
+    adjust_markers_for_replace (from, from_byte, nchars_del, nbytes_del,
+				inschars, outgoing_insbytes);
 
   offset_intervals (current_buffer, from, inschars - nchars_del);