[Emacs-diffs] /srv/bzr/emacs/trunk r109186: Simplify and cleanup markers positioning code.

[Dmitry Antipov]

------------------------------------------------------------
revno: 109186
committer: Dmitry Antipov <address@hidden>
branch nick: trunk
timestamp: Sun 2012-07-22 09:37:24 +0400
message:
  Simplify and cleanup markers positioning code.
  * marker.c (attach_marker): More useful eassert.
  (live_buffer, set_marker_internal): New function.
  (Fset_marker, set_marker_restricted): Use set_marker_internal.
  (set_marker_both, set_marker_restricted_both): Use live_buffer.
modified:
  src/ChangeLog
  src/marker.c

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2012-07-22 03:44:35 +0000
+++ b/src/ChangeLog     2012-07-22 05:37:24 +0000
@@ -1,3 +1,11 @@
+2012-07-22  Dmitry Antipov  <address@hidden>
+
+       Simplify and cleanup markers positioning code.
+       * marker.c (attach_marker): More useful eassert.
+       (live_buffer, set_marker_internal): New function.
+       (Fset_marker, set_marker_restricted): Use set_marker_internal.
+       (set_marker_both, set_marker_restricted_both): Use live_buffer.
+
 2012-07-22  Paul Eggert  <address@hidden>
 
        * buffer.h (struct buffer.indirections): Now ptrdiff_t, not int,

=== modified file 'src/marker.c'
--- a/src/marker.c      2012-07-09 03:15:10 +0000
+++ b/src/marker.c      2012-07-22 05:37:24 +0000
@@ -431,8 +431,12 @@
 attach_marker (struct Lisp_Marker *m, struct buffer *b,
               ptrdiff_t charpos, ptrdiff_t bytepos)
 {
-  /* Every character is at least one byte.  */
-  eassert (charpos <= bytepos);
+  /* In a single-byte buffer, two positions must be equal.
+     Otherwise, every character is at least one byte.  */
+  if (BUF_Z (b) == BUF_Z_BYTE (b))
+    eassert (charpos == bytepos);
+  else
+    eassert (charpos <= bytepos);
 
   m->charpos = charpos;
   m->bytepos = bytepos;
@@ -446,191 +450,136 @@
     }
 }
 
