bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[Michael Heerdegen]

When pop-up-windows is nil, the byte compiler sometimes aborts with
the error message

  End of file during parsing

after displaying warnings, although the source file is ok.


You can reproduce this with CVS Gnu Emacs as follows:

1. Create a file test.el with the following contents:

    (setq a 1)

2. emacs -Q

3. M-: (setq pop-up-windows nil) RET

4. Byte compile test.el


The reason for the bug is an error in `byte-compile-from-buffer'. Its
definition looks like that:

(defun byte-compile-from-buffer (inbuffer &optional filename)
  ...
  (let ...
    (byte-compile-close-variables
     ...
     (displaying-byte-compile-warnings
      ...
      (with-current-buffer inbuffer
        ...
	(while ...
	 ...)
        ...)
      ...))
    ...))

If pop-up-windows is nil, and warnings have to be displayed,
`byte-compile-report-error' is called, and after that,
(current-buffer) will be the Compile Log buffer, and the current
buffer is not restored for further iterations of the while loop.

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[martin rudalics]

 > If pop-up-windows is nil, and warnings have to be displayed,
 > `byte-compile-report-error' is called, and after that,
 > (current-buffer) will be the Compile Log buffer, and the current
 > buffer is not restored for further iterations of the while loop.

I suppose the byte-compiler should either refrain from using
`display-warning' or, if `pop-up-windows' and `pop-up-frames' are nil,
not display a warning until compilation is complete.

martin

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[Richard M. Stallman]

     > If pop-up-windows is nil, and warnings have to be displayed,
     > `byte-compile-report-error' is called, and after that,
     > (current-buffer) will be the Compile Log buffer, and the current
     > buffer is not restored for further iterations of the while loop.

    I suppose the byte-compiler should either refrain from using
    `display-warning'

That would be very drastic and cause a lot of inconvenience
in the normal situation where there is no bug!

		      or, if `pop-up-windows' and `pop-up-frames' are nil,
    not display a warning until compilation is complete.

The solution that occurs to me is that `display-warning' should
bind `pop-up-windows' to t in that case.

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[martin rudalics]

 >     I suppose the byte-compiler should either refrain from using
 >     `display-warning'
 >
 > That would be very drastic and cause a lot of inconvenience
 > in the normal situation where there is no bug!

I obviously meant use something different instead of `display-warning'.

 > 		      or, if `pop-up-windows' and `pop-up-frames' are nil,
 >     not display a warning until compilation is complete.
 >
 > The solution that occurs to me is that `display-warning' should
 > bind `pop-up-windows' to t in that case.

This would (1) ignore a user preference and (2) not work when the window
can't be split for some reason.

martin

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[Richard M. Stallman]

     >     I suppose the byte-compiler should either refrain from using
     >     `display-warning'
     >
     > That would be very drastic and cause a lot of inconvenience
     > in the normal situation where there is no bug!

    I obviously meant use something different instead of `display-warning'.

`display-warning' was made for uses like this.  If what it does now is
not right in some circumstances, we should change it to do something
better.  Whatever is the right thing to do for these messages,
`display-warning' should do it.

     > The solution that occurs to me is that `display-warning' should
     > bind `pop-up-windows' to t in that case.

    This would (1) ignore a user preference

That is a general preference.  There is no need to be rigid about it
in a special case where it leads to clearly undesirable behavior.

					    and (2) not work when the window
    can't be split for some reason.

That is true.  Fortunately such cases are rare.  `display-warning'
could do something special for those cases.  Perhaps just mention in
the echo area that there are warnings.

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[martin rudalics]

 > `display-warning' was made for uses like this.  If what it does now is
 > not right in some circumstances, we should change it to do something
 > better.  Whatever is the right thing to do for these messages,
 > `display-warning' should do it.

We could do something special if `warning-series' is non-nil.  Or maybe
`displaying-byte-compile-warnings' could do that.

martin

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[Stefan Monnier]

> If pop-up-windows is nil, and warnings have to be displayed,
> `byte-compile-report-error' is called, and after that,
> (current-buffer) will be the Compile Log buffer, and the current
> buffer is not restored for further iterations of the while loop.

