Debugger and mode-line

Debugger and mode-line

[Lars Hansen]

For a long time, I have had problems with the Emacs lisp debugger: The
value returned from match-string calls was often wrong. Now I have found
out that match-data was overwritten during evaluation of the mode-line
because I have an abbreviate-file-name call there, and
abbreviate-file-name overwrites match-data.

Is this a debugger bug? Or should evaluation of the mode-line save
match-data?

Re: Debugger and mode-line

[Richard Stallman]

    Is this a debugger bug? Or should evaluation of the mode-line save
    match-data?

It should save the match-data, because it is a generally useful
function and regexp matching is not part of its _purpose_.

I'll install that sometime soon.

Re: Debugger and mode-line

[Lars Hansen]

Richard Stallman wrote:

>    Is this a debugger bug? Or should evaluation of the mode-line save
>    match-data?
>
>It should save the match-data, because it is a generally useful
>function and regexp matching is not part of its _purpose_.
>  
>
I agree that abbreviate-file-name should save match data.
But what I meant was: Maybe Emacs should save match data when
*evaluating the modeline*. Otherwise someone will run into the problem
of malfunctioning debugger because they happen to have something else in
the modeline that overwrite match data.

Re: Debugger and mode-line

[Richard Stallman]

    I agree that abbreviate-file-name should save match data.
    But what I meant was: Maybe Emacs should save match data when
    *evaluating the modeline*.

That is a good idea.  Can someone please implement it and ack?

Re: Debugger and mode-line

[Richard Stallman]

[I sent this message a week ago but did not get a response.
Could we get the discussion moving again?]

    I agree that abbreviate-file-name should save match data.
    But what I meant was: Maybe Emacs should save match data when
    *evaluating the modeline*.

That is a good idea.  Can someone please implement it and ack?

Re: Debugger and mode-line

[Thien-Thi Nguyen]

Richard Stallman <rms@gnu.org> writes:

> That is a good idea.  Can someone please implement it and ack?

does this patch look ok?

thi

_________________________________________________________
*** xdisp.c	1 Jul 2006 23:29:04 -0000	1.1109
--- xdisp.c	8 Jul 2006 09:50:41 -0000
***************
*** 16522,16527 ****
--- 16522,16528 ----
  {
    struct it it;
    struct face *face;
+   Lisp_Object match_data;
    int count = SPECPDL_INDEX ();
  
    init_iterator (&it, w, -1, -1, NULL, face_id);
***************
*** 16542,16548 ****
--- 16543,16551 ----
       kboard-local variables in the mode_line_format will get the right
       values.  */
    push_frame_kboard (it.f);
+   match_data = Fmatch_data (Qnil, Qnil, Qnil);
    display_mode_element (&it, 0, 0, 0, format, Qnil, 0);
+   Fset_match_data (match_data, Qnil);
    pop_frame_kboard ();
  
    unbind_to (count, Qnil);

Re: Debugger and mode-line

[Andreas Schwab]

Thien-Thi Nguyen <ttn@gnu.org> writes:

> ***************
> *** 16542,16548 ****
> --- 16543,16551 ----
>        kboard-local variables in the mode_line_format will get the right
>        values.  */
>     push_frame_kboard (it.f);
> +   match_data = Fmatch_data (Qnil, Qnil, Qnil);
>     display_mode_element (&it, 0, 0, 0, format, Qnil, 0);
> +   Fset_match_data (match_data, Qnil);

You'll need to protect match_data from GC, since display_mode_element can
call eval.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Debugger and mode-line

[Stefan Monnier]

>> +   match_data = Fmatch_data (Qnil, Qnil, Qnil);
>> display_mode_element (&it, 0, 0, 0, format, Qnil, 0);
>> +   Fset_match_data (match_data, Qnil);

> You'll need to protect match_data from GC, since display_mode_element can
> call eval.

And from non-local exits as well, I believe,


        Stefan

Re: Debugger and mode-line

[Richard Stallman]

This change should use record_unwind_protect.

Re: Debugger and mode-line

[Thien-Thi Nguyen]

Richard Stallman <rms@gnu.org> writes:

> This change should use record_unwind_protect.

how about the following?  is it true that using
`record_unwind_save_match_data' means it is not
necessary to use a separate auto var?

thi


___________________________________________________________
*** xdisp.c	1 Jul 2006 23:29:04 -0000	1.1109
--- xdisp.c	16 Jul 2006 10:24:53 -0000
***************
*** 16542,16547 ****
--- 16542,16548 ----
       kboard-local variables in the mode_line_format will get the right
       values.  */
    push_frame_kboard (it.f);
+   record_unwind_save_match_data ();
    display_mode_element (&it, 0, 0, 0, format, Qnil, 0);
    pop_frame_kboard ();

Re: Debugger and mode-line

[Richard Stallman]

    how about the following?  is it true that using
    `record_unwind_save_match_data' means it is not
    necessary to use a separate auto var?

Yes.  Would someone please install this change:

    *** xdisp.c	1 Jul 2006 23:29:04 -0000	1.1109
    --- xdisp.c	16 Jul 2006 10:24:53 -0000
    ***************
    *** 16542,16547 ****
    --- 16542,16548 ----
	   kboard-local variables in the mode_line_format will get the right
	   values.  */
	push_frame_kboard (it.f);
    +   record_unwind_save_match_data ();
	display_mode_element (&it, 0, 0, 0, format, Qnil, 0);
	pop_frame_kboard ();

Re: Debugger and mode-line

[Thien-Thi Nguyen]

Richard Stallman <rms@gnu.org> writes:

>     how about the following?  is it true that using
>     `record_unwind_save_match_data' means it is not
>     necessary to use a separate auto var?
> 
> Yes.  Would someone please install this change:

thanks for the confirmation.  installed.

thi

Re: Debugger and mode-line

[Lars Hansen]

Thien-Thi Nguyen wrote:

>thanks for the confirmation.  installed.
>  
>
Thanks for fixing this!