Re: Inefficient code in reftex-index.el

[storm@cua.dk (Kim F. Storm)]

Richard Stallman <rms@gnu.org> writes:

>     In any case, to me, the match-data interface should not be considered
>     a user-level feature _at all_.
>
> I don't agree.  It is legitimate for user code to call match-data
> directly.  

In special cases, yes.  

However, the majority of code should just use save-match-data.

>            There is no reason for the changes people are proposing.

IMO, getting rid of unused markers asap is a good thing.

Here is a different approach, adding an optional RESEAT arg to
match-data and set-match-data as I originally envisioned:

The majority of the patch is due to intrucing a new
record_unwind_save_match_data function and renaming restore_match_data
to restore_search_regs (for analogy with save_search_regs).

I didn't implement "match-count" in this patch -- it would be trivial
to add.

Index: subr.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/subr.el,v
retrieving revision 1.459
diff -c -r1.459 subr.el
*** subr.el	29 May 2005 08:34:46 -0000	1.459
--- subr.el	7 Jun 2005 14:40:17 -0000
***************
*** 1970,1976 ****
  	'((save-match-data-internal (match-data)))
  	(list 'unwind-protect
  	      (cons 'progn body)
! 	      '(set-match-data save-match-data-internal))))
  
  (defun match-string (num &optional string)
    "Return string of text matched by last search.
--- 1970,1976 ----
  	'((save-match-data-internal (match-data)))
  	(list 'unwind-protect
  	      (cons 'progn body)
! 	      '(set-match-data save-match-data-internal t))))
  
  (defun match-string (num &optional string)
    "Return string of text matched by last search.
Index: replace.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/replace.el,v
retrieving revision 1.211
diff -c -r1.211 replace.el
*** replace.el	26 May 2005 13:08:57 -0000	1.211
--- replace.el	7 Jun 2005 14:40:17 -0000
***************
*** 1268,1279 ****
  	     (and (eq new reuse)
  		  (eq (null integers) (markerp (car reuse)))
  		  new)))
!       (match-data integers
! 		  (prog1 reuse
! 		    (while reuse
! 		      (if (markerp (car reuse))
! 			  (set-marker (car reuse) nil))
! 		      (setq reuse (cdr reuse)))))))
  
  (defun replace-match-maybe-edit (newtext fixedcase literal noedit match-data)
    "Make a replacement with `replace-match', editing `\\?'.
--- 1268,1274 ----
  	     (and (eq new reuse)
  		  (eq (null integers) (markerp (car reuse)))
  		  new)))
!       (match-data integers reuse t)))
  
  (defun replace-match-maybe-edit (newtext fixedcase literal noedit match-data)
    "Make a replacement with `replace-match', editing `\\?'.


Index: search.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/search.c,v
retrieving revision 1.192
diff -c -r1.192 search.c
*** search.c	20 Apr 2005 07:21:47 -0000	1.192
--- search.c	7 Jun 2005 14:28:53 -0000
***************
*** 2739,2745 ****
    return match_limit (subexp, 0);
  }
  
! DEFUN ("match-data", Fmatch_data, Smatch_data, 0, 2, 0,
         doc: /* Return a list containing all info on what the last search matched.
  Element 2N is `(match-beginning N)'; element 2N + 1 is `(match-end N)'.
  All the elements are markers or nil (nil if the Nth pair didn't match)
--- 2739,2745 ----
    return match_limit (subexp, 0);
  }
  
! DEFUN ("match-data", Fmatch_data, Smatch_data, 0, 3, 0,
         doc: /* Return a list containing all info on what the last search matched.
  Element 2N is `(match-beginning N)'; element 2N + 1 is `(match-end N)'.
  All the elements are markers or nil (nil if the Nth pair didn't match)
***************
*** 2751,2767 ****
  this case, and if the last match was in a buffer, the buffer will get
  stored as one additional element at the end of the list.
  
! If REUSE is a list, reuse it as part of the value.  If REUSE is long enough
! to hold all the values, and if INTEGERS is non-nil, no consing is done.
  
  Return value is undefined if the last search failed.  */)
