Package: emacs;
Reported by: Gerd Möllmann <gerd.moellmann <at> gmail.com>
Date: Sat, 19 Apr 2025 16:06:02 UTC
Severity: normal
Found in version 31.0.50
View this message in rfc822 format
From: Pip Cet <pipcet <at> protonmail.com> To: Gerd Möllmann <gerd.moellmann <at> gmail.com> Cc: Eli Zaretskii <eliz <at> gnu.org>, monnier <at> iro.umontreal.ca, 77924 <at> debbugs.gnu.org, stefankangas <at> gmail.com Subject: bug#77924: 31.0.50; [Feature branch] Change marker implementation Date: Thu, 24 Apr 2025 19:26:10 +0000
Gerd Möllmann <gerd.moellmann <at> gmail.com> writes:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> + DO_MARKERS (b, m)
>>> + {
>>> + if (!vectorlike_marked_p (&m->header))
>>> + marker_vector_remove (XVECTOR (BUF_MARKERS (b)), m);
>>> + }
>>> + END_DO_MARKERS;
>>
>> Would it be possible not to introduce macros that modify the C syntax?
>> These are problematic for more than one reason (one being that they
>> are not recognized by C modes we use). We have macros whose names
>> start with FOR_EACH_ (like FOR_EACH_FRAME); can we introduce
>> FOR_EACH_MARKER instead, and not hide opening/closing braces inside
>> macros?
>
> I tried, already in igc, but failed. IOW, I don't now a way to get rid
> of the END_DO_MARKERS.
Okay, I'll bite.
I think what we should do is mimic FOR_EACH_TAIL, and use
FOR_EACH_MARKER like this:
struct Lisp_Marker *m;
FOR_EACH_MARKER (b, m)
{
/* do something with m */
}
However, if we wanted to, we could do this (the "continued" thing is
overkill to make "break" work in a FOR_EACH_MARKER loop):
diff --git a/src/marker-vector.h b/src/marker-vector.h
index 141e79ac0ee..bfca02fc33d 100644
--- a/src/marker-vector.h
+++ b/src/marker-vector.h
@@ -39,27 +39,55 @@ #define EMACS_MARKER_VECTOR_H
MARKER_VECTOR_ENTRY_SIZE = 2,
};
-/* Iterate over markers in marker vector MV, binding a variable with
- name M to a pointer to Lisp_Marker. The loop must be ended
- with an END_DO_MARKERS. */
-
-# define DO_MARKERS_OF_VECTOR(mv, m) \
- for (ptrdiff_t e_ = MARKER_VECTOR_HEADER_SIZE, \
- end_ = XFIXNUM (AREF (mv, MARKER_VECTOR_MAX_ENTRY)); \
- e_ <= end_; \
- e_ += MARKER_VECTOR_ENTRY_SIZE) \
- { \
- Lisp_Object m_ = AREF (mv, e_ + MARKER_VECTOR_OFFSET_MARKER); \
- if (MARKERP (m_)) \
- { \
- struct Lisp_Marker *m = XMARKER (m_);
-
-/* Iterate over markers of buffer B, binding a variable with name M to a
- pointer to Lisp_Marker. The loop must be ended with an
- END_DO_MARKERS. */
-
-# define DO_MARKERS(b, m) DO_MARKERS_OF_VECTOR (BUF_MARKERS (b), m)
-# define END_DO_MARKERS }}
+struct for_each_marker_data
+{
+ ptrdiff_t e;
+ ptrdiff_t end;
+ Lisp_Object m;
+ Lisp_Object mv;
+ bool continued;
+ struct Lisp_Marker *marker;
+};
+
+INLINE struct for_each_marker_data
+build_for_each_marker_data(Lisp_Object mv)
+{
+ struct for_each_marker_data ret;
+ ret.e = MARKER_VECTOR_HEADER_SIZE;
+ ret.end = XFIXNUM (AREF (mv, MARKER_VECTOR_MAX_ENTRY));
+ ret.m = Qnil;
+ ret.mv = mv;
+ ret.marker = NULL;
+ ret.continued = true;
+ return ret;
+}
+
+INLINE bool
+next_marker_entry (struct for_each_marker_data *d)
+{
+ if (!d->continued)
+ return false;
+ while (d->e <= d->end)
+ {
+ d->m = AREF (d->mv, d->e + MARKER_VECTOR_OFFSET_MARKER);
+ d->e += MARKER_VECTOR_ENTRY_SIZE;
+ if (MARKERP (d->m))
+ {
+ d->marker = XMARKER (d->m);
+ d->continued = false;
+ return true;
+ }
+ }
+ return false;
+}
+
+#define FOR_EACH_MARKER_OF_VECTOR(v, m) \
+ for (struct for_each_marker_data d_ = build_for_each_marker_data (v); \
+ next_marker_entry (&d_);) \
+ for (struct Lisp_Marker *m = d_.marker; !d_.continued; d_.continued = true)
+
+#define FOR_EACH_MARKER(b, m) \
+ FOR_EACH_MARKER_OF_VECTOR(BUF_MARKERS (b), m)
Lisp_Object make_marker_vector (void);
Lisp_Object alloc_marker_vector (ptrdiff_t len);
diff --git a/src/alloc.c b/src/alloc.c
index c90f60ee518..30bdd1140b8 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -7114,12 +7114,11 @@ sweep_symbols (void)
static void
unchain_dead_markers (struct buffer *b)
{
- DO_MARKERS (b, m)
+ FOR_EACH_MARKER (b, m)
{
if (!vectorlike_marked_p (&m->header))
marker_vector_remove (XVECTOR (BUF_MARKERS (b)), m);
}
- END_DO_MARKERS;
}
NO_INLINE /* For better stack traces */
diff --git a/src/buffer.c b/src/buffer.c
index 93a2edb3693..21b6096517d 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -2073,12 +2073,11 @@ DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ",
/* Unchain all markers that belong to this indirect buffer.
Don't unchain the markers that belong to the base buffer
or its other indirect buffers. */
- DO_MARKERS (b, m)
+ FOR_EACH_MARKER (b, m)
{
if (m->buffer == b)
marker_vector_remove (XVECTOR (BUF_MARKERS (b)), m);
}
- END_DO_MARKERS;
/* Intervals should be owned by the base buffer (Bug#16502). */
i = buffer_intervals (b);
@@ -2618,7 +2617,7 @@ #define swapfield_(field, type) \
other_buffer->text->end_unchanged = other_buffer->text->gpt;
swap_buffer_overlays (current_buffer, other_buffer);
{
- DO_MARKERS (current_buffer, m)
+ FOR_EACH_MARKER (current_buffer, m)
{
if (m->buffer == other_buffer)
m->buffer = current_buffer;
@@ -2627,8 +2626,7 @@ #define swapfield_(field, type) \
BUF_MARKERS(buf) should either be for `buf' or dead. */
eassert (!m->buffer);
}
- END_DO_MARKERS;
- DO_MARKERS (other_buffer, m)
+ FOR_EACH_MARKER (other_buffer, m)
{
if (m->buffer == current_buffer)
m->buffer = other_buffer;
@@ -2637,7 +2635,6 @@ #define swapfield_(field, type) \
BUF_MARKERS(buf) should either be for `buf' or dead. */
eassert (!m->buffer);
}
- END_DO_MARKERS;
}
{ /* Some of the C code expects that both window markers of a
@@ -2746,12 +2743,11 @@ DEFUN ("set-buffer-multibyte", Fset_buffer_multibyte, Sset_buffer_multibyte,
GPT = GPT_BYTE;
TEMP_SET_PT_BOTH (PT_BYTE, PT_BYTE);
- DO_MARKERS (current_buffer, tail)
+ FOR_EACH_MARKER (current_buffer, tail)
{
const ptrdiff_t bytepos = marker_vector_bytepos (tail);
marker_vector_set_charpos (tail, bytepos);
}
- END_DO_MARKERS;
/* Convert multibyte form of 8-bit characters to unibyte. */
pos = BEG;
@@ -2901,13 +2897,12 @@ DEFUN ("set-buffer-multibyte", Fset_buffer_multibyte, Sset_buffer_multibyte,
TEMP_SET_PT_BOTH (position, byte);
}
- DO_MARKERS (current_buffer, tail)
+ FOR_EACH_MARKER (current_buffer, tail)
{
ptrdiff_t bytepos = marker_vector_bytepos (tail);
bytepos = advance_to_char_boundary (bytepos);
marker_vector_set_charpos (tail, BYTE_TO_CHAR (bytepos));
}
- END_DO_MARKERS;
/* Do this last, so it can calculate the new correspondences
between chars and bytes. */
diff --git a/src/coding.c b/src/coding.c
index f3dd2639e73..51c87fb3161 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -8118,14 +8118,13 @@ decode_coding_object (struct coding_system *coding,
move_gap_both (from, from_byte);
if (BASE_EQ (src_object, dst_object))
{
- DO_MARKERS (current_buffer, tail)
+ FOR_EACH_MARKER (current_buffer, tail)
{
const ptrdiff_t charpos = marker_vector_charpos (tail);
tail->need_adjustment
= charpos == (tail->insertion_type ? from : to);
need_marker_adjustment |= tail->need_adjustment;
}
- END_DO_MARKERS;
saved_pt = PT, saved_pt_byte = PT_BYTE;
TEMP_SET_PT_BOTH (from, from_byte);
current_buffer->text->inhibit_shrinking = true;
@@ -8250,7 +8249,7 @@ decode_coding_object (struct coding_system *coding,
if (need_marker_adjustment)
{
- DO_MARKERS (current_buffer, tail)
+ FOR_EACH_MARKER (current_buffer, tail)
{
if (tail->need_adjustment)
{
@@ -8269,7 +8268,6 @@ decode_coding_object (struct coding_system *coding,
}
}
}
- END_DO_MARKERS;
}
}
@@ -8340,14 +8338,13 @@ encode_coding_object (struct coding_system *coding,
if (BASE_EQ (src_object, dst_object) && BUFFERP (src_object))
{
same_buffer = true;
- DO_MARKERS (XBUFFER (src_object), tail)
+ FOR_EACH_MARKER (XBUFFER (src_object), tail)
{
const ptrdiff_t charpos = marker_vector_charpos (tail);
tail->need_adjustment
= charpos == (tail->insertion_type ? from : to);
need_marker_adjustment |= tail->need_adjustment;
}
- END_DO_MARKERS;
}
if (! NILP (CODING_ATTR_PRE_WRITE (attrs)))
@@ -8505,7 +8502,7 @@ encode_coding_object (struct coding_system *coding,
if (need_marker_adjustment)
{
- DO_MARKERS (current_buffer, tail)
+ FOR_EACH_MARKER (current_buffer, tail)
{
if (tail->need_adjustment)
{
@@ -8524,7 +8521,6 @@ encode_coding_object (struct coding_system *coding,
}
}
}
- END_DO_MARKERS;
}
}
diff --git a/src/editfns.c b/src/editfns.c
index 61cb7bdd201..b96275dc0a3 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4438,7 +4438,7 @@ transpose_markers (ptrdiff_t start1, ptrdiff_t end1,
amt1 = (end2 - start2) + (start2 - end1);
amt2 = (end1 - start1) + (start2 - end1);
- DO_MARKERS (current_buffer, marker)
+ FOR_EACH_MARKER (current_buffer, marker)
{
mpos = marker_vector_charpos (marker);
if (mpos >= start1 && mpos < end2)
@@ -4452,7 +4452,6 @@ transpose_markers (ptrdiff_t start1, ptrdiff_t end1,
}
marker_vector_set_charpos (marker, mpos);
}
- END_DO_MARKERS;
}
DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
diff --git a/src/marker-vector.c b/src/marker-vector.c
index b65bbe15845..cd2603f9561 100644
--- a/src/marker-vector.c
+++ b/src/marker-vector.c
@@ -164,7 +164,7 @@ check_marker_vector (struct Lisp_Vector *v, bool allocating)
size_t nused = 0;
Lisp_Object mv = make_lisp_ptr (v, Lisp_Vectorlike);
- DO_MARKERS_OF_VECTOR (mv, m)
+ FOR_EACH_MARKER_OF_VECTOR (mv, m)
{
eassert (m->entry == e_);
eassert (m->buffer != NULL);
@@ -175,7 +175,6 @@ check_marker_vector (struct Lisp_Vector *v, bool allocating)
}
++nused;
}
- END_DO_MARKERS;
eassert ((nused + nfree) * MARKER_VECTOR_ENTRY_SIZE
+ MARKER_VECTOR_HEADER_SIZE == gc_vsize (v));
@@ -286,11 +285,10 @@ marker_vector_reset (struct buffer *b)
/* The old GC contains at least one assertion that unchaining markers
in kill-buffer resets the markers' buffers. IGC does not do this,
can't do this, and does not need it. */
- DO_MARKERS (b, m)
+ FOR_EACH_MARKER (b, m)
{
m->buffer = NULL;
}
- END_DO_MARKERS;
BUF_MARKERS (b) = Qnil;
}
@@ -348,7 +346,7 @@ marker_vector_adjust_for_insert (struct buffer *b,
{
const ptrdiff_t nchars = to_charpos - from_charpos;
struct Lisp_Vector *v = XVECTOR (BUF_MARKERS (b));
- DO_MARKERS (b, m)
+ FOR_EACH_MARKER (b, m)
{
const ptrdiff_t charpos = XFIXNUM (CHARPOS (v, m->entry));
if (charpos == from_charpos)
@@ -359,7 +357,6 @@ marker_vector_adjust_for_insert (struct buffer *b,
else if (charpos > from_charpos)
CHARPOS (v, m->entry) = make_fixnum (charpos + nchars);
}
- END_DO_MARKERS;
}
/* Adjust marker positions of buffer Bs for a replacement of text at
@@ -375,7 +372,7 @@ marker_vector_adjust_for_replace (struct buffer *b,
const ptrdiff_t diff_nchars = new_nchars - old_nchars;
const ptrdiff_t old_to_charpos = from_charpos + old_nchars;
struct Lisp_Vector *v = XVECTOR (BUF_MARKERS (b));
- DO_MARKERS (b, m)
+ FOR_EACH_MARKER (b, m)
{
const ptrdiff_t charpos = XFIXNUM (CHARPOS (v, m->entry));
if (charpos >= old_to_charpos)
@@ -383,5 +380,4 @@ marker_vector_adjust_for_replace (struct buffer *b,
else if (charpos > from_charpos)
CHARPOS (v, m->entry) = make_fixnum (from_charpos);
}
- END_DO_MARKERS;
}
diff --git a/src/undo.c b/src/undo.c
index 65de0bd4e13..3e63c8af3f9 100644
--- a/src/undo.c
+++ b/src/undo.c
@@ -128,7 +128,7 @@ record_marker_adjustments (ptrdiff_t from, ptrdiff_t to)
{
prepare_record ();
- DO_MARKERS (current_buffer, m)
+ FOR_EACH_MARKER (current_buffer, m)
{
ptrdiff_t charpos = marker_vector_charpos (m);
eassert (charpos <= Z);
@@ -154,7 +154,6 @@ record_marker_adjustments (ptrdiff_t from, ptrdiff_t to)
}
}
}
- END_DO_MARKERS;
}
/* Record that a deletion is about to take place, of the characters in
Note that "m" appears only once, so there is another possibility:
FOR_EACH_MARKER (v, struct Lisp_Marker *m)
{
/* do something with m */
}
I'm not saying any of this is a good idea, but it appears possible.
Pip
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.