4:14 p.m.
APPROVE 21.5
Not for 21.4, the code is completely different.
Will commit in a day or so; I want to run with it for a bit (I haven't
yet tested with a clean workspace).
Reply-To set to xemacs-beta.
It's possible that there will be unexpected changes in behavior,
specifically in font-lock and in cperl-mode. I'm not sure what they
might be (since this is a cache in theory there shouldn't be any).
Please let me know if you observe any.
The important change is the first bullet in the ChangeLog. The
immediate problem was that due to faulty logic in setup_syntax_cache
a value (Qnil) intended for use with syntax caches attached to strings
was being used in syntax caches attached to buffers, which expected a
marker. The marker code asserted markerp, and blooey!
I reorganized the code so that as much as possible it does what it
used to do, but I'm not sure about those parts that had to change.
The apparently gratuitous renames are intended to make it possible to
find most all code related to the syntax cache by grepping for
"syntax_cache", in preparation for more analysis and documentation.
There should be a regression test for this, but that needs some work
on test-harness.el, since it's not designed to be robust to code that
crashes or infloops.
Index: src/ChangeLog
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/src/ChangeLog,v
retrieving revision 1.921
diff -u -r1.921 ChangeLog
--- src/ChangeLog 17 Feb 2006 11:19:17 -0000 1.921
+++ src/ChangeLog 20 Feb 2006 15:12:36 -0000
＠＠ -0,0 +1,31 ＠＠
+2006-02-19 Stephen J. Turnbull <stephen(a)xemacs.org>
+
+ Fix crash (cf. Holger Schauer <yxz7j7xzk97.fsf(a)gimli.holgi.priv>).
+ Improve nomenclature (some identifiers were misnamed with
+ "syntax_table" although they are purely related to syntax cache).
+ Add lots of comments explaining logic and use of arguments.
+
+ * syntax.c (setup_syntax_cache): Fix broken logic that
+ initialized prev_change and next_change members to Qnil for syntax
+ caches associated with buffers, triggering an assertion because
+ the update functions expect those members to markers.
+
+ * syntax.c (signal_syntax_cache_extent_changed):
+ * syntax.c (reset_buffer_syntax_cache_range):
+ Rename reset_buffer_cache_range to reset_buffer_syntax_cache_range.
+
+ * lisp.h (signal_syntax_cache_extent_changed):
+ * syntax.c (signal_syntax_cache_extent_changed)
+ * syntax.c (update_syntax_cache):
+ * extents.c (signal_single_extent_changed):
+ Rename signal_syntax_table_extent_changed to
+ signal_syntax_cache_extent_changed.
+
+ * lisp.h (signal_syntax_cache_extent_adjust):
+ * syntax.c (signal_syntax_cache_extent_adjust):
+ * insdel.c (buffer_delete_range, buffer_insert_string_1):
+ Rename signal_syntax_table_extent_adjust to
+ signal_syntax_cache_extent_adjust.
+
+ * syntax.h (update_syntax_cache): Fix typo in comment.
+
Index: src/extents.c
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/src/extents.c,v
retrieving revision 1.63
diff -u -r1.63 extents.c
--- src/extents.c 25 Nov 2005 01:42:01 -0000 1.63
+++ src/extents.c 20 Feb 2006 15:12:36 -0000
＠＠ -1922,7 +1922,7 ＠＠
if (NILP (property) ? !NILP (Fextent_property (wrap_extent (extent),
Qsyntax_table, Qnil)) :
EQ (property, Qsyntax_table))
- signal_syntax_table_extent_changed (extent);
+ signal_syntax_cache_extent_changed (extent);
}
/* Make note that a change has happened in EXTENT. The change was either
Index: src/insdel.c
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/src/insdel.c,v
retrieving revision 1.34
diff -u -r1.34 insdel.c
--- src/insdel.c 24 Jan 2005 23:33:59 -0000 1.34
+++ src/insdel.c 20 Feb 2006 15:12:37 -0000
＠＠ -1265,7 +1265,7 ＠＠
MAP_INDIRECT_BUFFERS (buf, mbuf, bufcons)
{
- signal_syntax_table_extent_adjust (mbuf);
+ signal_syntax_cache_extent_adjust (mbuf);
}
signal_after_change (buf, pos, pos, pos + cclen);
＠＠ -1546,7 +1546,7 ＠＠
MAP_INDIRECT_BUFFERS (buf, mbuf, bufcons)
{
- signal_syntax_table_extent_adjust (mbuf);
+ signal_syntax_cache_extent_adjust (mbuf);
}
/* &&#### Here we consider converting the buffer from default to
Index: src/lisp.h
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/src/lisp.h,v
retrieving revision 1.136
diff -u -r1.136 lisp.h
--- src/lisp.h 26 Nov 2005 11:46:09 -0000 1.136
+++ src/lisp.h 20 Feb 2006 15:12:37 -0000
＠＠ -5279,8 +5279,8 ＠＠
EXFUN (Fchar_syntax, 2);
EXFUN (Fforward_word, 2);
extern Lisp_Object Vstandard_syntax_table;
-void signal_syntax_table_extent_changed (EXTENT extent);
-void signal_syntax_table_extent_adjust (struct buffer *buf);
+void signal_syntax_cache_extent_changed (EXTENT extent);
+void signal_syntax_cache_extent_adjust (struct buffer *buf);
void init_buffer_syntax_cache (struct buffer *buf);
void mark_buffer_syntax_cache (struct buffer *buf);
void uninit_buffer_syntax_cache (struct buffer *buf);
Index: src/syntax.c
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/src/syntax.c,v
retrieving revision 1.26
diff -u -r1.26 syntax.c
--- src/syntax.c 18 Dec 2005 22:55:52 -0000 1.26
+++ src/syntax.c 20 Feb 2006 15:12:37 -0000
＠＠ -254,6 +254,11 ＠＠
BUFFER_MIRROR_SYNTAX_TABLE (cache->buffer);
cache->start = Qnil;
cache->end = Qnil;
+ /* #### I'm not sure what INFINITE is for, but it's apparently needed by
+ setup_syntax_cache(). It looks like it's supposed to guarantee that
+ the test for POS outside of cache-valid range will never succeed, so
+ that update_syntax_cache won't get called, but it's hard to be sure.
+ Cf reset_buffer_syntax_cache_range. -- sjt */
if (infinite)
{
cache->prev_change = EMACS_INT_MIN;
＠＠ -266,15 +271,29 ＠＠
}
}
-struct syntax_cache *
-setup_syntax_cache (struct syntax_cache *cache, Lisp_Object object,
- struct buffer *buffer, Charxpos from, int count)
+/* #### This function and associated logic still needs work, and especially
+ documentation. */
+struct syntax_cache * /* return CACHE or the cache of OBJECT */
+setup_syntax_cache (struct syntax_cache *cache, /* syntax cache, may be NULL
+ if OBJECT is a buffer */
+ Lisp_Object object, /* the object (if any) cache
+ is associated with */
+ struct buffer *buffer, /* the buffer to use as source
+ of the syntax table */
+ Charxpos from, /* initial position of cache */
+ int count) /* direction? see code */
{
+ /* If OBJECT is a buffer, use its cache. Initialize cache. Make it valid
+ for the whole buffer if the syntax-table property is not being respected.
+ Else if OBJECT is not a buffer, initialize the cache passed in CACHE.
+ If the syntax-table property is being respected, update the cache. */
if (BUFFERP (object))
- cache = XBUFFER (object)->syntax_cache;
- if (!lookup_syntax_properties)
- init_syntax_cache (cache, object, buffer, 1);
- else if (!BUFFERP (object))
+ {
+ cache = XBUFFER (object)->syntax_cache;
+ if (!lookup_syntax_properties)
+ reset_buffer_syntax_cache_range (cache, object, 1);
+ }
+ else
init_syntax_cache (cache, object, buffer, 0);
if (lookup_syntax_properties)
{
＠＠ -339,14 +358,25 ＠＠
}
static void
-reset_buffer_cache_range (struct syntax_cache *cache, Lisp_Object buffer)
+reset_buffer_syntax_cache_range (struct syntax_cache *cache,
+ Lisp_Object buffer, int infinite)
{
Fset_marker (cache->start, make_int (1), buffer);
Fset_marker (cache->end, make_int (1), buffer);
Fset_marker_insertion_type (cache->start, Qt);
Fset_marker_insertion_type (cache->end, Qnil);
- cache->prev_change = -1;
- cache->next_change = -1;
+ /* #### Should we "cache->no_syntax_table_prop = 1;" here? */
+ /* #### Cf comment on INFINITE in init_syntax_cache. -- sjt */
+ if (infinite)
+ {
+ cache->prev_change = EMACS_INT_MIN;
+ cache->next_change = EMACS_INT_MAX;
+ }
+ else
+ {
+ cache->prev_change = -1;
+ cache->next_change = -1;
+ }
}
void
＠＠ -367,9 +397,10 ＠＠
cache->mirror_table = BUFFER_MIRROR_SYNTAX_TABLE (cache->buffer);
cache->start = Fmake_marker ();
cache->end = Fmake_marker ();
- reset_buffer_cache_range (cache, cache->object);
+ reset_buffer_syntax_cache_range (cache, cache->object, 0);
}
+/* finalize the syntax cache for BUF */
void
uninit_buffer_syntax_cache (struct buffer *buf)
{
＠＠ -401,27 +432,34 ＠＠
the value of EXTENT's syntax-table property is changing. */
void
-signal_syntax_table_extent_changed (EXTENT extent)
+signal_syntax_cache_extent_changed (EXTENT extent)
{
Lisp_Object buffer = Fextent_object (wrap_extent (extent));
if (BUFFERP (buffer))
{
+ /* This was getting called with the buffer's start and end null, eg in
+ cperl mode, which triggers an assert in byte_marker_position. Cf
+ thread rooted at <yxz7j7xzk97.fsf(a)gimli.holgi.priv> on xemacs-beta.
+ <yxzfymklb6p.fsf(a)gimli.holgi.priv> has a recipe, but you also need
+ to delete or type SPC to get the crash.
+ #### Delete this comment when setup_syntax_cache is made sane. */
struct syntax_cache *cache = XBUFFER (buffer)->syntax_cache;
+ /* #### would this be slower or less accurate in character terms? */
Bytexpos start = extent_endpoint_byte (extent, 0);
Bytexpos end = extent_endpoint_byte (extent, 1);
Bytexpos start2 = byte_marker_position (cache->start);
Bytexpos end2 = byte_marker_position (cache->end);
- /* If the extent is entirely before or entirely after the cache range,
- it doesn't overlap. Otherwise, invalidate the range. */
+ /* If the extent is entirely before or entirely after the cache
+ range, it doesn't overlap. Otherwise, invalidate the range. */
if (!(end < start2 || start > end2))
- reset_buffer_cache_range (cache, buffer);
+ reset_buffer_syntax_cache_range (cache, buffer, 0);
}
}
/* Extents have been adjusted for insertion or deletion, so we need to
refetch the start and end position of the extent */
void
-signal_syntax_table_extent_adjust (struct buffer *buf)
+signal_syntax_cache_extent_adjust (struct buffer *buf)
{
struct syntax_cache *cache = buf->syntax_cache;
/* If the cache was invalid before, leave it that way. We only want
＠＠ -499,7 +537,7 ＠＠
a zero-length `syntax-table' extent there (highly unlikely); if not,
then we can safely make the end closed, so it will take in newly
inserted text. (If such an extent is inserted, we will be informed
- through signal_syntax_table_extent_changed().) */
+ through signal_syntax_cache_extent_changed().) */
Fset_marker (cache->start, make_int (cache->prev_change), cache->object);
Fset_marker_insertion_type
(cache->start,
Index: src/syntax.h
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/src/syntax.h,v
retrieving revision 1.13
diff -u -r1.13 syntax.h
--- src/syntax.h 25 Nov 2005 01:42:07 -0000 1.13
+++ src/syntax.h 20 Feb 2006 15:12:38 -0000
＠＠ -354,7 +354,7 ＠＠
extern const struct sized_memory_description syntax_cache_description;
/* Note that the external interface to the syntax-cache uses charpos's, but
- intnernally we use bytepos's, for speed. */
+ internally we use bytepos's, for speed. */
void update_syntax_cache (struct syntax_cache *cache, Charxpos pos, int count);
struct syntax_cache *setup_syntax_cache (struct syntax_cache *cache,
--
School of Systems and Information Engineering http://turnbull.sk.tsukuba.ac.jp
University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN
Ask not how you can "do" free software business;
ask what your business can "do for" free software.