bug#34525: replace-regexp missing some matches

[Alan Mackenzie <acm@muc.de>]

Hello again, Stefan.

On Wed, Feb 27, 2019 at 14:22:51 +0000, Alan Mackenzie wrote:
> On Tue, Feb 26, 2019 at 15:09:54 -0500, Stefan Monnier wrote:
> > > gl_state contains a cached interval, gl_state->backward_i, and there
> > > is no guarantee that its ->position will have been updated by
> > > adjust_intervals_for_insertion.  In the current bug, I believe it
> > > hasn't been adjusted.

> > Hmm... gl_state is not supposed to be kept "live" across buffer
> > modifications.  It's supposed to be used only *within* read-only
> > primitives which set it from scratch at the beginning (by calling
> > SETUP_SYNTAX_TABLE, SETUP_BUFFER_SYNTAX_TABLE, or
> > SETUP_SYNTAX_TABLE_FOR_OBJECT).  The backward_i and forward_i fields are
> > actually reset in the first call to update_syntax_table, by passing it
> > a true value for the `init` arg.

> > So the problem you describe might be due to some place where we fail to
> > reset gl_state before using it, or maybe it's a bug in
> > SETUP_*_SYNTAX_TABLE*

> I see another potential problem, and I'd like your view on it, please.

> Namely, in next_interval, we have

>   if (! NULL_RIGHT_CHILD (i))
>     {
>       i = i->right;
>       while (! NULL_LEFT_CHILD (i))
>         i = i->left;                  <===============

>       i->position = next_position;
>       return i;
>     }

> Here, in seeking the next interval, we go down a chain of `left's.  We
> do not set the ->position field of these intervals, except for the last
> one, which we return.

> So the returned interval doesn't satisfy the condition that all its
> parents have their ->position's set correctly.  Thus if we use this
> interval as an argument to update_interval, we will likely fail.  I
> think this can happen in update_syntax_table.

> There is an analogous situation in previous_interval.

> I might try adding code to this to set these ->position's.  Trouble is,
> it might slow things down quite a bit.

I've done this, and it appears to have fixed the bug.  :-)  As for the
slowdown, I haven't timed it, yet.

Here is the diff of the current state of my changes:



diff --git a/src/intervals.c b/src/intervals.c
index 524bb944e5..d37ca64bd0 100644
--- a/src/intervals.c
+++ b/src/intervals.c
@@ -654,7 +654,14 @@ next_interval (register INTERVAL interval)
     {
       i = i->right;
       while (! NULL_LEFT_CHILD (i))
-	i = i->left;
+        /* OLD STOUGH, 2019-02-27 */
+	/* i = i->left; */
+        /* NEW STOUGH, 2019-02-27 */
+        {
+          i->position = next_position + LEFT_TOTAL_LENGTH (i);
+          i = i->left;
+        }
+      /* END OF NEW STOUGH */
 
       i->position = next_position;
       return i;
@@ -691,7 +698,15 @@ previous_interval (register INTERVAL interval)
     {
       i = interval->left;
       while (! NULL_RIGHT_CHILD (i))
-	i = i->right;
+	/* OLD STOUGH, 2019-02-27 */
+        /* i = i->right; */
+        /* NEW STOUGH, 2019-02-27 */
+        {
+          i->position = interval->position - TOTAL_LENGTH (i)
+            + LEFT_TOTAL_LENGTH(i);
+          i = i->right;
+        }
+      /* END OF NEW STOUGH */
 
       i->position = interval->position - LENGTH (i);
       return i;


> >         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).