!      (integers, reuse)
!      Lisp_Object integers, reuse;
  {
    Lisp_Object tail, prev;
    Lisp_Object *data;
    int i, len;
  
    if (NILP (last_thing_searched))
      return Qnil;
  
--- 2751,2779 ----
  this case, and if the last match was in a buffer, the buffer will get
  stored as one additional element at the end of the list.
  
! If REUSE is a list, reuse it as part of the value.  If REUSE is long
! enough to hold all the values, and if INTEGERS is non-nil, no consing
! is done.
! 
! If optional third arg RESEAT is non-nil, any previous markers on the
! REUSE list will be modified to point to nowhere.
  
  Return value is undefined if the last search failed.  */)
!   (integers, reuse, reseat)
!      Lisp_Object integers, reuse, reseat;
  {
    Lisp_Object tail, prev;
    Lisp_Object *data;
    int i, len;
  
+   if (!NILP (reseat))
+     for (tail = reuse; CONSP (tail); tail = XCDR (tail))
+       if (MARKERP (XCAR (tail)))
+ 	{
+ 	  unchain_marker (XMARKER (XCAR (tail)));
+ 	  XSETCAR (tail, Qnil);
+ 	}
+ 
    if (NILP (last_thing_searched))
      return Qnil;
  
***************
*** 2797,2806 ****
  	    /* last_thing_searched must always be Qt, a buffer, or Qnil.  */
  	    abort ();
  
! 	  len = 2*(i+1);
  	}
        else
! 	data[2 * i] = data [2 * i + 1] = Qnil;
      }
  
    if (BUFFERP (last_thing_searched) && !NILP (integers))
--- 2809,2818 ----
  	    /* last_thing_searched must always be Qt, a buffer, or Qnil.  */
  	    abort ();
  
! 	  len = 2 * i + 2;
  	}
        else
! 	data[2 * i] = data[2 * i + 1] = Qnil;
      }
  
    if (BUFFERP (last_thing_searched) && !NILP (integers))
***************
*** 2834,2844 ****
  }
  
  
! DEFUN ("set-match-data", Fset_match_data, Sset_match_data, 1, 1, 0,
         doc: /* Set internal data on last search match from elements of LIST.
! LIST should have been created by calling `match-data' previously.  */)
!      (list)
!      register Lisp_Object list;
  {
    register int i;
    register Lisp_Object marker;
--- 2846,2858 ----
  }
  
  
! DEFUN ("set-match-data", Fset_match_data, Sset_match_data, 1, 2, 0,
         doc: /* Set internal data on last search match from elements of LIST.
! LIST should have been created by calling `match-data' previously.
! 
! If optional arg RESEAT is non-nil, make markers on LIST point nowhere.  */)
!     (list, reseat)
!      register Lisp_Object list, reseat;
  {
    register int i;
    register Lisp_Object marker;
***************
*** 2882,2890 ****
  	search_regs.num_regs = length;
        }
  