Does the patch below fix the problem?
Maybe the fix should even be pushed further down into display-buffer?


        Stefan


=== modified file 'lisp/emacs-lisp/warnings.el'
--- lisp/emacs-lisp/warnings.el	2008-05-06 14:18:59 +0000
+++ lisp/emacs-lisp/warnings.el	2008-09-09 14:32:38 +0000
@@ -286,11 +286,15 @@
 	  (or (< (warning-numeric-level level)
 		 (warning-numeric-level warning-minimum-level))
 	      (warning-suppress-p type warning-suppress-types)
+              ;; `display-buffer' usually preserves current-buffer, except
+              ;; when it ends up doing switch-to-buffer (e.g. pop-up-windows
+              ;; is nil).
+              (save-current-buffer
 	      (let ((window (display-buffer buffer)))
 		(when (and (markerp warning-series)
 			   (eq (marker-buffer warning-series) buffer))
 		  (set-window-start window warning-series))
-		(sit-for 0)))))))
+                  (sit-for 0))))))))
 \f
 ;;;###autoload
 (defun lwarn (type level message &rest args)

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[martin rudalics]

 > +              ;; `display-buffer' usually preserves current-buffer, except
 > +              ;; when it ends up doing switch-to-buffer (e.g. pop-up-windows
 > +              ;; is nil).

FWIW it's set_window_buffer in

   /* We must select BUFFER for running the window-scroll-functions.
      If WINDOW is selected, switch permanently.
      Otherwise, switch but go back to the ambient buffer afterward.  */
   if (EQ (window, selected_window))
     Fset_buffer (buffer);

which is not very clean (and not documented in `set-window-buffer').

 > +              (save-current-buffer
 >  	      (let ((window (display-buffer buffer)))
 >  		(when (and (markerp warning-series)
 >  			   (eq (marker-buffer warning-series) buffer))
 >  		  (set-window-start window warning-series))
 > -		(sit-for 0)))))))
 > +                  (sit-for 0))))))))

This will cause the *Warnings* buffer to flash shortly here and
immediately get replaced when compilation finished.

martin

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[Stefan Monnier]

>> +              (save-current-buffer
>> (let ((window (display-buffer buffer)))
>> (when (and (markerp warning-series)
>> (eq (marker-buffer warning-series) buffer))
>> (set-window-start window warning-series))
>> -		(sit-for 0)))))))
>> +                  (sit-for 0))))))))

> This will cause the *Warnings* buffer to flash shortly here and
> immediately get replaced when compilation finished.

Are you sure?  save-current-buffer does not mess with selected windows,
so it should have no such effect.


        Stefan

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[martin rudalics]

 >> This will cause the *Warnings* buffer to flash shortly here and
 >> immediately get replaced when compilation finished.
 >
 > Are you sure?  save-current-buffer does not mess with selected windows,
 > so it should have no such effect.

The *Warnings* buffer is displayed for less than a second and gets
replaced by the buffer I compile.  Did you mean the *Warnings* buffer
would stay in the selected window after compilation finishes?  That
would mean that `display-buffer' messed with the selected window.

martin

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[Stefan Monnier]

>>> This will cause the *Warnings* buffer to flash shortly here and
>>> immediately get replaced when compilation finished.
>> Are you sure?  save-current-buffer does not mess with selected windows,
>> so it should have no such effect.
> The *Warnings* buffer is displayed for less than a second and gets
> replaced by the buffer I compile.

OK.  But it does fix the OP's problem (that the compilation fails with
an error), right?


        Stefan

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[martin rudalics]

> OK.  But it does fix the OP's problem (that the compilation fails with
> an error), right?

Sure ;-)

martin

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[Stefan Monnier]

