bug#9642: closed (Re: bug#9642: move-overlay creates an empty overlay with the evaporate property)

[Troels Nielsen]

On Mon, May 14, 2012 at 7:46 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> I ran into this and I think I got my head around the problem with
>> revision 108012.
> - Add a space before the opening paren of function calls.
> - Use two spaces to end sentences in comments.
> - Don't forget to punctuate your comments.
>
> [ Of course, I could do it myself, but we can't install your patch right
> now anyway because that's going beyond what we consider as "tiny patch"
> so we need your copyright paperwork first.  ]
>

Thanks, the small stuff should be fixed now, and paperwork completed,
and I have changed the patch to go against current trunk.

It furthermore fixes a newly introduced issue where overlays could be
made to end at a buffer position before they began.

Regards
Troels

=== modified file 'src/ChangeLog'
--- src/ChangeLog       2012-05-27 07:51:09 +0000
+++ src/ChangeLog       2012-05-27 21:43:24 +0000
@@ -1,3 +1,11 @@
+2012-05-27  Troels Nielsen <bn.troels@gmail.com>
+
+       * buffer.c (Fmove_overlay): Reinstate the earlier fix for
+       (Bug#9642), but explicitly check that the buffer the overlay would
+       be moved to is live and rearrange lines to make sure that errors
+       will not put the overlay in an inconsistent state.
+       (Fdelete_overlay): Cosmetics.
+
 2012-05-27  Paul Eggert  <eggert@cs.ucla.edu>

        * lisp.h [REL_ALLOC]: Omit duplicate prototypes.

=== modified file 'src/buffer.c'
--- src/buffer.c        2012-05-27 01:06:44 +0000
+++ src/buffer.c        2012-05-27 22:00:22 +0000
@@ -3676,10 +3676,10 @@
 buffer.  */)
   (Lisp_Object overlay, Lisp_Object beg, Lisp_Object end, Lisp_Object buffer)
 {
-  struct buffer *b, *ob;
+  struct buffer *b, *ob = 0;
   Lisp_Object obuffer;
   ptrdiff_t count = SPECPDL_INDEX ();
-  ptrdiff_t n_beg, n_end;
+  ptrdiff_t n_beg, n_end, o_beg, o_end;

   CHECK_OVERLAY (overlay);
   if (NILP (buffer))
@@ -3688,6 +3688,9 @@
     XSETBUFFER (buffer, current_buffer);
   CHECK_BUFFER (buffer);

+  if (NILP (Fbuffer_live_p (buffer)))
+    error ("Attempt to move overlay to a dead buffer");
+
   if (MARKERP (beg)
       && ! EQ (Fmarker_buffer (beg), buffer))
     error ("Marker points into wrong buffer");
@@ -3697,73 +3700,69 @@

   CHECK_NUMBER_COERCE_MARKER (beg);
   CHECK_NUMBER_COERCE_MARKER (end);
-  n_beg = clip_to_bounds (PTRDIFF_MIN, XINT (beg), PTRDIFF_MAX);
-  n_end = clip_to_bounds (PTRDIFF_MIN, XINT (end), PTRDIFF_MAX);
-
-  if (n_beg == n_end && ! NILP (Foverlay_get (overlay, Qevaporate)))
-    return Fdelete_overlay (overlay);
-
-  if (n_beg > n_end)
+
+  if (XINT (beg) > XINT (end))
     {
-      ptrdiff_t temp;
-      temp = n_beg; n_beg = n_end; n_end = temp;
+      Lisp_Object temp;
+      temp = beg; beg = end; end = temp;
     }

   specbind (Qinhibit_quit, Qt);

+  b = XBUFFER (buffer);
   obuffer = Fmarker_buffer (OVERLAY_START (overlay));
-  b = XBUFFER (buffer);
-  ob = BUFFERP (obuffer) ? XBUFFER (obuffer) : (struct buffer *) 0;
-
-  /* If the overlay has changed buffers, do a thorough redisplay.  */
-  if (!EQ (buffer, obuffer))
-    {
-      /* Redisplay where the overlay was.  */
-      if (!NILP (obuffer))
-       {
-         ptrdiff_t o_beg;
-         ptrdiff_t o_end;
-
-         o_beg = OVERLAY_POSITION (OVERLAY_START (overlay));
-         o_end = OVERLAY_POSITION (OVERLAY_END (overlay));
-
-         modify_overlay (ob, o_beg, o_end);
-       }
-
-      /* Redisplay where the overlay is going to be.  */
-      modify_overlay (b, n_beg, n_end);
-    }
-  else
-    /* Redisplay the area the overlay has just left, or just enclosed.  */
-    {
-      ptrdiff_t o_beg, o_end;
+
+  if (!NILP (obuffer))
+    {
+      ob = XBUFFER (obuffer);

       o_beg = OVERLAY_POSITION (OVERLAY_START (overlay));
       o_end = OVERLAY_POSITION (OVERLAY_END (overlay));

+      ob->overlays_before =
+        unchain_overlay (ob->overlays_before, XOVERLAY (overlay));
+      ob->overlays_after =
+        unchain_overlay (ob->overlays_after, XOVERLAY (overlay));
+      eassert (XOVERLAY (overlay)->next == NULL);
+    }
+
+  /* Set the overlay boundaries, which may clip them.  */
+  Fset_marker (OVERLAY_START (overlay), beg, buffer);
+  Fset_marker (OVERLAY_END (overlay), end, buffer);
+
+  n_beg = marker_position (OVERLAY_START (overlay));
+  n_end = marker_position (OVERLAY_END (overlay));
+
+  /* Find out how much we should redisplay.  */
+  if (EQ (buffer, obuffer))
+    /* Redisplay the area the overlay has just left, or just enclosed.  */
+    {
       if (o_beg == n_beg)
-       modify_overlay (b, o_end, n_end);
+        modify_overlay (b, o_end, n_end);
       else if (o_end == n_end)
-       modify_overlay (b, o_beg, n_beg);
+        modify_overlay (b, o_beg, n_beg);
       else
-       modify_overlay (b, min (o_beg, n_beg), max (o_end, n_end));
+        modify_overlay (b, min(n_beg, o_beg), max(o_end, n_end));
     }
-
-  if (!NILP (obuffer))
+  else
+    /* If the overlay has changed buffers, do a thorough redisplay.  */
     {
-      ob->overlays_before
-       = unchain_overlay (ob->overlays_before, XOVERLAY (overlay));
-      ob->overlays_after
-       = unchain_overlay (ob->overlays_after, XOVERLAY (overlay));
-      eassert (XOVERLAY (overlay)->next == NULL);
+      /* Redisplay where the overlay was.  */
+      if (ob)
+        modify_overlay (ob, o_beg, o_end);
+
+      /* Redisplay where the overlay is going to be.  */
+      modify_overlay (b, n_beg, n_end);
     }

-  Fset_marker (OVERLAY_START (overlay), beg, buffer);
-  Fset_marker (OVERLAY_END   (overlay), end, buffer);
+  /* Delete the overlay if it is empty after clipping and has the
+     evaporate property.  */
+  if (n_beg == n_end && !NILP (Foverlay_get (overlay, Qevaporate)))
+    return unbind_to (count, Fdelete_overlay (overlay));

-  /* Put the overlay on the wrong list.  */
-  end = OVERLAY_END (overlay);
-  if (OVERLAY_POSITION (end) < b->overlay_center)
+  /* Put the overlay into the new buffer's overlay lists, first on the
+     wrong list.  */
+  if (n_end < b->overlay_center)
     {
       XOVERLAY (overlay)->next = b->overlays_after;
       b->overlays_after = XOVERLAY (overlay);
@@ -3797,9 +3796,12 @@
   b = XBUFFER (buffer);
   specbind (Qinhibit_quit, Qt);

-  b->overlays_before = unchain_overlay (b->overlays_before,XOVERLAY (overlay));
-  b->overlays_after  = unchain_overlay (b->overlays_after, XOVERLAY (overlay));
+  b->overlays_before =
+    unchain_overlay (b->overlays_before, XOVERLAY (overlay));
+  b->overlays_after =
+    unchain_overlay (b->overlays_after, XOVERLAY (overlay));
   eassert (XOVERLAY (overlay)->next == NULL);
+
   modify_overlay (b,
                  marker_position (OVERLAY_START (overlay)),
                  marker_position (OVERLAY_END   (overlay)));