!     for (i = 0;; i++)
        {
! 	marker = Fcar (list);
  	if (BUFFERP (marker))
  	  {
  	    last_thing_searched = marker;
--- 2896,2904 ----
  	search_regs.num_regs = length;
        }
  
!     for (i = 0; CONSP (list); i++)
        {
! 	marker = XCAR (list);
  	if (BUFFERP (marker))
  	  {
  	    last_thing_searched = marker;
***************
*** 2895,2906 ****
  	if (NILP (marker))
  	  {
  	    search_regs.start[i] = -1;
! 	    list = Fcdr (list);
  	  }
  	else
  	  {
  	    int from;
  
  	    if (MARKERP (marker))
  	      {
  		if (XMARKER (marker)->buffer == 0)
--- 2909,2922 ----
  	if (NILP (marker))
  	  {
  	    search_regs.start[i] = -1;
! 	    list = XCDR (list);
  	  }
  	else
  	  {
  	    int from;
+ 	    Lisp_Object m;
  
+ 	    m = marker;
  	    if (MARKERP (marker))
  	      {
  		if (XMARKER (marker)->buffer == 0)
***************
*** 2911,2927 ****
  
  	    CHECK_NUMBER_COERCE_MARKER (marker);
  	    from = XINT (marker);
- 	    list = Fcdr (list);
  
! 	    marker = Fcar (list);
  	    if (MARKERP (marker) && XMARKER (marker)->buffer == 0)
  	      XSETFASTINT (marker, 0);
  
  	    CHECK_NUMBER_COERCE_MARKER (marker);
  	    search_regs.start[i] = from;
  	    search_regs.end[i] = XINT (marker);
  	  }
! 	list = Fcdr (list);
        }
  
      for (; i < search_regs.num_regs; i++)
--- 2927,2952 ----
  
  	    CHECK_NUMBER_COERCE_MARKER (marker);
  	    from = XINT (marker);
  
! 	    if (!NILP (reseat) && MARKERP (m))
! 	      unchain_marker (XMARKER (m));
! 
! 	    if ((list = XCDR (list), !CONSP (list)))
! 	      break;
! 
! 	    m = marker = XCAR (list);
! 
  	    if (MARKERP (marker) && XMARKER (marker)->buffer == 0)
  	      XSETFASTINT (marker, 0);
  
  	    CHECK_NUMBER_COERCE_MARKER (marker);
  	    search_regs.start[i] = from;
  	    search_regs.end[i] = XINT (marker);
+ 
+ 	    if (!NILP (reseat) && MARKERP (m))
+ 	      unchain_marker (XMARKER (m));
  	  }
! 	list = XCDR (list);
        }
  
      for (; i < search_regs.num_regs; i++)
***************
*** 2959,2965 ****
  
  /* Called upon exit from filters and sentinels. */
  void
! restore_match_data ()
  {
    if (search_regs_saved)
      {
--- 2984,2990 ----
  
  /* Called upon exit from filters and sentinels. */
  void
! restore_search_regs ()
  {
    if (search_regs_saved)
      {
***************
*** 2977,2982 ****
--- 3002,3022 ----
      }
  }
  
+ static Lisp_Object
+ unwind_set_match_data (list)
+      Lisp_Object list;
+ {
+   return Fset_match_data (list, Qt);
+ }
+ 
+ /* Called to unwind protect the match data.  */
+ void
+ record_unwind_save_match_data ()
+ {
+   record_unwind_protect (unwind_set_match_data,
+ 			 Fmatch_data (Qnil, Qnil, Qnil));
+ }
+ 
  /* Quote a string to inactivate reg-expr chars */
  
  DEFUN ("regexp-quote", Fregexp_quote, Sregexp_quote, 1, 1, 0,
Index: composite.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/composite.c,v
retrieving revision 1.31
diff -c -r1.31 composite.c
*** composite.c	26 Dec 2003 11:39:22 -0000	1.31
--- composite.c	7 Jun 2005 14:28:52 -0000
***************
*** 628,634 ****
      }
  
    /* Preserve the match data.  */
!   record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
  
    /* If none of ASCII characters have composition functions, we can
       skip them quickly.  */
--- 628,634 ----
      }
  
    /* Preserve the match data.  */
!   record_unwind_save_match_data ();
  
    /* If none of ASCII characters have composition functions, we can
       skip them quickly.  */
Index: eval.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/eval.c,v
retrieving revision 1.240
diff -c -r1.240 eval.c
*** eval.c	3 Jun 2005 23:02:21 -0000	1.240
--- eval.c	7 Jun 2005 14:28:52 -0000
***************
*** 1971,1977 ****
    GCPRO3 (fun, funname, fundef);
  
    /* Preserve the match data.  */
!   record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
  
    /* Value saved here is to be restored into Vautoload_queue.  */
    record_unwind_protect (un_autoload, Vautoload_queue);
--- 1971,1977 ----
    GCPRO3 (fun, funname, fundef);
  
    /* Preserve the match data.  */
!   record_unwind_save_match_data ();
  
    /* Value saved here is to be restored into Vautoload_queue.  */
    record_unwind_protect (un_autoload, Vautoload_queue);
Index: macmenu.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/macmenu.c,v
retrieving revision 1.28
diff -c -r1.28 macmenu.c
*** macmenu.c	6 Jun 2005 20:24:13 -0000	1.28
--- macmenu.c	7 Jun 2005 14:28:52 -0000
***************
*** 1475,1481 ****
  	 because it is not reentrant.  */
        specbind (Qdebug_on_next_call, Qnil);
  
!       record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
        if (NILP (Voverriding_local_map_menu_flag))
  	{
  	  specbind (Qoverriding_terminal_local_map, Qnil);
--- 1475,1481 ----
  	 because it is not reentrant.  */
        specbind (Qdebug_on_next_call, Qnil);
  
!       record_unwind_save_match_data ();
        if (NILP (Voverriding_local_map_menu_flag))
  	{
  	  specbind (Qoverriding_terminal_local_map, Qnil);
Index: process.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/process.c,v
retrieving revision 1.455
diff -c -r1.455 process.c
*** process.c	7 Jun 2005 13:19:25 -0000	1.455
--- process.c	7 Jun 2005 14:28:53 -0000
***************
*** 4888,4897 ****
  	{
  	  Lisp_Object tem;
  	  /* Don't clobber the CURRENT match data, either!  */
! 	  tem = Fmatch_data (Qnil, Qnil);
! 	  restore_match_data ();
! 	  record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
! 	  Fset_match_data (tem);
  	}
  
        /* For speed, if a search happens within this code,
--- 4888,4897 ----
  	{
  	  Lisp_Object tem;
  	  /* Don't clobber the CURRENT match data, either!  */
! 	  tem = Fmatch_data (Qnil, Qnil, Qnil);
! 	  restore_search_regs ();
! 	  record_unwind_save_match_data ();
! 	  Fset_match_data (tem, Qt);
  	}
  
        /* For speed, if a search happens within this code,
***************
*** 4945,4951 ****
  				   read_process_output_error_handler);
  
        /* If we saved the match data nonrecursively, restore it now.  */
!       restore_match_data ();
        running_asynch_code = outer_running_asynch_code;
  
        /* Handling the process output should not deactivate the mark.  */
--- 4945,4951 ----
  				   read_process_output_error_handler);
  
        /* If we saved the match data nonrecursively, restore it now.  */
!       restore_search_regs ();
        running_asynch_code = outer_running_asynch_code;
  
        /* Handling the process output should not deactivate the mark.  */
***************
*** 6349,6358 ****
    if (outer_running_asynch_code)
      {
        Lisp_Object tem;
!       tem = Fmatch_data (Qnil, Qnil);
!       restore_match_data ();
!       record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
!       Fset_match_data (tem);
      }
  
    /* For speed, if a search happens within this code,
--- 6349,6358 ----
    if (outer_running_asynch_code)
      {
        Lisp_Object tem;
!       tem = Fmatch_data (Qnil, Qnil, Qnil);
!       restore_search_regs ();
!       record_unwind_save_match_data ();
!       Fset_match_data (tem, Qt);
      }
  
    /* For speed, if a search happens within this code,
***************
*** 6366,6372 ****
  			     exec_sentinel_error_handler);
  
    /* If we saved the match data nonrecursively, restore it now.  */
!   restore_match_data ();
    running_asynch_code = outer_running_asynch_code;
  
    Vdeactivate_mark = odeactivate;
--- 6366,6372 ----
  			     exec_sentinel_error_handler);
  
    /* If we saved the match data nonrecursively, restore it now.  */
!   restore_search_regs ();
    running_asynch_code = outer_running_asynch_code;
  
    Vdeactivate_mark = odeactivate;
Index: w32menu.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/w32menu.c,v
retrieving revision 1.72
diff -c -r1.72 w32menu.c
*** w32menu.c	24 May 2005 08:44:25 -0000	1.72
--- w32menu.c	7 Jun 2005 14:28:54 -0000
***************
*** 1443,1449 ****
  	 because it is not reentrant.  */
        specbind (Qdebug_on_next_call, Qnil);
  
!       record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
        if (NILP (Voverriding_local_map_menu_flag))
  	{
  	  specbind (Qoverriding_terminal_local_map, Qnil);
--- 1443,1450 ----
  	 because it is not reentrant.  */
        specbind (Qdebug_on_next_call, Qnil);
  
!       record_unwind_save_match_data ();
! 
        if (NILP (Voverriding_local_map_menu_flag))
  	{
  	  specbind (Qoverriding_terminal_local_map, Qnil);
Index: xdisp.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v
retrieving revision 1.1019
diff -c -r1.1019 xdisp.c
*** xdisp.c	6 Jun 2005 12:36:29 -0000	1.1019
--- xdisp.c	7 Jun 2005 14:28:55 -0000
***************
*** 8458,8464 ****
        Lisp_Object tail, frame;
        int count = SPECPDL_INDEX ();
  
!       record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
  
        FOR_EACH_FRAME (tail, frame)
  	{
--- 8458,8464 ----
        Lisp_Object tail, frame;
        int count = SPECPDL_INDEX ();
  
!       record_unwind_save_match_data ();
  
        FOR_EACH_FRAME (tail, frame)
  	{
***************
*** 8581,8587 ****
  
  	  set_buffer_internal_1 (XBUFFER (w->buffer));
  	  if (save_match_data)
! 	    record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
  	  if (NILP (Voverriding_local_map_menu_flag))
  	    {
  	      specbind (Qoverriding_terminal_local_map, Qnil);
--- 8581,8587 ----
  
  	  set_buffer_internal_1 (XBUFFER (w->buffer));
  	  if (save_match_data)
! 	    record_unwind_save_match_data ();
  	  if (NILP (Voverriding_local_map_menu_flag))
  	    {
  	      specbind (Qoverriding_terminal_local_map, Qnil);
***************
*** 8772,8778 ****
  
  	  /* Save match data, if we must.  */
  	  if (save_match_data)
! 	    record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
  
  	  /* Make sure that we don't accidentally use bogus keymaps.  */
  	  if (NILP (Voverriding_local_map_menu_flag))
--- 8772,8778 ----
  
  	  /* Save match data, if we must.  */
  	  if (save_match_data)
! 	    record_unwind_save_match_data ();
  
  	  /* Make sure that we don't accidentally use bogus keymaps.  */
  	  if (NILP (Voverriding_local_map_menu_flag))
Index: xmenu.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xmenu.c,v
retrieving revision 1.292
diff -c -r1.292 xmenu.c
*** xmenu.c	6 Jun 2005 12:56:53 -0000	1.292
--- xmenu.c	7 Jun 2005 14:28:55 -0000
***************
*** 2030,2036 ****
  	 because it is not reentrant.  */
        specbind (Qdebug_on_next_call, Qnil);
  
!       record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
        record_unwind_protect (unuse_menu_items, Qnil);
        if (NILP (Voverriding_local_map_menu_flag))
  	{
--- 2030,2036 ----
  	 because it is not reentrant.  */
        specbind (Qdebug_on_next_call, Qnil);
  
!       record_unwind_save_match_data ();
        record_unwind_protect (unuse_menu_items, Qnil);
        if (NILP (Voverriding_local_map_menu_flag))
  	{
cvs diff: I know nothing about config.h
cvs diff: I know nothing about epaths.h
Index: lisp.h
===================================================================
RCS file: /cvsroot/emacs/emacs/src/lisp.h,v
retrieving revision 1.528
diff -c -r1.528 lisp.h
*** lisp.h	24 May 2005 03:44:09 -0000	1.528
--- lisp.h	7 Jun 2005 14:28:55 -0000
***************
*** 2768,2773 ****
--- 2768,2774 ----
  EXFUN (Fbuffer_enable_undo, 1);
  EXFUN (Ferase_buffer, 0);
  extern Lisp_Object Qoverlayp;
+ extern Lisp_Object Qevaporate;
  extern Lisp_Object get_truename_buffer P_ ((Lisp_Object));
  extern struct buffer *all_buffers;
  EXFUN (Fprevious_overlay_change, 1);
***************
*** 2835,2845 ****
  /* defined in search.c */
  extern void shrink_regexp_cache P_ ((void));
  EXFUN (Fstring_match, 3);
! extern void restore_match_data P_ ((void));
! EXFUN (Fmatch_data, 2);
! EXFUN (Fset_match_data, 1);
  EXFUN (Fmatch_beginning, 1);
  EXFUN (Fmatch_end, 1);
  EXFUN (Flooking_at, 1);
  extern int fast_string_match P_ ((Lisp_Object, Lisp_Object));
  extern int fast_c_string_match_ignore_case P_ ((Lisp_Object, const char *));
--- 2836,2847 ----
  /* defined in search.c */
  extern void shrink_regexp_cache P_ ((void));
  EXFUN (Fstring_match, 3);
! extern void restore_search_regs P_ ((void));
! EXFUN (Fmatch_data, 3);
! EXFUN (Fset_match_data, 2);
  EXFUN (Fmatch_beginning, 1);
  EXFUN (Fmatch_end, 1);
+ extern void record_unwind_save_match_data P_ ((void));
  EXFUN (Flooking_at, 1);
  extern int fast_string_match P_ ((Lisp_Object, Lisp_Object));
  extern int fast_c_string_match_ignore_case P_ ((Lisp_Object, const char *));

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk