bug#14281: 24.3; replace-match leaves point at wrong place

[Barry OReilly <gundaetiapo@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 510 bytes --]

> Maybe we could add code that saves just the (match-beginning 0) and
> signals an error if it was not properly preserved.  This would still
> require change-functions to save the match-data if they use it, but it
> might catch the offenders earlier.

I made a crack at it. It was necessary to use save-match-data in a couple
of around advices and disable undo-tree to get some basic commands to work.
I imagine there would be more cases to resolve.

Are the changes in the .diff what you had in mind roughly?

[-- Attachment #1.2: Type: text/html, Size: 556 bytes --]

[-- Attachment #2: match-data-corrupted-error.diff --]
[-- Type: application/octet-stream, Size: 3492 bytes --]

diff --git a/src/insdel.c b/src/insdel.c
index 8029291..ff90a93 100644
--- a/src/insdel.c
+++ b/src/insdel.c
@@ -53,6 +53,9 @@ static Lisp_Object combine_after_change_buffer;
 
 Lisp_Object Qinhibit_modification_hooks;
 
+/* Error condition used when change hooks inappropriately change match-data. */
+static Lisp_Object Qmatch_data_corrupted;
+
 static void signal_before_change (ptrdiff_t, ptrdiff_t, ptrdiff_t *);
 
 /* Also used in marker.c to enable expensive marker checks.  */
@@ -2034,6 +2037,10 @@ signal_after_change (ptrdiff_t charpos, ptrdiff_t lendel, ptrdiff_t lenins)
 
   specbind (Qinhibit_modification_hooks, Qt);
 
+  /* Change hook functions are expected to use save-match-data if using regexps.
+     Check (match-beginning 0) to verify hooks do not corrupt match-data. */
+  Lisp_Object match_beginning = match_limit(make_number(0), 1);
+
   if (!NILP (Vafter_change_functions))
     {
       Lisp_Object args[4];
@@ -2053,6 +2060,9 @@ signal_after_change (ptrdiff_t charpos, ptrdiff_t lendel, ptrdiff_t lenins)
       XSETCDR (rvoe_arg, Qt);
     }
 
+  if( ! EQ(match_beginning, match_limit(make_number(0), 1)) )
+    xsignal1 (Qmatch_data_corrupted, build_string ("Match data corrupted in after-change-functions; consider using save-match-data"));
+
   if (buffer_has_overlays ())
     report_overlay_modification (make_number (charpos),
 				 make_number (charpos + lenins),
@@ -2061,12 +2071,18 @@ signal_after_change (ptrdiff_t charpos, ptrdiff_t lendel, ptrdiff_t lenins)
 				 make_number (charpos + lenins),
 				 make_number (lendel));
 
+  if( ! EQ(match_beginning, match_limit(make_number(0), 1)) )
+    xsignal1 (Qmatch_data_corrupted, build_string ("Match data corrupted in overlay modification hooks; consider using save-match-data"));
+
   /* After an insertion, call the text properties
      insert-behind-hooks or insert-in-front-hooks.  */
   if (lendel == 0)
     report_interval_modification (make_number (charpos),
 				  make_number (charpos + lenins));
 
+  if( ! EQ(match_beginning, match_limit(make_number(0), 1)) )
+    xsignal1 (Qmatch_data_corrupted, build_string ("Match data corrupted in text property insertion hooks; consider using save-match-data"));
+
   unbind_to (count, Qnil);
 }
 
@@ -2181,5 +2197,12 @@ as well as hooks attached to text properties and overlays.  */);
   inhibit_modification_hooks = 0;
   DEFSYM (Qinhibit_modification_hooks, "inhibit-modification-hooks");
 
+  DEFSYM (Qmatch_data_corrupted, "match-data-corrupted");
+
+  Fput (Qmatch_data_corrupted, Qerror_conditions,
+	listn (CONSTYPE_PURE, 2, Qmatch_data_corrupted, Qerror));
+  Fput (Qmatch_data_corrupted, Qerror_message,
+	build_pure_c_string ("Match data corrupted"));
+
   defsubr (&Scombine_after_change_execute);
 }
diff --git a/src/lisp.h b/src/lisp.h
index e2c24ee..8596fa0 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3414,6 +3414,7 @@ extern bool check_existing (const char *);
 
 /* Defined in search.c.  */
 extern void shrink_regexp_cache (void);
+extern Lisp_Object match_limit (Lisp_Object, bool);
 extern void restore_search_regs (void);
 extern void record_unwind_save_match_data (void);
 struct re_registers;
diff --git a/src/search.c b/src/search.c
index ea36133..c5a50cb 100644
--- a/src/search.c
+++ b/src/search.c
@@ -2695,8 +2695,8 @@ since only regular expressions have distinguished subexpressions.  */)
 
   return Qnil;
 }
-\f
-static Lisp_Object
+
+Lisp_Object
 match_limit (Lisp_Object num, bool beginningp)
 {
   EMACS_INT n;