>> +              ;; `display-buffer' usually preserves current-buffer, except
>> +              ;; when it ends up doing switch-to-buffer (e.g. pop-up-windows
>> +              ;; is nil).

> FWIW it's set_window_buffer in

>   /* We must select BUFFER for running the window-scroll-functions.
>      If WINDOW is selected, switch permanently.
>      Otherwise, switch but go back to the ambient buffer afterward.  */
>   if (EQ (window, selected_window))
>     Fset_buffer (buffer);

> which is not very clean (and not documented in `set-window-buffer').

I've fixed this so set-window-buffer should never change
current-buffer now.  Does it fix the OP's original problem?


        Stefan

bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings

[martin rudalics]

 > I've fixed this so set-window-buffer should never change
 > current-buffer now.  Does it fix the OP's original problem?

Keeping my fingers still crossed for that change, so I can't type much
today ;-)

FWIW it fixes the original problem but we should show *Compile-Log*
instead of the original buffer when compilation finishes.  As I
suggested earlier `displaying-byte-compile-warnings' could do that when
`pop-up-windows' is nil (maybe only if a critical bug has been found).
This could be a wishlist item.

martin

bug#931: marked as done (23.0.60; Bug in bytecomp.el: displaying warnings)

[Emacs bug Tracking System]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]


Your message dated Fri, 12 Sep 2008 15:51:59 -0400
with message-id <jwvljxxgn4j.fsf-monnier+emacsbugreports@gnu.org>
and subject line Re: bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings
has caused the Emacs bug report #931,
regarding 23.0.60; Bug in bytecomp.el: displaying warnings
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact don@donarmstrong.com
immediately.)


-- 
931: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=931
Emacs Bug Tracking System
Contact don@donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 3127 bytes --]

From: Michael Heerdegen <michael_heerdegen@web.de>
To: emacs-pretest-bug@gnu.org
Subject: 23.0.60; Bug in bytecomp.el: displaying warnings
Date: Mon, 8 Sep 2008 21:35:23 +0200
Message-ID: <1220902523.951243.11457@green>

When pop-up-windows is nil, the byte compiler sometimes aborts with
the error message

  End of file during parsing

after displaying warnings, although the source file is ok.


You can reproduce this with CVS Gnu Emacs as follows:

1. Create a file test.el with the following contents:

    (setq a 1)

2. emacs -Q

3. M-: (setq pop-up-windows nil) RET

4. Byte compile test.el


The reason for the bug is an error in `byte-compile-from-buffer'. Its
definition looks like that:

(defun byte-compile-from-buffer (inbuffer &optional filename)
  ...
  (let ...
    (byte-compile-close-variables
     ...
     (displaying-byte-compile-warnings
      ...
      (with-current-buffer inbuffer
        ...
	(while ...
	 ...)
        ...)
      ...))
    ...))

If pop-up-windows is nil, and warnings have to be displayed,
`byte-compile-report-error' is called, and after that,
(current-buffer) will be the Compile Log buffer, and the current
buffer is not restored for further iterations of the while loop.






[-- Attachment #3: Type: message/rfc822, Size: 2289 bytes --]

From: Stefan Monnier <monnier@iro.umontreal.ca>
To: martin rudalics <rudalics@gmx.at>
Cc: 931-close@emacsbugs.donarmstrong.com, michael_heerdegen@web.de
Subject: Re: bug#931: 23.0.60; Bug in bytecomp.el: displaying warnings
Date: Fri, 12 Sep 2008 15:51:59 -0400
Message-ID: <jwvljxxgn4j.fsf-monnier+emacsbugreports@gnu.org>

>> I've fixed this so set-window-buffer should never change
>> current-buffer now.  Does it fix the OP's original problem?
> Keeping my fingers still crossed for that change, so I can't type much
> today ;-)

> FWIW it fixes the original problem

Good, thanks.

> but we should show *Compile-Log* instead of the original buffer when
> compilation finishes.

That's a separate problem.  And I'm not even sure if that's really what
should happen anyway.

> This could be a wishlist item.

Indeed.


        Stefan