-DEFUN ("set-marker", Fset_marker, Sset_marker, 2, 3, 0,
-       doc: /* Position MARKER before character number POSITION in BUFFER.
-BUFFER defaults to the current buffer.
-If POSITION is nil, makes marker point nowhere.
-Then it no longer slows down editing in any buffer.
-Returns MARKER.  */)
-  (Lisp_Object marker, Lisp_Object position, Lisp_Object buffer)
-{
-  register ptrdiff_t charpos;
-  register ptrdiff_t bytepos;
-  register struct buffer *b;
+/* If BUFFER is nil, return current buffer pointer.  Next, check
+   whether BUFFER is a buffer object and return buffer pointer
+   corresponding to BUFFER if BUFFER is live, or NULL otherwise.  */
+
+static inline struct buffer *
+live_buffer (Lisp_Object buffer)
+{
+  struct buffer *b;
+
+  if (NILP (buffer))
+    {
+      b = current_buffer;
+      eassert (!NILP (BVAR (b, name)));
+    }
+  else
+    {
+      CHECK_BUFFER (buffer);
+      b = XBUFFER (buffer);
+      if (NILP (BVAR (b, name)))
+       b = NULL;
+    }
+  return b;
+}
+
+/* Internal function to set MARKER in BUFFER at POSITION.  Non-zero
+   RESTRICTED means limit the POSITION by the visible part of BUFFER.  */
+
+static inline Lisp_Object
+set_marker_internal (Lisp_Object marker, Lisp_Object position,
+                    Lisp_Object buffer, int restricted)
+{
   register struct Lisp_Marker *m;
+  register struct buffer *b = live_buffer (buffer);
 
   CHECK_MARKER (marker);
   m = XMARKER (marker);
 
-  /* If position is nil or a marker that points nowhere,
-     make this marker point nowhere.  */
+  /* Set MARKER to point nowhere if BUFFER is dead, or
+     POSITION is nil or a marker points to nowhere.  */
   if (NILP (position)
-      || (MARKERP (position) && !XMARKER (position)->buffer))
-    {
-      unchain_marker (m);
-      return marker;
-    }
-
-  if (NILP (buffer))
-    b = current_buffer;
-  else
-    {
-      CHECK_BUFFER (buffer);
-      b = XBUFFER (buffer);
-      /* If buffer is dead, set marker to point nowhere.  */
-      if (EQ (BVAR (b, name), Qnil))
-       {
-         unchain_marker (m);
-         return marker;
-       }
-    }
-
-  /* Optimize the special case where we are copying the position
-     of an existing marker, and MARKER is already in the same buffer.  */
-  if (MARKERP (position) && b == XMARKER (position)->buffer
-      && b == m->buffer)
+      || (MARKERP (position) && !XMARKER (position)->buffer)
+      || !b)
+    unchain_marker (m);
+
+  /* Optimize the special case where we are copying the position of
+     an existing marker, and MARKER is already in the same buffer.  */
+  else if (MARKERP (position) && b == XMARKER (position)->buffer
+          && b == m->buffer)
     {
       m->bytepos = XMARKER (position)->bytepos;
       m->charpos = XMARKER (position)->charpos;
-      return marker;
-    }
-
-  CHECK_NUMBER_COERCE_MARKER (position);
-  charpos = clip_to_bounds (BUF_BEG (b), XINT (position), BUF_Z (b));
-  bytepos = buf_charpos_to_bytepos (b, charpos);
-
-  attach_marker (m, b, charpos, bytepos);
+    }
+
+  else
+    {
+      register ptrdiff_t charpos, bytepos;
+    
+      CHECK_NUMBER_COERCE_MARKER (position);
+      charpos = clip_to_bounds (restricted ? BUF_BEGV (b) : BUF_BEG (b),
+                               XINT (position),
+                               restricted ? BUF_ZV (b) : BUF_Z (b));
+      bytepos = buf_charpos_to_bytepos (b, charpos);
+      attach_marker (m, b, charpos, bytepos);
+    }
   return marker;
 }
 
-/* This version of Fset_marker won't let the position
-   be outside the visible part.  */
+DEFUN ("set-marker", Fset_marker, Sset_marker, 2, 3, 0,
+       doc: /* Position MARKER before character number POSITION in BUFFER,
+which defaults to the current buffer.  If POSITION is nil,
+makes marker point nowhere so it no longer slows down
+editing in any buffer.  Returns MARKER.  */)
+  (Lisp_Object marker, Lisp_Object position, Lisp_Object buffer)
+{
+  return set_marker_internal (marker, position, buffer, 0);
+}
+
+/* Like the above, but won't let the position be outside the visible part.  */
 
 Lisp_Object
-set_marker_restricted (Lisp_Object marker, Lisp_Object pos, Lisp_Object buffer)
+set_marker_restricted (Lisp_Object marker, Lisp_Object position,
+                      Lisp_Object buffer)
 {
-  register ptrdiff_t charpos;
-  register ptrdiff_t bytepos;
-  register struct buffer *b;
-  register struct Lisp_Marker *m;
-
-  CHECK_MARKER (marker);
-  m = XMARKER (marker);
-
-  /* If position is nil or a marker that points nowhere,
-     make this marker point nowhere.  */
-  if (NILP (pos)
-      || (MARKERP (pos) && !XMARKER (pos)->buffer))
-    {
-      unchain_marker (m);
-      return marker;
-    }
-
-  if (NILP (buffer))
-    b = current_buffer;
-  else
-    {
-      CHECK_BUFFER (buffer);
-      b = XBUFFER (buffer);
-      /* If buffer is dead, set marker to point nowhere.  */
-      if (EQ (BVAR (b, name), Qnil))
-       {
-         unchain_marker (m);
-         return marker;
-       }
-    }
-
-  /* Optimize the special case where we are copying the position
-     of an existing marker, and MARKER is already in the same buffer.  */
-  if (MARKERP (pos) && b == XMARKER (pos)->buffer
-      && b == m->buffer)
-    {
-      m->bytepos = XMARKER (pos)->bytepos;
-      m->charpos = XMARKER (pos)->charpos;
-      return marker;
-    }
-
-  CHECK_NUMBER_COERCE_MARKER (pos);
-  charpos = clip_to_bounds (BUF_BEGV (b), XINT (pos), BUF_ZV (b));
-  bytepos = buf_charpos_to_bytepos (b, charpos);
-
-  attach_marker (m, b, charpos, bytepos);
-  return marker;
+  return set_marker_internal (marker, position, buffer, 1);
 }
-
+
 /* Set the position of MARKER, specifying both the
    character position and the corresponding byte position.  */
 
 Lisp_Object
-set_marker_both (Lisp_Object marker, Lisp_Object buffer, ptrdiff_t charpos, 
ptrdiff_t bytepos)
+set_marker_both (Lisp_Object marker, Lisp_Object buffer,
+                ptrdiff_t charpos, ptrdiff_t bytepos)
 {
-  register struct buffer *b;
   register struct Lisp_Marker *m;
+  register struct buffer *b = live_buffer (buffer);
 
   CHECK_MARKER (marker);
   m = XMARKER (marker);
 
-  if (NILP (buffer))
-    b = current_buffer;
+  if (b)
+    attach_marker (m, b, charpos, bytepos);
   else
-    {
-      CHECK_BUFFER (buffer);
-      b = XBUFFER (buffer);
-      /* If buffer is dead, set marker to point nowhere.  */
-      if (EQ (BVAR (b, name), Qnil))
-       {
-         unchain_marker (m);
-         return marker;
-       }
-    }
-
-  /* In a single-byte buffer, the two positions must be equal.  */
-  if (BUF_Z (b) == BUF_Z_BYTE (b)
-      && charpos != bytepos)
-    abort ();
-
-  attach_marker (m, b, charpos, bytepos);
+    unchain_marker (m);
   return marker;
 }
 
-/* This version of set_marker_both won't let the position
-   be outside the visible part.  */
+/* Like the above, but won't let the position be outside the visible part.  */
 
 Lisp_Object
-set_marker_restricted_both (Lisp_Object marker, Lisp_Object buffer, ptrdiff_t 
charpos, ptrdiff_t bytepos)
+set_marker_restricted_both (Lisp_Object marker, Lisp_Object buffer,
+                           ptrdiff_t charpos, ptrdiff_t bytepos)
 {
-  register struct buffer *b;
   register struct Lisp_Marker *m;
+  register struct buffer *b = live_buffer (buffer);
 
   CHECK_MARKER (marker);
   m = XMARKER (marker);
 
-  if (NILP (buffer))
-    b = current_buffer;
-  else
+  if (b)
     {
-      CHECK_BUFFER (buffer);
-      b = XBUFFER (buffer);
-      /* If buffer is dead, set marker to point nowhere.  */
-      if (EQ (BVAR (b, name), Qnil))
-       {
-         unchain_marker (m);
-         return marker;
-       }
+      attach_marker 
+       (m, b, 
+        clip_to_bounds (BUF_BEGV (b), charpos, BUF_ZV (b)),
+        clip_to_bounds (BUF_BEGV_BYTE (b), bytepos, BUF_ZV_BYTE (b)));
     }
-
-  charpos = clip_to_bounds (BUF_BEGV (b), charpos, BUF_ZV (b));
-  bytepos = clip_to_bounds (BUF_BEGV_BYTE (b), bytepos, BUF_ZV_BYTE (b));
-
-  /* In a single-byte buffer, the two positions must be equal.  */
-  if (BUF_Z (b) == BUF_Z_BYTE (b)
-      && charpos != bytepos)
-    abort ();
-
-  attach_marker (m, b, charpos, bytepos);
+  else
+    unchain_marker (m);
   return marker;
 }
-
+
 /* Remove MARKER from the chain of whatever buffer it is in,
    leaving it points to nowhere.  This is called during garbage
    collection, so we must be careful to ignore and preserve