* Inefficient code in reftex-index.el @ 2005-06-06 12:08 Kim F. Storm 2005-06-06 12:39 ` David Kastrup 0 siblings, 1 reply; 35+ messages in thread From: Kim F. Storm @ 2005-06-06 12:08 UTC (permalink / raw) Cc: emacs-devel I noticed the following code in reftex-index.el: (condition-case nil (texmathp) (error nil)))) (setq beg (car (match-data)) end (nth 1 (match-data))) Using match-data like that seems inefficient. I suggtest using match-beginning/match-end instead. -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-06 12:08 Inefficient code in reftex-index.el Kim F. Storm @ 2005-06-06 12:39 ` David Kastrup 2005-06-06 13:52 ` Stefan Monnier ` (2 more replies) 0 siblings, 3 replies; 35+ messages in thread From: David Kastrup @ 2005-06-06 12:39 UTC (permalink / raw) Cc: emacs-devel, Carsten Dominik storm@cua.dk (Kim F. Storm) writes: > I noticed the following code in reftex-index.el: > > (condition-case nil (texmathp) (error nil)))) > (setq beg (car (match-data)) > end (nth 1 (match-data))) > > Using match-data like that seems inefficient. > > I suggtest using match-beginning/match-end instead. That is not the same: the above will set beg and end to markers, whereas match-beginning/match-end happen to be integers. However, the above will also create markers that are unused, so it would be saner to call (match-data) only once _if_ indeed markers are what is required. Unused markers slow done editing operations afterwards. So even if markers _are_ what is wanted for some reason, they should be explicitly unseated with set-marker once they are no longer needed. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-06 12:39 ` David Kastrup @ 2005-06-06 13:52 ` Stefan Monnier 2005-06-06 14:14 ` Kim F. Storm 2005-06-06 14:15 ` Juanma Barranquero 2005-06-06 14:18 ` Kim F. Storm 2005-06-06 14:41 ` Carsten Dominik 2 siblings, 2 replies; 35+ messages in thread From: Stefan Monnier @ 2005-06-06 13:52 UTC (permalink / raw) Cc: emacs-devel, Carsten Dominik, Kim F. Storm > That is not the same: the above will set beg and end to markers, > whereas match-beginning/match-end happen to be integers. However, the If you want markers, then say so: (make-marker (match-beginning N)) Stefan ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-06 13:52 ` Stefan Monnier @ 2005-06-06 14:14 ` Kim F. Storm 2005-06-06 14:15 ` Juanma Barranquero 1 sibling, 0 replies; 35+ messages in thread From: Kim F. Storm @ 2005-06-06 14:14 UTC (permalink / raw) Cc: Carsten Dominik, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> That is not the same: the above will set beg and end to markers, >> whereas match-beginning/match-end happen to be integers. However, the > > If you want markers, then say so: (make-marker (match-beginning N)) Yes, indeed. -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-06 13:52 ` Stefan Monnier 2005-06-06 14:14 ` Kim F. Storm @ 2005-06-06 14:15 ` Juanma Barranquero 1 sibling, 0 replies; 35+ messages in thread From: Juanma Barranquero @ 2005-06-06 14:15 UTC (permalink / raw) On 6/6/05, Stefan Monnier <monnier@iro.umontreal.ca> wrote: > If you want markers, then say so: (make-marker (match-beginning N)) (copy-marker (match-beginning-N)) or (set-marker (make-marker) (match-beginning-N)) perhaps? -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-06 12:39 ` David Kastrup 2005-06-06 13:52 ` Stefan Monnier @ 2005-06-06 14:18 ` Kim F. Storm 2005-06-06 22:24 ` Kim F. Storm 2005-06-07 12:24 ` Richard Stallman 2005-06-06 14:41 ` Carsten Dominik 2 siblings, 2 replies; 35+ messages in thread From: Kim F. Storm @ 2005-06-06 14:18 UTC (permalink / raw) Cc: emacs-devel, Carsten Dominik David Kastrup <dak@gnu.org> writes: > That is not the same: the above will set beg and end to markers, How "clever" ... I would NEVER have guessed that. > whereas match-beginning/match-end happen to be integers. However, the > above will also create markers that are unused, so it would be saner > to call (match-data) only once _if_ indeed markers are what is > required. > > Unused markers slow done editing operations afterwards. So even if > markers _are_ what is wanted for some reason, they should be > explicitly unseated with set-marker once they are no longer needed. Which brings me to the suggestion that we add an optional arg to set-match-data like this: (set-match-data list &optional destroy-markers) and change save-match-data to use it (defmacro save-match-data (&rest body) "Execute the BODY forms, restoring the global value of the match data. The value returned is the value of the last form in BODY." ;; It is better not to use backquote here, ;; because that makes a bootstrapping problem ;; if you need to recompile all the Lisp files using interpreted code. (declare (indent 0) (debug t)) (list 'let '((save-match-data-internal (match-data))) (list 'unwind-protect (cons 'progn body) '(set-match-data save-match-data-internal t)))) -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-06 14:18 ` Kim F. Storm @ 2005-06-06 22:24 ` Kim F. Storm 2005-06-06 23:55 ` David Kastrup 2005-06-08 12:01 ` Richard Stallman 2005-06-07 12:24 ` Richard Stallman 1 sibling, 2 replies; 35+ messages in thread From: Kim F. Storm @ 2005-06-06 22:24 UTC (permalink / raw) Cc: Carsten Dominik, emacs-devel storm@cua.dk (Kim F. Storm) writes: > > Which brings me to the suggestion that we add an optional arg to > set-match-data like this: > > (set-match-data list &optional destroy-markers) > > and change save-match-data to use it > > (defmacro save-match-data (&rest body) > "Execute the BODY forms, restoring the global value of the match data. > The value returned is the value of the last form in BODY." > ;; It is better not to use backquote here, > ;; because that makes a bootstrapping problem > ;; if you need to recompile all the Lisp files using interpreted code. > (declare (indent 0) (debug t)) > (list 'let > '((save-match-data-internal (match-data))) > (list 'unwind-protect > (cons 'progn body) > '(set-match-data save-match-data-internal t)))) I made the change slightly different to avoid adding another arg to Fset_match_data, which causes problems at C level. Instead, if first arg to match-data is 'evaporate', set-match-data will eventually free markers on the resulting list... Here is a patch to implement this, and use it at the C-level, and for save-match-data: I don't know for sure, but emacs feels a tiny bit more responsive with this change! Index: src/composite.c =================================================================== RCS file: /cvsroot/emacs/emacs/src/composite.c,v retrieving revision 1.31 diff -c -r1.31 composite.c *** src/composite.c 26 Dec 2003 11:39:22 -0000 1.31 --- src/composite.c 6 Jun 2005 22:19:14 -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_protect (Fset_match_data, Fmatch_data (Qevaporate, Qnil)); /* If none of ASCII characters have composition functions, we can skip them quickly. */ Index: src/eval.c =================================================================== RCS file: /cvsroot/emacs/emacs/src/eval.c,v retrieving revision 1.240 diff -c -r1.240 eval.c *** src/eval.c 3 Jun 2005 23:02:21 -0000 1.240 --- src/eval.c 6 Jun 2005 22:19:15 -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_protect (Fset_match_data, Fmatch_data (Qevaporate, Qnil)); /* Value saved here is to be restored into Vautoload_queue. */ record_unwind_protect (un_autoload, Vautoload_queue); cvs diff: I know nothing about src/lisp.c Index: src/macmenu.c =================================================================== RCS file: /cvsroot/emacs/emacs/src/macmenu.c,v retrieving revision 1.28 diff -c -r1.28 macmenu.c *** src/macmenu.c 6 Jun 2005 20:24:13 -0000 1.28 --- src/macmenu.c 6 Jun 2005 22:19:15 -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_protect (Fset_match_data, Fmatch_data (Qevaporate, Qnil)); if (NILP (Voverriding_local_map_menu_flag)) { specbind (Qoverriding_terminal_local_map, Qnil); Index: src/search.c =================================================================== RCS file: /cvsroot/emacs/emacs/src/search.c,v retrieving revision 1.192 diff -c -r1.192 search.c *** src/search.c 20 Apr 2005 07:21:47 -0000 1.192 --- src/search.c 6 Jun 2005 22:19:15 -0000 *************** *** 2751,2777 **** 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; prev = Qnil; ! data = (Lisp_Object *) alloca ((2 * search_regs.num_regs + 1) * sizeof (Lisp_Object)); len = 0; ! for (i = 0; i < search_regs.num_regs; i++) { int start = search_regs.start[i]; if (start >= 0) --- 2751,2803 ---- 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 INTEGER is the symbol `evaporate', mark the returned list so that ! any markers on the list shall be automatically freed when passed to ! `set-match-data'. ! ! 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. ! Note: 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) Lisp_Object integers, reuse; { Lisp_Object tail, prev; Lisp_Object *data; ! int i, j, len; if (NILP (last_thing_searched)) return Qnil; prev = Qnil; ! for (tail = reuse; CONSP (tail); tail = XCDR (tail)) ! if (MARKERP (XCAR (tail))) ! { ! unchain_marker (XMARKER (XCAR (tail))); ! XSETCAR (tail, Qnil); ! } ! ! data = (Lisp_Object *) alloca ((2 * search_regs.num_regs + 2) * sizeof (Lisp_Object)); + j = 0; len = 0; ! ! if (EQ (integers, Qevaporate)) ! { ! if (BUFFERP (last_thing_searched)) ! { ! data[j++] = Qevaporate; ! len++; ! } ! integers = Qnil; ! } ! ! for (i = 0; i < search_regs.num_regs; i++, j += 2) { int start = search_regs.start[i]; if (start >= 0) *************** *** 2779,2795 **** if (EQ (last_thing_searched, Qt) || ! NILP (integers)) { ! XSETFASTINT (data[2 * i], start); ! XSETFASTINT (data[2 * i + 1], search_regs.end[i]); } else if (BUFFERP (last_thing_searched)) { ! data[2 * i] = Fmake_marker (); ! Fset_marker (data[2 * i], make_number (start), last_thing_searched); ! data[2 * i + 1] = Fmake_marker (); ! Fset_marker (data[2 * i + 1], make_number (search_regs.end[i]), last_thing_searched); } --- 2805,2821 ---- if (EQ (last_thing_searched, Qt) || ! NILP (integers)) { ! XSETFASTINT (data[j], start); ! XSETFASTINT (data[j + 1], search_regs.end[i]); } else if (BUFFERP (last_thing_searched)) { ! data[j] = Fmake_marker (); ! Fset_marker (data[j], make_number (start), last_thing_searched); ! data[j + 1] = Fmake_marker (); ! Fset_marker (data[j + 1], make_number (search_regs.end[i]), last_thing_searched); } *************** *** 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)) --- 2823,2832 ---- /* last_thing_searched must always be Qt, a buffer, or Qnil. */ abort (); ! len = j + 2; } else ! data[j] = data[j + 1] = Qnil; } if (BUFFERP (last_thing_searched) && !NILP (integers)) *************** *** 2836,2847 **** 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; if (running_asynch_code) save_search_regs (); --- 2862,2878 ---- 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. ! ! If CLEAR-MARKERS is non-nil, make markers on the list point nowhere. ! If value is `evaporate', put the markers back on the free list; the ! caller must ensure that there are no other references to these markers. */) ! (list) register Lisp_Object list; { register int i; register Lisp_Object marker; + int free_markers = 0; if (running_asynch_code) save_search_regs (); *************** *** 2853,2858 **** --- 2884,2897 ---- in LIST, assume that this match data came from a string. */ last_thing_searched = Qt; + /* If first element of list is `evaporate', free all markers + on the list after restoring search registers. */ + if (CONSP (list) && EQ (XCAR (list), Qevaporate)) + { + free_markers = 1; + list = XCDR (list); + } + /* Allocate registers if they don't already exist. */ { int length = XFASTINT (Flength (list)) / 2; *************** *** 2882,2890 **** search_regs.num_regs = length; } ! for (i = 0;; i++) { ! marker = Fcar (list); if (BUFFERP (marker)) { last_thing_searched = marker; --- 2921,2929 ---- search_regs.num_regs = length; } ! for (i = 0; CONSP (list); i++) { ! marker = XCAR (list); if (BUFFERP (marker)) { last_thing_searched = marker; *************** *** 2900,2906 **** --- 2939,2947 ---- 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++) --- 2952,2985 ---- CHECK_NUMBER_COERCE_MARKER (marker); from = XINT (marker); ! if (free_markers && MARKERP (m)) ! { ! XSETCAR (list, Qnil); ! free_marker (m); ! } ! ! list = XCDR (list); ! ! if (!CONSP (list)) ! break; ! ! m = 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); + + if (free_markers && MARKERP (m)) + { + XSETCAR (list, Qnil); + free_marker (m); + } } ! list = XCDR (list); } for (; i < search_regs.num_regs; i++) Index: src/w32menu.c =================================================================== RCS file: /cvsroot/emacs/emacs/src/w32menu.c,v retrieving revision 1.72 diff -c -r1.72 w32menu.c *** src/w32menu.c 24 May 2005 08:44:25 -0000 1.72 --- src/w32menu.c 6 Jun 2005 22:19:15 -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,1449 ---- because it is not reentrant. */ specbind (Qdebug_on_next_call, Qnil); ! record_unwind_protect (Fset_match_data, Fmatch_data (Qevaporate, Qnil)); if (NILP (Voverriding_local_map_menu_flag)) { specbind (Qoverriding_terminal_local_map, Qnil); Index: src/xdisp.c =================================================================== RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v retrieving revision 1.1019 diff -c -r1.1019 xdisp.c *** src/xdisp.c 6 Jun 2005 12:36:29 -0000 1.1019 --- src/xdisp.c 6 Jun 2005 22:19:17 -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_protect (Fset_match_data, Fmatch_data (Qevaporate, Qnil)); 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_protect (Fset_match_data, Fmatch_data (Qevaporate, Qnil)); 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_protect (Fset_match_data, Fmatch_data (Qevaporate, Qnil)); /* Make sure that we don't accidentally use bogus keymaps. */ if (NILP (Voverriding_local_map_menu_flag)) Index: src/xmenu.c =================================================================== RCS file: /cvsroot/emacs/emacs/src/xmenu.c,v retrieving revision 1.292 diff -c -r1.292 xmenu.c *** src/xmenu.c 6 Jun 2005 12:56:53 -0000 1.292 --- src/xmenu.c 6 Jun 2005 22:19:18 -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_protect (Fset_match_data, Fmatch_data (Qevaporate, Qnil)); record_unwind_protect (unuse_menu_items, Qnil); if (NILP (Voverriding_local_map_menu_flag)) { Index: lisp/subr.el =================================================================== RCS file: /cvsroot/emacs/emacs/lisp/subr.el,v retrieving revision 1.459 diff -c -r1.459 subr.el *** lisp/subr.el 29 May 2005 08:34:46 -0000 1.459 --- lisp/subr.el 6 Jun 2005 22:19:18 -0000 *************** *** 1967,1973 **** ;; if you need to recompile all the Lisp files using interpreted code. (declare (indent 0) (debug t)) (list 'let ! '((save-match-data-internal (match-data))) (list 'unwind-protect (cons 'progn body) '(set-match-data save-match-data-internal)))) --- 1967,1973 ---- ;; if you need to recompile all the Lisp files using interpreted code. (declare (indent 0) (debug t)) (list 'let ! '((save-match-data-internal (match-data 'evaporate))) (list 'unwind-protect (cons 'progn body) '(set-match-data save-match-data-internal)))) -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-06 22:24 ` Kim F. Storm @ 2005-06-06 23:55 ` David Kastrup 2005-06-07 8:46 ` Kim F. Storm 2005-06-08 12:01 ` Richard Stallman 1 sibling, 1 reply; 35+ messages in thread From: David Kastrup @ 2005-06-06 23:55 UTC (permalink / raw) Cc: Carsten Dominik, emacs-devel storm@cua.dk (Kim F. Storm) writes: > storm@cua.dk (Kim F. Storm) writes: > >> >> Which brings me to the suggestion that we add an optional arg to >> set-match-data like this: >> >> (set-match-data list &optional destroy-markers) >> >> and change save-match-data to use it >> >> (defmacro save-match-data (&rest body) >> "Execute the BODY forms, restoring the global value of the match data. >> The value returned is the value of the last form in BODY." >> ;; It is better not to use backquote here, >> ;; because that makes a bootstrapping problem >> ;; if you need to recompile all the Lisp files using interpreted code. >> (declare (indent 0) (debug t)) >> (list 'let >> '((save-match-data-internal (match-data))) >> (list 'unwind-protect >> (cons 'progn body) >> '(set-match-data save-match-data-internal t)))) > > I made the change slightly different to avoid adding another arg to > Fset_match_data, which causes problems at C level. Instead, if first > arg to match-data is 'evaporate', set-match-data will eventually free > markers on the resulting list... So if INTEGERS is 'evaporate, we don't get integers but markers, and those will be unseated at restore time. In addition, the list will get prolonged by one additional member. This is probably not too tragic since I don't see anybody else accessing a data structure created with (set-match-data 'evaporate), but it is hardly an element of beauty. Should not (match-data whatever '(evaporate marker1 marker2 ...)) also clean up the markers passed in for reuse? Anyway, allow me to throw a fit. The whole match-data thing was ugly previously, but this surely takes the cake, platter and all. At the very least, the INTEGERS argument should be renamed to something more in line with the new function. But I don't like that interface at all, really. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-06 23:55 ` David Kastrup @ 2005-06-07 8:46 ` Kim F. Storm 2005-06-07 9:23 ` David Kastrup ` (2 more replies) 0 siblings, 3 replies; 35+ messages in thread From: Kim F. Storm @ 2005-06-07 8:46 UTC (permalink / raw) Cc: Carsten Dominik, emacs-devel David Kastrup <dak@gnu.org> writes: >> I made the change slightly different to avoid adding another arg to >> Fset_match_data, which causes problems at C level. Instead, if first >> arg to match-data is 'evaporate', set-match-data will eventually free >> markers on the resulting list... > > So if INTEGERS is 'evaporate, we don't get integers but markers, and > those will be unseated at restore time. Right -- either you want integers or you want markers (that evaporates). There is no ambiguity in that respect! > > In addition, the list will get prolonged by one additional member. > This is probably not too tragic since I don't see anybody else > accessing a data structure created with (set-match-data 'evaporate), That's (match-data 'evaporate)... > but it is hardly an element of beauty. The new interface is fully backwards compatible -- so unless the user specifies 'evaporate, the list is formatted exactly as it was before. > Should not > (match-data whatever '(evaporate marker1 marker2 ...)) also clean > up the markers passed in for reuse? It should, yes. Currently, it just reseats the markers on the list. > Anyway, allow me to throw a fit. The whole match-data thing was ugly > previously, but this surely takes the cake, platter and all. What are the alternatives? I can add new optional arguments to both match-data and set-match-data to indicate what to do with markers in the REUSE and LIST arguments. I can do that if people think it is better/cleaner. The drawback is that it requires more changes at the C-level -- but it may actually clean-up a few things, so perhaps it would be good... In any case, to me, the match-data interface should not be considered a user-level feature _at all_. The user level feature is save-match-data, which does not (in principle) allow the user to mess with the saved data. And in that respect, nobody should really care about what the format of the match data is. IMO, the byte-compiler should warn about using match-data. > > At the very least, the INTEGERS argument should be renamed to > something more in line with the new function. I will do that, if the current interface is acceptable. > But I don't like that > interface at all, really. I'm not excited about it either... but it does the job. -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 8:46 ` Kim F. Storm @ 2005-06-07 9:23 ` David Kastrup 2005-06-07 10:38 ` Kim F. Storm 2005-06-07 14:28 ` Richard Stallman 2005-06-07 18:11 ` Stefan Monnier 2 siblings, 1 reply; 35+ messages in thread From: David Kastrup @ 2005-06-07 9:23 UTC (permalink / raw) Cc: Carsten Dominik, emacs-devel storm@cua.dk (Kim F. Storm) writes: > David Kastrup <dak@gnu.org> writes: > >>> I made the change slightly different to avoid adding another arg to >>> Fset_match_data, which causes problems at C level. Instead, if first >>> arg to match-data is 'evaporate', set-match-data will eventually free >>> markers on the resulting list... >> >> So if INTEGERS is 'evaporate, we don't get integers but markers, and >> those will be unseated at restore time. > > Right -- either you want integers or you want markers (that evaporates). > There is no ambiguity in that respect! > >> >> In addition, the list will get prolonged by one additional member. >> This is probably not too tragic since I don't see anybody else >> accessing a data structure created with (set-match-data 'evaporate), > > That's (match-data 'evaporate)... > >> but it is hardly an element of beauty. > > The new interface is fully backwards compatible -- so unless the user > specifies 'evaporate, the list is formatted exactly as it was > before. Anyway, match-data states somehing like match-data is a built-in function in `src/search.c'. (match-data &optional INTEGERS REUSE) 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) if the last match was on a buffer; integers or nil if a string was matched. Use `store-match-data' to reinstate the data in this list. If INTEGERS (the optional first argument) is non-nil, always use integers (rather than markers) to represent buffer positions. In 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. Notice the last sentence. This keeps the description for Element 2N and Element 2N+1 valid. I think that, if at all, the evaporate thingy should also occupy the last element on the list rather than the first. We got enough inconsistency already as it is. >> Should not (match-data whatever '(evaporate marker1 marker2 ...)) >> also clean up the markers passed in for reuse? > > It should, yes. Currently, it just reseats the markers on the list. Oh, it does? I thought that it just recycled the CONS cells, but not the markers. > In any case, to me, the match-data interface should not be > considered a user-level feature _at all_. There is no other interface into the number of accessible match strings (which might be nil) rather than (/ (length (match-data t)) 2). -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 9:23 ` David Kastrup @ 2005-06-07 10:38 ` Kim F. Storm 2005-06-07 11:05 ` David Kastrup 2005-06-07 14:28 ` Richard Stallman 0 siblings, 2 replies; 35+ messages in thread From: Kim F. Storm @ 2005-06-07 10:38 UTC (permalink / raw) Cc: emacs-devel, Carsten Dominik David Kastrup <dak@gnu.org> writes: > If INTEGERS (the optional first argument) is non-nil, always use > integers (rather than markers) to represent buffer positions. In > 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. > > Notice the last sentence. This keeps the description for Element 2N > and Element 2N+1 valid. > > I think that, if at all, the evaporate thingy should also occupy the > last element on the list rather than the first. We got enough > inconsistency already as it is. That's probably a good idea -- I'll give it a try. > >>> Should not (match-data whatever '(evaporate marker1 marker2 ...)) >>> also clean up the markers passed in for reuse? >> >> It should, yes. Currently, it just reseats the markers on the list. > > Oh, it does? I thought that it just recycled the CONS cells, but not > the markers. It used to do that, but my patch changed it to reseat the markers. In any case, I didn't find any code which actually uses the REUSE form of match-data, so that feature is pretty obscure already, so I think we should just keep the original behavior of the REUSE arg. > >> In any case, to me, the match-data interface should not be >> considered a user-level feature _at all_. > > There is no other interface into the number of accessible match > strings (which might be nil) rather than > (/ (length (match-data t)) 2). That's still pretty inefficient -- I suggest that we introduce a new function `match-count' to return that number. -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 10:38 ` Kim F. Storm @ 2005-06-07 11:05 ` David Kastrup 2005-06-07 11:28 ` Kim F. Storm 2005-06-07 14:28 ` Richard Stallman 1 sibling, 1 reply; 35+ messages in thread From: David Kastrup @ 2005-06-07 11:05 UTC (permalink / raw) Cc: emacs-devel, Carsten Dominik storm@cua.dk (Kim F. Storm) writes: > In any case, I didn't find any code which actually uses the REUSE > form of match-data, so that feature is pretty obscure already, so I > think we should just keep the original behavior of the REUSE arg. Look at replace-match-data in replace.el... I think it was me that did that piece of prescient paranoia. >>> In any case, to me, the match-data interface should not be >>> considered a user-level feature _at all_. >> >> There is no other interface into the number of accessible match >> strings (which might be nil) rather than >> (/ (length (match-data t)) 2). > > That's still pretty inefficient -- I suggest that we introduce a new > function `match-count' to return that number. No objection here. No idea whether match-size would be more appropriate. The C code is not really helpful deciding about that since it uses search_regs.num_regs and that is not suggestive of any sensible Lisp name. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 11:05 ` David Kastrup @ 2005-06-07 11:28 ` Kim F. Storm 0 siblings, 0 replies; 35+ messages in thread From: Kim F. Storm @ 2005-06-07 11:28 UTC (permalink / raw) Cc: Carsten Dominik, emacs-devel David Kastrup <dak@gnu.org> writes: > storm@cua.dk (Kim F. Storm) writes: > >> In any case, I didn't find any code which actually uses the REUSE >> form of match-data, so that feature is pretty obscure already, so I >> think we should just keep the original behavior of the REUSE arg. > > Look at replace-match-data in replace.el... I see. But that doesn't contradict my feeling that match-data doesn't have to do anything special about markers in REUSE. But we could make an optional arg RESEAT to match-data which causes the markers in REUSE to be reseated to nowhere (if t), or be freed if RESEAT equals `evaporate'. > > I think it was me that did that piece of prescient paranoia. > >>>> In any case, to me, the match-data interface should not be >>>> considered a user-level feature _at all_. >>> >>> There is no other interface into the number of accessible match >>> strings (which might be nil) rather than >>> (/ (length (match-data t)) 2). >> >> That's still pretty inefficient -- I suggest that we introduce a new >> function `match-count' to return that number. > > No objection here. No idea whether match-size would be more > appropriate. The C code is not really helpful deciding about that > since it uses search_regs.num_regs and that is not suggestive of any > sensible Lisp name. IMO, match-count is better than match-size. BTW, match-count != search_regs.num_regs, as match-data only returns elements up to the last successful submatch. -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 10:38 ` Kim F. Storm 2005-06-07 11:05 ` David Kastrup @ 2005-06-07 14:28 ` Richard Stallman 2005-06-07 14:46 ` Kim F. Storm 2005-06-07 18:06 ` Stefan Monnier 1 sibling, 2 replies; 35+ messages in thread From: Richard Stallman @ 2005-06-07 14:28 UTC (permalink / raw) Cc: dominik, emacs-devel > There is no other interface into the number of accessible match > strings (which might be nil) rather than > (/ (length (match-data t)) 2). That's still pretty inefficient -- I suggest that we introduce a new function `match-count' to return that number. Is there sufficient use for this function to justify introducing it? I think that most cases where this would be used, the code would then proceed to call match-data. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 14:28 ` Richard Stallman @ 2005-06-07 14:46 ` Kim F. Storm 2005-06-07 18:06 ` Stefan Monnier 1 sibling, 0 replies; 35+ messages in thread From: Kim F. Storm @ 2005-06-07 14:46 UTC (permalink / raw) Cc: dominik, emacs-devel Richard Stallman <rms@gnu.org> writes: > > There is no other interface into the number of accessible match > > strings (which might be nil) rather than > > (/ (length (match-data t)) 2). > > That's still pretty inefficient -- I suggest that we introduce a new > function `match-count' to return that number. > > Is there sufficient use for this function to justify introducing it? > I think that most cases where this would be used, the code would > then proceed to call match-data. I don't know. -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 14:28 ` Richard Stallman 2005-06-07 14:46 ` Kim F. Storm @ 2005-06-07 18:06 ` Stefan Monnier 2005-06-08 8:44 ` Kim F. Storm ` (2 more replies) 1 sibling, 3 replies; 35+ messages in thread From: Stefan Monnier @ 2005-06-07 18:06 UTC (permalink / raw) Cc: dominik, emacs-devel, Kim F. Storm >> There is no other interface into the number of accessible match >> strings (which might be nil) rather than >> (/ (length (match-data t)) 2). > That's still pretty inefficient -- I suggest that we introduce a new > function `match-count' to return that number. > Is there sufficient use for this function to justify introducing it? > I think that most cases where this would be used, the code would > then proceed to call match-data. Actually the only cases I can vaguely remember using the (/ (length (match-data)) 2) idiom didn't use the whole (match-data). They typically used the idiom in order to know *which* subgroup matched (of course it only works if you craft your regexp carefully). Stefan ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 18:06 ` Stefan Monnier @ 2005-06-08 8:44 ` Kim F. Storm 2005-06-08 9:47 ` David Kastrup 2005-06-08 15:59 ` Richard Stallman 2 siblings, 0 replies; 35+ messages in thread From: Kim F. Storm @ 2005-06-08 8:44 UTC (permalink / raw) Cc: emacs-devel, rms, dominik Stefan Monnier <monnier@iro.umontreal.ca> writes: >>> There is no other interface into the number of accessible match >>> strings (which might be nil) rather than >>> (/ (length (match-data t)) 2). > >> That's still pretty inefficient -- I suggest that we introduce a new >> function `match-count' to return that number. > >> Is there sufficient use for this function to justify introducing it? >> I think that most cases where this would be used, the code would >> then proceed to call match-data. > > Actually the only cases I can vaguely remember using the > (/ (length (match-data)) 2) idiom didn't use the whole (match-data). > They typically used the idiom in order to know *which* subgroup matched (of > course it only works if you craft your regexp carefully). Maybe this would be more versatile: (match-subgroups) => "match count" (match-subgroups t) => list of subgroups that matches -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 18:06 ` Stefan Monnier 2005-06-08 8:44 ` Kim F. Storm @ 2005-06-08 9:47 ` David Kastrup 2005-06-08 10:00 ` Kim F. Storm 2005-06-08 15:59 ` Richard Stallman 2 siblings, 1 reply; 35+ messages in thread From: David Kastrup @ 2005-06-08 9:47 UTC (permalink / raw) Cc: emacs-devel, rms, Kim F. Storm, dominik Stefan Monnier <monnier@iro.umontreal.ca> writes: >>> There is no other interface into the number of accessible match >>> strings (which might be nil) rather than >>> (/ (length (match-data t)) 2). > >> That's still pretty inefficient -- I suggest that we introduce a new >> function `match-count' to return that number. > >> Is there sufficient use for this function to justify introducing it? >> I think that most cases where this would be used, the code would >> then proceed to call match-data. > > Actually the only cases I can vaguely remember using the > (/ (length (match-data)) 2) idiom didn't use the whole (match-data). > They typically used the idiom in order to know *which* subgroup matched (of > course it only works if you craft your regexp carefully). That would not work. (length (match-data)) is a property of the regexp, not the match. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-08 9:47 ` David Kastrup @ 2005-06-08 10:00 ` Kim F. Storm 2005-06-08 10:11 ` David Kastrup 0 siblings, 1 reply; 35+ messages in thread From: Kim F. Storm @ 2005-06-08 10:00 UTC (permalink / raw) Cc: dominik, emacs-devel, Stefan Monnier, rms David Kastrup <dak@gnu.org> writes: >> Actually the only cases I can vaguely remember using the >> (/ (length (match-data)) 2) idiom didn't use the whole (match-data). >> They typically used the idiom in order to know *which* subgroup matched (of >> course it only works if you craft your regexp carefully). > > That would not work. (length (match-data)) is a property of the > regexp, not the match. According to the code, it does work. Match data only contains markers until the last successful match. -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-08 10:00 ` Kim F. Storm @ 2005-06-08 10:11 ` David Kastrup 0 siblings, 0 replies; 35+ messages in thread From: David Kastrup @ 2005-06-08 10:11 UTC (permalink / raw) Cc: dominik, emacs-devel, Stefan Monnier, rms storm@cua.dk (Kim F. Storm) writes: > David Kastrup <dak@gnu.org> writes: > >>> Actually the only cases I can vaguely remember using the >>> (/ (length (match-data)) 2) idiom didn't use the whole (match-data). >>> They typically used the idiom in order to know *which* subgroup matched (of >>> course it only works if you craft your regexp carefully). >> >> That would not work. (length (match-data)) is a property of the >> regexp, not the match. > > According to the code, it does work. > > Match data only contains markers until the last successful match. (progn (string-match "\\(x\\)\\|\\(y\\)" "x") (match-data)) => (0 1 0 1) I stand corrected. Hey, can I rely on that? It would probably speed up some code of mine where I have to scan for the only non-nil match-beginning. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 18:06 ` Stefan Monnier 2005-06-08 8:44 ` Kim F. Storm 2005-06-08 9:47 ` David Kastrup @ 2005-06-08 15:59 ` Richard Stallman 2005-06-08 16:25 ` David Kastrup 2 siblings, 1 reply; 35+ messages in thread From: Richard Stallman @ 2005-06-08 15:59 UTC (permalink / raw) Cc: dominik, emacs-devel, storm Actually the only cases I can vaguely remember using the (/ (length (match-data)) 2) idiom didn't use the whole (match-data). They typically used the idiom in order to know *which* subgroup matched (of course it only works if you craft your regexp carefully). Can't you tell that more easily by seeing if match-beginning returns nil? ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-08 15:59 ` Richard Stallman @ 2005-06-08 16:25 ` David Kastrup 2005-06-09 14:40 ` Richard Stallman 0 siblings, 1 reply; 35+ messages in thread From: David Kastrup @ 2005-06-08 16:25 UTC (permalink / raw) Cc: emacs-devel, Stefan Monnier, storm, dominik Richard Stallman <rms@gnu.org> writes: > Actually the only cases I can vaguely remember using the (/ > (length (match-data)) 2) idiom didn't use the whole > (match-data). They typically used the idiom in order to know > *which* subgroup matched (of course it only works if you craft > your regexp carefully). > > Can't you tell that more easily by seeing if match-beginning returns nil? Which match-beginning? After (string-match "\\(a\\)\\|\\(b\\)\\|\\(c\\)" input) I can just consult (length (match-data)) for distinguishing between all three alternatives. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-08 16:25 ` David Kastrup @ 2005-06-09 14:40 ` Richard Stallman 2005-06-09 15:05 ` David Kastrup 0 siblings, 1 reply; 35+ messages in thread From: Richard Stallman @ 2005-06-09 14:40 UTC (permalink / raw) Cc: emacs-devel, monnier, storm, dominik > Can't you tell that more easily by seeing if match-beginning returns nil? Which match-beginning? One for a subexpression inside the alternative you're trying to test for. After (string-match "\\(a\\)\\|\\(b\\)\\|\\(c\\)" input) I can just consult (length (match-data)) for distinguishing between all three alternatives. You could, but you'd have to compare the value of that against various constants, which would be ugly. I think this code is cleaner: (cond ((match-beginning 1) ...) ((match-beginning 2) ...) ((match-beginning 3) ...) ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-09 14:40 ` Richard Stallman @ 2005-06-09 15:05 ` David Kastrup 2005-06-10 13:30 ` Richard Stallman 0 siblings, 1 reply; 35+ messages in thread From: David Kastrup @ 2005-06-09 15:05 UTC (permalink / raw) Cc: emacs-devel, monnier, storm, dominik Richard Stallman <rms@gnu.org> writes: > > Can't you tell that more easily by seeing if match-beginning returns nil? > > Which match-beginning? > > One for a subexpression inside the alternative you're trying to test for. > > After (string-match "\\(a\\)\\|\\(b\\)\\|\\(c\\)" input) > > I can just consult (length (match-data)) for distinguishing between > all three alternatives. > > You could, but you'd have to compare the value of that against > various constants, which would be ugly. Uh, no. In the applications I am using, the expression for the string match is created programmatically from a large list of strings, and the result is used for indexing into corresponding data structures. > I think this code is cleaner: > > (cond ((match-beginning 1) > ...) > ((match-beginning 2) > ...) > ((match-beginning 3) > ...) Certainly cleaner than the straw man you are trying to put up, no question about that. And I have been using this idiom a number of times for other code. In the application that I had in mind, however, no different code paths were taken, and so this boiled down to (while (not (match-beginning index)) (setq index (1+ index))) (do-something-about index) -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-09 15:05 ` David Kastrup @ 2005-06-10 13:30 ` Richard Stallman 2005-06-10 14:13 ` David Kastrup 0 siblings, 1 reply; 35+ messages in thread From: Richard Stallman @ 2005-06-10 13:30 UTC (permalink / raw) Cc: emacs-devel, monnier, storm, dominik Certainly cleaner than the straw man you are trying to put up, no question about that. It's not a straw man. It's what I'd expect most such cases to do. If it is very important to optimize your code, we could add a function match-count to do so. But is it really that important to optimize? ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-10 13:30 ` Richard Stallman @ 2005-06-10 14:13 ` David Kastrup 0 siblings, 0 replies; 35+ messages in thread From: David Kastrup @ 2005-06-10 14:13 UTC (permalink / raw) Cc: emacs-devel, monnier, storm, dominik Richard Stallman <rms@gnu.org> writes: > Certainly cleaner than the straw man you are trying to put up, no > question about that. > > It's not a straw man. It's what I'd expect most such cases to do. > > If it is very important to optimize your code, we could add a > function match-count to do so. But is it really that important to > optimize? I think that if one has the situation of a number of strings and corresponding actions in a data structure, and one wants to repeatedly search for the next occurence of any matching string, and then do the recorded corresponding action, this would be a typical situation where match-count would be convenient. grepping through the Emacs tree suggests that this usage is not common, however. But I did see several occasions where match-count seemed appropriate as a loop limit for running through possible matched groups after a match. There is also the following in tramp.el: /home/tmp/emacs/lisp/net/tramp.el:6450: (let* ((nmatches (/ (length (match-data)) 2)) /home/tmp/emacs/lisp/net/tramp.el:6793: (setq len (/ (length (match-data t)) 2)) In particular the former usage suffers from the "trailing markers" slowdown. While it is something that should be fixed here, it would seem that the availability of match-count would have prevented this particular abuse. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 8:46 ` Kim F. Storm 2005-06-07 9:23 ` David Kastrup @ 2005-06-07 14:28 ` Richard Stallman 2005-06-07 14:35 ` David Kastrup 2005-06-07 14:42 ` Kim F. Storm 2005-06-07 18:11 ` Stefan Monnier 2 siblings, 2 replies; 35+ messages in thread From: Richard Stallman @ 2005-06-07 14:28 UTC (permalink / raw) Cc: emacs-devel, dominik 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. There is no reason for the changes people are proposing. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 14:28 ` Richard Stallman @ 2005-06-07 14:35 ` David Kastrup 2005-06-08 15:59 ` Richard Stallman 2005-06-07 14:42 ` Kim F. Storm 1 sibling, 1 reply; 35+ messages in thread From: David Kastrup @ 2005-06-07 14:35 UTC (permalink / raw) Cc: dominik, emacs-devel, 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. There is no reason for the changes people are proposing. Problem is that markers slow down editing, and significantly so. And normal editing operations are not associated with extensive consing, so they won't trigger frequent garbage collection. It is not good that save-match-data leaves markers lying around, and there is little evidence that user code using match-data is written with the issues in mind. Providing ways to deal with the issues right with the functions themselves might make it both more obvious and convenient to people to recognize and do the right thing. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 14:35 ` David Kastrup @ 2005-06-08 15:59 ` Richard Stallman 0 siblings, 0 replies; 35+ messages in thread From: Richard Stallman @ 2005-06-08 15:59 UTC (permalink / raw) Cc: dominik, emacs-devel, storm Problem is that markers slow down editing, and significantly so. And normal editing operations are not associated with extensive consing, so they won't trigger frequent garbage collection. That is sufficient reason to urge people to use match-data only when really necessary, but not a sufficient reason to eliminate match-data. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 14:28 ` Richard Stallman 2005-06-07 14:35 ` David Kastrup @ 2005-06-07 14:42 ` Kim F. Storm 1 sibling, 0 replies; 35+ messages in thread From: Kim F. Storm @ 2005-06-07 14:42 UTC (permalink / raw) Cc: emacs-devel, dominik 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 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-07 8:46 ` Kim F. Storm 2005-06-07 9:23 ` David Kastrup 2005-06-07 14:28 ` Richard Stallman @ 2005-06-07 18:11 ` Stefan Monnier 2 siblings, 0 replies; 35+ messages in thread From: Stefan Monnier @ 2005-06-07 18:11 UTC (permalink / raw) Cc: emacs-devel, Carsten Dominik > In any case, to me, the match-data interface should not be considered > a user-level feature _at all_. The user level feature is > save-match-data, which does not (in principle) allow the user to mess > with the saved data. And in that respect, nobody should really care > about what the format of the match data is. Not sure about `match-data', but `set-match-data' is definitely useful for cases like font-lock where the MATCHER function is expected to return its info in the match-data but there may not be any regexp that properly matches what we're looking for, so the MATCHER function needs to do the match "by hand" and then manually construct a match data to pass to set-match-data. See smerge-mode for an example. Stefan ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-06 22:24 ` Kim F. Storm 2005-06-06 23:55 ` David Kastrup @ 2005-06-08 12:01 ` Richard Stallman 2005-06-08 23:08 ` Kim F. Storm 1 sibling, 1 reply; 35+ messages in thread From: Richard Stallman @ 2005-06-08 12:01 UTC (permalink / raw) Cc: emacs-devel, dominik This `evaporate' idea is a good one, but I agree with the person who said that the indication should go at the end, not the beginning, of the return value. The value returned by `match-data' has a defined format and this should not be broken. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-08 12:01 ` Richard Stallman @ 2005-06-08 23:08 ` Kim F. Storm 0 siblings, 0 replies; 35+ messages in thread From: Kim F. Storm @ 2005-06-08 23:08 UTC (permalink / raw) Cc: emacs-devel, dominik Richard Stallman <rms@gnu.org> writes: > This `evaporate' idea is a good one, but I agree with the person > who said that the indication should go at the end, not the beginning, > of the return value. The value returned by `match-data' has > a defined format and this should not be broken. I have installed the second version of the patch, adding the RESEAT arg to match-data and set-match-data. I extended the patch so that specifying `evaporate' for RESEAT will free the markers immediately. I changed save-match-data to use evaporate, but the change to replace.el only passes `t' to match-data -- maybe it could pass evaporate as well. David? -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-06 14:18 ` Kim F. Storm 2005-06-06 22:24 ` Kim F. Storm @ 2005-06-07 12:24 ` Richard Stallman 1 sibling, 0 replies; 35+ messages in thread From: Richard Stallman @ 2005-06-07 12:24 UTC (permalink / raw) Cc: dominik, emacs-devel Which brings me to the suggestion that we add an optional arg to set-match-data like this: (set-match-data list &optional destroy-markers) and change save-match-data to use it That would be an improvement, but the next GC will destroy these markers anyway, so it may not be a big improvement except in code that does this an awful lot. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Inefficient code in reftex-index.el 2005-06-06 12:39 ` David Kastrup 2005-06-06 13:52 ` Stefan Monnier 2005-06-06 14:18 ` Kim F. Storm @ 2005-06-06 14:41 ` Carsten Dominik 2 siblings, 0 replies; 35+ messages in thread From: Carsten Dominik @ 2005-06-06 14:41 UTC (permalink / raw) Cc: Carsten Dominik, emacs-devel, Kim F. Storm >>>>> "DK" == David Kastrup <dak@gnu.org> writes: DK> storm@cua.dk (Kim F. Storm) writes: >> I noticed the following code in reftex-index.el: >> >> (condition-case nil (texmathp) (error nil)))) >> (setq beg (car (match-data)) >> end (nth 1 (match-data))) >> >> Using match-data like that seems inefficient. >> >> I suggtest using match-beginning/match-end instead. DK> That is not the same: the above will set beg and end to markers, DK> whereas match-beginning/match-end happen to be integers. Yes, indeed markers are needed here. I will change it like Stafan proposed it. Thanks for picking this up, Kim. I don't understand your ironic reply to Davids email. Entirely unnecessary. - Carsten -- Carsten Dominik <dominik@science.uva.nl> \ _ / Sterrenkundig Instituut "Anton Pannekoek" |X| _ Kruislaan 403; NL-1098 SJ Amsterdam /| |\ _ _ _/ \ phone +31 (20) 525-7477; FAX +31 (20) 525-7484 __|o|___/ ~~ \___/ ~~~ ^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2005-06-10 14:13 UTC | newest] Thread overview: 35+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2005-06-06 12:08 Inefficient code in reftex-index.el Kim F. Storm 2005-06-06 12:39 ` David Kastrup 2005-06-06 13:52 ` Stefan Monnier 2005-06-06 14:14 ` Kim F. Storm 2005-06-06 14:15 ` Juanma Barranquero 2005-06-06 14:18 ` Kim F. Storm 2005-06-06 22:24 ` Kim F. Storm 2005-06-06 23:55 ` David Kastrup 2005-06-07 8:46 ` Kim F. Storm 2005-06-07 9:23 ` David Kastrup 2005-06-07 10:38 ` Kim F. Storm 2005-06-07 11:05 ` David Kastrup 2005-06-07 11:28 ` Kim F. Storm 2005-06-07 14:28 ` Richard Stallman 2005-06-07 14:46 ` Kim F. Storm 2005-06-07 18:06 ` Stefan Monnier 2005-06-08 8:44 ` Kim F. Storm 2005-06-08 9:47 ` David Kastrup 2005-06-08 10:00 ` Kim F. Storm 2005-06-08 10:11 ` David Kastrup 2005-06-08 15:59 ` Richard Stallman 2005-06-08 16:25 ` David Kastrup 2005-06-09 14:40 ` Richard Stallman 2005-06-09 15:05 ` David Kastrup 2005-06-10 13:30 ` Richard Stallman 2005-06-10 14:13 ` David Kastrup 2005-06-07 14:28 ` Richard Stallman 2005-06-07 14:35 ` David Kastrup 2005-06-08 15:59 ` Richard Stallman 2005-06-07 14:42 ` Kim F. Storm 2005-06-07 18:11 ` Stefan Monnier 2005-06-08 12:01 ` Richard Stallman 2005-06-08 23:08 ` Kim F. Storm 2005-06-07 12:24 ` Richard Stallman 2005-06-06 14:41 ` Carsten Dominik
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.