unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Nonsensical byte compiler warning.
@ 2007-04-01 17:14 David Kastrup
  2007-04-01 18:10 ` Chong Yidong
  0 siblings, 1 reply; 29+ messages in thread
From: David Kastrup @ 2007-04-01 17:14 UTC (permalink / raw)
  To: emacs-devel


Hi,

when compiling the latest source (with "make cvs-update" in the lisp
subdirectory), I get

Compiling /home/tmp/emacs/lisp/progmodes/cc-cmds.el...

In c-end-of-defun:
progmodes/cc-cmds.el:1612:4:Warning: value returned by `char-after' is not
    used
Wrote /home/tmp/emacs/lisp/progmodes/cc-cmds.elc

There is no char-after at the given line, just an (interactive "p")
form.  This does not actually make any sense whatsoever.

I have no idea where this compiler warning is supposed to come from.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-01 17:14 Nonsensical byte compiler warning David Kastrup
@ 2007-04-01 18:10 ` Chong Yidong
  2007-04-01 20:57   ` Alan Mackenzie
  2007-04-02 12:29   ` Richard Stallman
  0 siblings, 2 replies; 29+ messages in thread
From: Chong Yidong @ 2007-04-01 18:10 UTC (permalink / raw)
  To: David Kastrup, Alan Mackenzie; +Cc: emacs-devel

David Kastrup <dak@gnu.org> writes:

> when compiling the latest source (with "make cvs-update" in the lisp
> subdirectory), I get
>
> Compiling /home/tmp/emacs/lisp/progmodes/cc-cmds.el...
>
> In c-end-of-defun:
> progmodes/cc-cmds.el:1612:4:Warning: value returned by `char-after' is not
>     used
> Wrote /home/tmp/emacs/lisp/progmodes/cc-cmds.elc
>
> I have no idea where this compiler warning is supposed to come from.

I don't know why the byte-compiler printed this confusing message, but
it seems to be issuing a real warning.  There is a `when' statement at
cc-cmds.el:1633 that has no effect, because the return value of the
surrounding `if' form is discarded.

This is probably just a bit of cruft, and easily corrected
(eliminating the warning in the process).  Alan, could you verify
this?

*** emacs/lisp/progmodes/cc-cmds.el.~1.55.~	2007-03-30 18:31:07.000000000 -0400
--- emacs/lisp/progmodes/cc-cmds.el	2007-04-01 14:05:04.000000000 -0400
***************
*** 1630,1639 ****
  	      (setq arg (1+ arg)))
  	  (if (< arg 0)
  	      (setq arg (c-backward-to-nth-BOF-{ (- arg) where)))
! 	  (when (and (= arg 0)
! 		     (c-syntactic-skip-backward "^}")
! 		     (eq (char-before) ?\}))
! 	    t))
  
        ;; Move forward to the } of a function
        (if (> arg 0)
--- 1630,1637 ----
  	      (setq arg (1+ arg)))
  	  (if (< arg 0)
  	      (setq arg (c-backward-to-nth-BOF-{ (- arg) where)))
! 	  (if (= arg 0)
! 	      (c-syntactic-skip-backward "^}")))
  
        ;; Move forward to the } of a function
        (if (> arg 0)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-01 18:10 ` Chong Yidong
@ 2007-04-01 20:57   ` Alan Mackenzie
  2007-04-02 12:29   ` Richard Stallman
  1 sibling, 0 replies; 29+ messages in thread
From: Alan Mackenzie @ 2007-04-01 20:57 UTC (permalink / raw)
  To: Chong Yidong; +Cc: emacs-devel

Hi Chong and David!

On Sun, Apr 01, 2007 at 02:10:18PM -0400, Chong Yidong wrote:
> David Kastrup <dak@gnu.org> writes:

> > when compiling the latest source (with "make cvs-update" in the lisp
> > subdirectory), I get

> > Compiling /home/tmp/emacs/lisp/progmodes/cc-cmds.el...

> > In c-end-of-defun:
> > progmodes/cc-cmds.el:1612:4:Warning: value returned by `char-after' is not
> >     used
> > Wrote /home/tmp/emacs/lisp/progmodes/cc-cmds.elc

> > I have no idea where this compiler warning is supposed to come from.

> I don't know why the byte-compiler printed this confusing message, but
> it seems to be issuing a real warning.  There is a `when' statement at
> cc-cmds.el:1633 that has no effect, because the return value of the
> surrounding `if' form is discarded.

Yes.  There's something stupid about the code there - it looks as though
I've half deleted something at some stage.  I'll sort it out early this
week, sometime.  (Other things are of higher priority at the moment.)

> This is probably just a bit of cruft, and easily corrected
> (eliminating the warning in the process).  Alan, could you verify
> this?

No, this is a long standing problem with cc-cmds.el.  I first tried to
track it down (compiled with Emacs 21) in April 2006, but didn't succeed,
beyond identifying the line which I needed to comment out to get rid of
the warning.  The line was "(eq (char-before) ?\})" and was at the time
in `c-end-of-defun' (which has since been radically changed).  That line
is now L1635; commenting it out silences the warning.

The bug never seemed important enough to lose a lot of sleep over, but it
does irritate.  The (more detailed) warning message reported by Emacs 21
is:

    While compiling toplevel forms:
      ** `(char-after (1- (point)))' called for effect

Seemingly, `char-before' is implemented as a macro in the byte-compiler
in Emacs 21.

I strongly believe that cc-cmds.el gets compiled correctly, and that the
spurious warning is a bug in the byte compiler, triggered by something
unusual in cc-cmds.el.

Chong, could you ask the byte-compiler hacker to look at this, please?
(I don't know who that is).


> *** emacs/lisp/progmodes/cc-cmds.el.~1.55.~	2007-03-30 18:31:07.000000000 -0400
> --- emacs/lisp/progmodes/cc-cmds.el	2007-04-01 14:05:04.000000000 -0400
> ***************
> *** 1630,1639 ****
>   	      (setq arg (1+ arg)))
>   	  (if (< arg 0)
>   	      (setq arg (c-backward-to-nth-BOF-{ (- arg) where)))
> ! 	  (when (and (= arg 0)
> ! 		     (c-syntactic-skip-backward "^}")
> ! 		     (eq (char-before) ?\}))
> ! 	    t))

>         ;; Move forward to the } of a function
>         (if (> arg 0)
> --- 1630,1637 ----
>   	      (setq arg (1+ arg)))
>   	  (if (< arg 0)
>   	      (setq arg (c-backward-to-nth-BOF-{ (- arg) where)))
> ! 	  (if (= arg 0)
> ! 	      (c-syntactic-skip-backward "^}")))

>         ;; Move forward to the } of a function
>         (if (> arg 0)

-- 
Alan.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-01 18:10 ` Chong Yidong
  2007-04-01 20:57   ` Alan Mackenzie
@ 2007-04-02 12:29   ` Richard Stallman
  2007-04-04  4:48     ` Markus Triska
  1 sibling, 1 reply; 29+ messages in thread
From: Richard Stallman @ 2007-04-02 12:29 UTC (permalink / raw)
  To: Chong Yidong; +Cc: acm, emacs-devel

The warning is correct, but the location in it is wrong.
That is a compiler bug.  Would someone please debug it?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-02 12:29   ` Richard Stallman
@ 2007-04-04  4:48     ` Markus Triska
  2007-04-04  6:15       ` David Kastrup
  0 siblings, 1 reply; 29+ messages in thread
From: Markus Triska @ 2007-04-04  4:48 UTC (permalink / raw)
  To: rms; +Cc: acm, Chong Yidong, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> The warning is correct, but the location in it is wrong.
> That is a compiler bug.  Would someone please debug it?

The location is also accurate: It's up to the caller to use return
values of functions known to be side-effect-free.  c-end-of-defun
doesn't (meaningfully), and that's where the message points to.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04  4:48     ` Markus Triska
@ 2007-04-04  6:15       ` David Kastrup
  2007-04-04  8:19         ` Markus Triska
  0 siblings, 1 reply; 29+ messages in thread
From: David Kastrup @ 2007-04-04  6:15 UTC (permalink / raw)
  To: Markus Triska; +Cc: acm, Chong Yidong, rms, emacs-devel

Markus Triska <markus.triska@gmx.at> writes:

> Richard Stallman <rms@gnu.org> writes:
>
>> The warning is correct, but the location in it is wrong.
>> That is a compiler bug.  Would someone please debug it?
>
> The location is also accurate: It's up to the caller to use return
> values of functions known to be side-effect-free.  c-end-of-defun
> doesn't (meaningfully), and that's where the message points to.

It points to c-end-of-defun, but the line number and described called
function are nonsensical.

-- 
David Kastrup

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04  6:15       ` David Kastrup
@ 2007-04-04  8:19         ` Markus Triska
  2007-04-04  8:46           ` David Kastrup
  2007-04-04 20:08           ` Alan Mackenzie
  0 siblings, 2 replies; 29+ messages in thread
From: Markus Triska @ 2007-04-04  8:19 UTC (permalink / raw)
  To: David Kastrup; +Cc: acm, Chong Yidong, rms, emacs-devel

David Kastrup <dak@gnu.org> writes:

> It points to c-end-of-defun, but the line number and described
> called function are nonsensical.

The line number is that of the first form of the function the
questionable code is in. That makes sense, since the problem is in
that function. It is *not* the call of char-before that's bogus. It's
that its return value isn't used in the caller, c-end-of-defun. Any
line of that function could contain the oversight. What line number
would in your view make more sense to report?

And yes, improving the optimiser to report `char-before' instead of
`char-after' would be nice. I doubt that it would help anyone who
can't find the problem with the current (quite good) message though.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04  8:19         ` Markus Triska
@ 2007-04-04  8:46           ` David Kastrup
  2007-04-04  9:50             ` Markus Triska
  2007-04-05  6:52             ` Richard Stallman
  2007-04-04 20:08           ` Alan Mackenzie
  1 sibling, 2 replies; 29+ messages in thread
From: David Kastrup @ 2007-04-04  8:46 UTC (permalink / raw)
  To: Markus Triska; +Cc: acm, Chong Yidong, rms, emacs-devel

Markus Triska <markus.triska@gmx.at> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> It points to c-end-of-defun, but the line number and described
>> called function are nonsensical.
>
> The line number is that of the first form of the function the
> questionable code is in. That makes sense, since the problem is in
> that function. It is *not* the call of char-before that's bogus. It's
> that its return value isn't used in the caller, c-end-of-defun. Any
> line of that function could contain the oversight. What line number
> would in your view make more sense to report?

The line number of the call to char-before, of course.  The line
number of the whole enclosing function is plain useless.  If the
function contain dozens of calls of char-before, the message would not
help at all in figuring out which one was involved.

> And yes, improving the optimiser to report `char-before' instead of
> `char-after' would be nice. I doubt that it would help anyone who
> can't find the problem with the current (quite good) message though.

So apparently I am not anyone.  I reported this warning without being
able to figure out where it came from, remember?

And I am not exactly such an idiot when it comes to programming that I
can be dismissed as irrelevant.

-- 
David Kastrup

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04  8:46           ` David Kastrup
@ 2007-04-04  9:50             ` Markus Triska
  2007-04-04 10:17               ` David Kastrup
  2007-04-05  6:52             ` Richard Stallman
  1 sibling, 1 reply; 29+ messages in thread
From: Markus Triska @ 2007-04-04  9:50 UTC (permalink / raw)
  To: David Kastrup; +Cc: acm, Chong Yidong, rms, emacs-devel

David Kastrup <dak@gnu.org> writes:

> I reported this warning without being able to figure out where it
> came from, remember?

Yes, I remember. I also remember that at least 3 other people figured
it out. One of them submitted a patch. Another one couldn't see the
problem despite having pinpointed the exact location of the call (as
you suggest that the optimiser report), believing in a "bug in the
byte compiler". While the error text can be improved, I find it
unjustified to call the current behaviour "nonsensical", "plain
useless" or "a compiler bug". It's a reasonable choice to point to the
enclosing defun, and if your function has dozens of calls to
char-before, there are probably graver problems to worry about.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04  9:50             ` Markus Triska
@ 2007-04-04 10:17               ` David Kastrup
  2007-04-04 12:35                 ` Markus Triska
  2007-04-04 18:25                 ` Markus Triska
  0 siblings, 2 replies; 29+ messages in thread
From: David Kastrup @ 2007-04-04 10:17 UTC (permalink / raw)
  To: Markus Triska; +Cc: acm, Chong Yidong, rms, emacs-devel

Markus Triska <markus.triska@gmx.at> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> I reported this warning without being able to figure out where it
>> came from, remember?
>
> Yes, I remember. I also remember that at least 3 other people figured
> it out.

So I am not anyone, after all.

> One of them submitted a patch. Another one couldn't see the problem
> despite having pinpointed the exact location of the call (as you
> suggest that the optimiser report), believing in a "bug in the byte
> compiler".

Probably not anyone, either.

> While the error text can be improved, I find it unjustified to call
> the current behaviour "nonsensical", "plain useless" or "a compiler
> bug". It's a reasonable choice to point to the enclosing defun,

Not regarding the line number.  I disagree strongly.  The name of the
enclosing function is useful.  The line number isn't, except for
verifying that one is not looking at the wrong file.

But not even for that the reported number is really helpful since it
does not return the line number of the start or end of the defun
(which would give the user the clue that the line number is not
related to the point of error, but rather the function definition),
but rather a line in the function body that is somewhat close to the
actual beginning of the defun.

And that is worse than useless since it suggests that the line number
tries pinpointing some location inside of the function.  Which it
doesn't.

> and if your function has dozens of calls to char-before, there are
> probably graver problems to worry about.

So you are of the opinion that a function that calls any other
function from more than one place is a grave problem, and the byte
compiler is not supposed to be helpful with grave problems?

Sorry, but I can't see how one could consider your points and
conclusions here even remotely tenable.

-- 
David Kastrup

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04 10:17               ` David Kastrup
@ 2007-04-04 12:35                 ` Markus Triska
  2007-04-04 18:25                 ` Markus Triska
  1 sibling, 0 replies; 29+ messages in thread
From: Markus Triska @ 2007-04-04 12:35 UTC (permalink / raw)
  To: David Kastrup; +Cc: acm, Chong Yidong, rms, emacs-devel

David Kastrup <dak@gnu.org> writes:

> But not even for that the reported number is really helpful since it
> does not return the line number of the start or end of the defun
> (which would give the user the clue that the line number is not
> related to the point of error, but rather the function definition),
> but rather a line in the function body that is somewhat close to the
> actual beginning of the defun.

That depends on whether an "interactive" declaration is present. See
byte-compile-lambda in bytecomp.el.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04 10:17               ` David Kastrup
  2007-04-04 12:35                 ` Markus Triska
@ 2007-04-04 18:25                 ` Markus Triska
  2007-04-04 22:13                   ` David Kastrup
  1 sibling, 1 reply; 29+ messages in thread
From: Markus Triska @ 2007-04-04 18:25 UTC (permalink / raw)
  To: David Kastrup; +Cc: acm, Chong Yidong, rms, emacs-devel

David Kastrup <dak@gnu.org> writes:

> So you are of the opinion that a function that calls any other
> function from more than one place is a grave problem, and the byte
> compiler is not supposed to be helpful with grave problems?

No, I think it's already good enough in this case. For example, set
`byte-optimize-log' to t. The warning is then preceded by:

  (char-before)	==>	(char-after (1- (point)))
  (if (and (= arg 0) (c-syntactic-skip-backward "^}") (eq (char-after
  ...) 125)) nil) ==> (progn (and (= arg 0) (c-syntactic-skip-backward
  "^}") (eq (char-after ...) 125)) nil)
  eq called for effect; deleted

I hope you find that remotely helpful.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04  8:19         ` Markus Triska
  2007-04-04  8:46           ` David Kastrup
@ 2007-04-04 20:08           ` Alan Mackenzie
  2007-04-04 21:45             ` Markus Triska
  2007-04-08  1:21             ` Kim F. Storm
  1 sibling, 2 replies; 29+ messages in thread
From: Alan Mackenzie @ 2007-04-04 20:08 UTC (permalink / raw)
  To: Markus Triska; +Cc: Chong Yidong, rms, emacs-devel

Hi, Markus!

On Wed, Apr 04, 2007 at 10:19:45AM +0200, Markus Triska wrote:
> David Kastrup <dak@gnu.org> writes:

> > It points to c-end-of-defun, but the line number and described
> > called function are nonsensical.

> The line number is that of the first form of the function the
> questionable code is in. That makes sense, since the problem is in
> that function. It is *not* the call of char-before that's bogus. It's
> that its return value isn't used in the caller, c-end-of-defun. Any
> line of that function could contain the oversight. What line number
> would in your view make more sense to report?

As the current maintainer of cc-cmds.el, I haven't found this message at
all helpful.  Here is the actual entry from my log of 2006-04-15, when I
first tried to crack the problem:

    But I did get "While compiling c-end-of-defun in file
    /home/acm/cc-mode-5.31.n/cc-cmds.el: ** `(char-after (1- (point)))' called for
    effect".  Track this down: It's in c-end-of-defun.  By commenting out bits of
    the function in a binary chop fashion, it's L1625, "(eq (char-before) ?\})".
    I can't make head or tail of this.  FIXME!!! POSTPONED.

It didn't occur to me at the time that char-after and char-before were
the same function.

> And yes, improving the optimiser to report `char-before' instead of
> `char-after' would be nice. I doubt that it would help anyone who
> can't find the problem with the current (quite good) message though.

As the "victim" of the situation, I would have found "char-before"
_exceptionally_ helpful.  It might have urged me to look at the code more
carefully, rather than dismissing the message as a byte-compiler bug.

As I say, I don't find the current message helpful:

    In c-end-of-defun:
    cc-cmds.el:1612:4:Warning: value returned by `char-after' is not used

Taking the message bit by bit:
(i) "c-end-of-defun" _is_ helpful;
(ii) "1612:4" is positively unhelpful - at that location is "(interactive
  "p")".  Giving "1612:4" actively hinders debugging.
(iii) "char-after" doesn't exist in the source of c-end-of-defun;
(iv) The value returned by "char-before" _is_ used; it is explicitly
  compared with "?\}".

I think this warning message is buggy.  What it says ("value ... is not
used") is manifestly false - that value _is_ used; what isn't used is a
different value which is derived from that value.

What would a better message look like?  I would suggest this:

    In c-end-of-defun:
    cc-cmds.el:1626:4:Warning: value returned from form is not used.

(where 1626 starts the `if' form, the most nested form for which the
warning holds).  Or perhaps even better:

    In c-end-of-defun:
    cc-cmds.el:1647:57:Warning: value returned from form is not used.

Would it be easy to make this change to the byte compiler?

-- 
Alan Mackenzie (Ittersbach, Germany).

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04 20:08           ` Alan Mackenzie
@ 2007-04-04 21:45             ` Markus Triska
  2007-04-04 22:11               ` Chong Yidong
  2007-04-08  1:21             ` Kim F. Storm
  1 sibling, 1 reply; 29+ messages in thread
From: Markus Triska @ 2007-04-04 21:45 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Chong Yidong, rms, emacs-devel


Alan Mackenzie <acm@muc.de> writes:

> Would it be easy to make this change to the byte compiler?

Alas, no. The optimiser hasn't got the positioning information, and
the byte-code emitter potentially doesn't see such calls at all:
Conceptually, the optimiser is capable of totally removing unneeded
function calls (the code is there, and disabled). Conversely, the
positioning code is fragile enough already to warrant keeping it well
separated from the optimiser, and limited to the emitter. Now, since
unnecessary function calls are currently kept, detection logic from
the optimiser could be used in the emitter, but that would duplicate a
lot of code, is error-prone, and can, in my view, not yield more
useful results than byte-compiling with byte-optimize-log set to t. On
top of that, it would result in the same char-before/after confusion,
since that rewriting is performed by the optimiser, not the emitter.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04 21:45             ` Markus Triska
@ 2007-04-04 22:11               ` Chong Yidong
  2007-04-05  5:44                 ` Markus Triska
  0 siblings, 1 reply; 29+ messages in thread
From: Chong Yidong @ 2007-04-04 22:11 UTC (permalink / raw)
  To: Markus Triska; +Cc: Alan Mackenzie, rms, emacs-devel

Markus Triska <markus.triska@gmx.at> writes:

> Alan Mackenzie <acm@muc.de> writes:
>
>> Would it be easy to make this change to the byte compiler?
>
> Alas, no. The optimiser hasn't got the positioning information, and
> the byte-code emitter potentially doesn't see such calls at all:
> Conceptually, the optimiser is capable of totally removing unneeded
> function calls (the code is there, and disabled). Conversely, the
> positioning code is fragile enough already to warrant keeping it well
> separated from the optimiser, and limited to the emitter. Now, since
> unnecessary function calls are currently kept, detection logic from
> the optimiser could be used in the emitter, but that would duplicate a
> lot of code, is error-prone, and can, in my view, not yield more
> useful results than byte-compiling with byte-optimize-log set to t. On
> top of that, it would result in the same char-before/after confusion,
> since that rewriting is performed by the optimiser, not the emitter.

If the offending function name is unreliable, maybe we should omit it
and issue a "value returned from form is not used" warning.  What do
you think?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04 18:25                 ` Markus Triska
@ 2007-04-04 22:13                   ` David Kastrup
  0 siblings, 0 replies; 29+ messages in thread
From: David Kastrup @ 2007-04-04 22:13 UTC (permalink / raw)
  To: Markus Triska; +Cc: acm, Chong Yidong, rms, emacs-devel

Markus Triska <markus.triska@gmx.at> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> So you are of the opinion that a function that calls any other
>> function from more than one place is a grave problem, and the byte
>> compiler is not supposed to be helpful with grave problems?
>
> No, I think it's already good enough in this case. For example, set
> `byte-optimize-log' to t.

It is good enough, since by setting some variables nobody would know
about one can get a complete log of everything and thus be able to
figure out that the warning points to the wrong place and cause?

> The warning is then preceded by:
>
>   (char-before)	==>	(char-after (1- (point)))
>   (if (and (= arg 0) (c-syntactic-skip-backward "^}") (eq (char-after
>   ...) 125)) nil) ==> (progn (and (= arg 0) (c-syntactic-skip-backward
>   "^}") (eq (char-after ...) 125)) nil)
>   eq called for effect; deleted
>
> I hope you find that remotely helpful.

So far, I have not been able to find your contributions to this thread
anything apart from bizarre.

I suppose we are wired differently.  I can't see your arguments doing
anything to support your point of view, quite contrary.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04 22:11               ` Chong Yidong
@ 2007-04-05  5:44                 ` Markus Triska
  0 siblings, 0 replies; 29+ messages in thread
From: Markus Triska @ 2007-04-05  5:44 UTC (permalink / raw)
  To: Chong Yidong; +Cc: Alan Mackenzie, rms, emacs-devel

Chong Yidong <cyd@stupidchicken.com> writes:

> If the offending function name is unreliable, maybe we should omit
> it and issue a "value returned from form is not used" warning.  What
> do you think?

The function name and its starting position/interactive declaration
are quite reliable, and I find reporting them OK. If by "unreliable"
you mean that any form, even before the defun, could contain the
oversight/mistake of not using the return value, you are right. In my
experience, the innermost surrounding defun is an OK indicator though.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04  8:46           ` David Kastrup
  2007-04-04  9:50             ` Markus Triska
@ 2007-04-05  6:52             ` Richard Stallman
  2007-04-05  7:55               ` Markus Triska
  2007-04-05 18:01               ` Chong Yidong
  1 sibling, 2 replies; 29+ messages in thread
From: Richard Stallman @ 2007-04-05  6:52 UTC (permalink / raw)
  To: David Kastrup; +Cc: acm, cyd, markus.triska, emacs-devel

    > The line number is that of the first form of the function the
    > questionable code is in. That makes sense, since the problem is in
    > that function. It is *not* the call of char-before that's bogus. It's
    > that its return value isn't used in the caller, c-end-of-defun. Any
    > line of that function could contain the oversight. What line number
    > would in your view make more sense to report?

    The line number of the call to char-before, of course.  The line
    number of the whole enclosing function is plain useless.

I agree.

I don't know how hard it will be to make this useful line number
appear, but someone should investigate and _try_ to fix it.
Let's have no more of the argument that this is not a bug!

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-05  6:52             ` Richard Stallman
@ 2007-04-05  7:55               ` Markus Triska
  2007-04-06 12:56                 ` Richard Stallman
  2007-04-05 18:01               ` Chong Yidong
  1 sibling, 1 reply; 29+ messages in thread
From: Markus Triska @ 2007-04-05  7:55 UTC (permalink / raw)
  To: rms; +Cc: acm, cyd, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> I don't know how hard it will be to make this useful line number
> appear, but someone should investigate and _try_ to fix it.

Warnings stemming from the optimiser are commonly restricted to
defun-level positional information. For example, byte compiling:

   (defun f ()
     (message "hi")
     (quote 0 1)
     (let ((x 0 1))))

gives two warnings (both to the defun). Should these be fixed too?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-05  6:52             ` Richard Stallman
  2007-04-05  7:55               ` Markus Triska
@ 2007-04-05 18:01               ` Chong Yidong
  1 sibling, 0 replies; 29+ messages in thread
From: Chong Yidong @ 2007-04-05 18:01 UTC (permalink / raw)
  To: rms; +Cc: acm, markus.triska, emacs-devel

Richard Stallman <rms@gnu.org> writes:

>     > The line number is that of the first form of the function the
>     > questionable code is in. That makes sense, since the problem is in
>     > that function. It is *not* the call of char-before that's bogus. It's
>     > that its return value isn't used in the caller, c-end-of-defun. Any
>     > line of that function could contain the oversight. What line number
>     > would in your view make more sense to report?
>
>     The line number of the call to char-before, of course.  The line
>     number of the whole enclosing function is plain useless.
>
> I agree.
>
> I don't know how hard it will be to make this useful line number
> appear, but someone should investigate and _try_ to fix it.
> Let's have no more of the argument that this is not a bug!

After looking at this, I don't think it is practical to fix the line
number.  However, I changed the warning to print the entire form,
which should make it much easier to figure out where the problematic
code is (in this case, even if the form has been changed by previous
optimizations, it shouldn't be too difficult to figure out what the
original form was).

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-05  7:55               ` Markus Triska
@ 2007-04-06 12:56                 ` Richard Stallman
  2007-04-06 15:11                   ` Chong Yidong
  2007-04-08 20:47                   ` Markus Triska
  0 siblings, 2 replies; 29+ messages in thread
From: Richard Stallman @ 2007-04-06 12:56 UTC (permalink / raw)
  To: Markus Triska; +Cc: acm, cyd, emacs-devel

    Warnings stemming from the optimiser are commonly restricted to
    defun-level positional information. For example, byte compiling:

       (defun f ()
	 (message "hi")
	 (quote 0 1)
	 (let ((x 0 1))))

    gives two warnings (both to the defun). Should these be fixed too?

I would like someone to try.  And if it can't be done, the second best
thing is to say explicitly in the warning that the line number
pertains only to the function, not the actual place.

Another solution might be to generate a dummy expression
to hold the warning.  The warning would actually be issued
when the compiler tries to really compile that expression.

I agree this is too hard to change now.
However, it would have been premature to give up without
checking.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-06 12:56                 ` Richard Stallman
@ 2007-04-06 15:11                   ` Chong Yidong
  2007-04-08 20:47                   ` Markus Triska
  1 sibling, 0 replies; 29+ messages in thread
From: Chong Yidong @ 2007-04-06 15:11 UTC (permalink / raw)
  To: rms; +Cc: acm, Markus Triska, emacs-devel

Richard Stallman <rms@gnu.org> writes:

>     Warnings stemming from the optimiser are commonly restricted to
>     defun-level positional information. For example, byte compiling:
>
>        (defun f ()
> 	 (message "hi")
> 	 (quote 0 1)
> 	 (let ((x 0 1))))
>
>     gives two warnings (both to the defun). Should these be fixed too?
>
> I agree this is too hard to change now.
> However, it would have been premature to give up without
> checking.

I've added a TODO item for this:

  ** Make byte-optimization warnings issue accurate line numbers.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-04 20:08           ` Alan Mackenzie
  2007-04-04 21:45             ` Markus Triska
@ 2007-04-08  1:21             ` Kim F. Storm
  2007-04-08 11:27               ` Alan Mackenzie
  1 sibling, 1 reply; 29+ messages in thread
From: Kim F. Storm @ 2007-04-08  1:21 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Chong Yidong, rms, Markus Triska, emacs-devel

Alan Mackenzie <acm@muc.de> writes:

>     But I did get "While compiling c-end-of-defun in file
>     /home/acm/cc-mode-5.31.n/cc-cmds.el: ** `(char-after (1- (point)))' called for
>     effect".  Track this down: It's in c-end-of-defun.  By commenting out bits of
>     the function in a binary chop fashion, it's L1625, "(eq (char-before) ?\})".
>     I can't make head or tail of this.  FIXME!!! POSTPONED.

The problem is still not fixed ...  The warning is still issued, and
the following post shows why:

Markus Triska <markus.triska@gmx.at> writes:

> No, I think it's already good enough in this case. For example, set
> `byte-optimize-log' to t. The warning is then preceded by:
>
>   (char-before)	==>	(char-after (1- (point)))
>   (if (and (= arg 0) (c-syntactic-skip-backward "^}") (eq (char-after
>   ...) 125)) nil) ==> (progn (and (= arg 0) (c-syntactic-skip-backward
>   "^}") (eq (char-after ...) 125)) nil)
>   eq called for effect; deleted

Looking at the offending code, it is rather obvious why it says eq is
called for effect.

The relevant part of the code looks like this:

    (if (< arg 0)
	;; Move backwards to the } of a function
	(progn
	  (if (memq where '(at-header outwith-function))
	      (setq arg (1+ arg)))
	  (if (< arg 0)
	      (setq arg (c-backward-to-nth-BOF-{ (- arg) where)))
	  (when (and (= arg 0)
		     (c-syntactic-skip-backward "^}")
		     (eq (char-before) ?\}))
	    t))

      ;; Move forward to the } of a function
      (if (> arg 0)
	  (setq arg (c-forward-to-nth-EOF-} arg where))))


Since the value of the surrounding "if" is not used, the value of the
(when ... t) is not used either, and as such it has no purpose to
evaluate the (eq (char-before) ...) part.

So as Chong already suggested, replacing

	  (when (and (= arg 0)
		     (c-syntactic-skip-backward "^}")
		     (eq (char-before) ?\}))
	    t))

with

	  (if (= arg 0)
              (c-syntactic-skip-backward "^}")))

does the same thing.


Here's the patch:

*** cc-cmds.el	08 Apr 2007 01:47:26 +0200	1.56
--- cc-cmds.el	08 Apr 2007 03:16:14 +0200	
***************
*** 1630,1639 ****
  	      (setq arg (1+ arg)))
  	  (if (< arg 0)
  	      (setq arg (c-backward-to-nth-BOF-{ (- arg) where)))
! 	  (when (and (= arg 0)
! 		     (c-syntactic-skip-backward "^}")
! 		     (eq (char-before) ?\}))
! 	    t))
  
        ;; Move forward to the } of a function
        (if (> arg 0)
--- 1630,1637 ----
  	      (setq arg (1+ arg)))
  	  (if (< arg 0)
  	      (setq arg (c-backward-to-nth-BOF-{ (- arg) where)))
! 	  (if (= arg 0)
! 	      (c-syntactic-skip-backward "^}")))
  
        ;; Move forward to the } of a function
        (if (> arg 0)


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-08  1:21             ` Kim F. Storm
@ 2007-04-08 11:27               ` Alan Mackenzie
  0 siblings, 0 replies; 29+ messages in thread
From: Alan Mackenzie @ 2007-04-08 11:27 UTC (permalink / raw)
  To: Kim F. Storm; +Cc: Chong Yidong, rms, Markus Triska, emacs-devel

Hi, all!

On Sun, Apr 08, 2007 at 03:21:07AM +0200, Kim F. Storm wrote:
> Alan Mackenzie <acm@muc.de> writes:

> >     But I did get "While compiling c-end-of-defun in file
> >     /home/acm/cc-mode-5.31.n/cc-cmds.el: ** `(char-after (1-
> >     (point)))' called for effect".  Track this down: It's in
> >     c-end-of-defun.  By commenting out bits of the function in a
> >     binary chop fashion, it's L1625, "(eq (char-before) ?\})".  I
> >     can't make head or tail of this.  FIXME!!! POSTPONED.

> The problem is still not fixed ...  The warning is still issued, and
> the following post shows why:

I've fixed it now.  Thanks for the patch, Chong!

Just for clarity, I wasn't being obtuse - just slow.  (I've been
concentrating on other bugs in the last few days).   c-end-of-defun's
code was clearly wrong, and in the paragraph you quoted, I was trying to
show how the warning message left me confused, not to defend the code as
it was.

It's a relief to have that warning finally cleared up.

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

-- 
Alan.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-06 12:56                 ` Richard Stallman
  2007-04-06 15:11                   ` Chong Yidong
@ 2007-04-08 20:47                   ` Markus Triska
  2007-04-09 15:42                     ` Richard Stallman
  1 sibling, 1 reply; 29+ messages in thread
From: Markus Triska @ 2007-04-08 20:47 UTC (permalink / raw)
  To: rms; +Cc: acm, cyd, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> I agree this is too hard to change now.
> However, it would have been premature to give up without
> checking.

Sure; here's a rather trivial compromise for `char-before': It makes
the compiler rewrite calls to char-before at the time the byte-code is
emitted instead of during the optimisation phase. This retains the
correct function call in the warning. It can yield slightly inferior
byte-code though (with usage-patterns that don't occur in Emacs trunk
and are probably rather atypical). If you find this change advisable,
I'll implement analogous changes for backward-char and backward-word.

2007-04-08  Markus Triska  <markus.triska@gmx.at>

	* emacs-lisp/byte-opt.el: remove char-before rewriting
	* emacs-lisp/bytecomp.el: add char-before rewriting


*** byte-opt.el	04 Apr 2007 19:36:54 +0200	1.88
--- byte-opt.el	08 Apr 2007 21:33:44 +0200	
***************
*** 1158,1171 ****
  	 '(forward-word -1))
  	(t form)))
  
- (put 'char-before 'byte-optimizer 'byte-optimize-char-before)
- (defun byte-optimize-char-before (form)
-   (cond ((= 2 (safe-length form))
- 	 `(char-after (1- ,(nth 1 form))))
- 	((= 1 (safe-length form))
- 	 '(char-after (1- (point))))
- 	(t form)))
- 
  ;; Fixme: delete-char -> delete-region (byte-coded)
  ;; optimize string-as-unibyte, string-as-multibyte, string-make-unibyte,
  ;; string-make-multibyte for constant args.
--- 1158,1163 ----
*** bytecomp.el	27 Mar 2007 08:08:58 +0200	2.196
--- bytecomp.el	08 Apr 2007 21:52:58 +0200	
***************
*** 3148,3153 ****
--- 3148,3154 ----
  \f
  ;; more complicated compiler macros
  
+ (byte-defop-compiler char-before)
  (byte-defop-compiler list)
  (byte-defop-compiler concat)
  (byte-defop-compiler fset)
***************
*** 3159,3164 ****
--- 3160,3172 ----
  (byte-defop-compiler19 (/ byte-quo) byte-compile-quo)
  (byte-defop-compiler19 nconc)
  
+ (defun byte-compile-char-before (form)
+   (cond ((= 2 (length form))
+ 	 (byte-compile-form `(char-after (1- ,(nth 1 form)))))
+ 	((= 1 (length form))
+ 	 (byte-compile-form '(char-after (1- (point)))))
+ 	(t (byte-compile-subr-wrong-args form "0-1"))))
+ 
  (defun byte-compile-list (form)
    (let ((count (length (cdr form))))
      (cond ((= count 0)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-08 20:47                   ` Markus Triska
@ 2007-04-09 15:42                     ` Richard Stallman
  2007-04-10  3:53                       ` Glenn Morris
  0 siblings, 1 reply; 29+ messages in thread
From: Richard Stallman @ 2007-04-09 15:42 UTC (permalink / raw)
  To: Markus Triska; +Cc: acm, cyd, emacs-devel

    Sure; here's a rather trivial compromise for `char-before': It makes
    the compiler rewrite calls to char-before at the time the byte-code is
    emitted instead of during the optimisation phase. This retains the
    correct function call in the warning. It can yield slightly inferior
    byte-code though (with usage-patterns that don't occur in Emacs trunk
    and are probably rather atypical).

This is a good improvement.  Would someone please install it?

				       If you find this change advisable,
    I'll implement analogous changes for backward-char and backward-word.

Please do, and thanks.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-09 15:42                     ` Richard Stallman
@ 2007-04-10  3:53                       ` Glenn Morris
  2007-04-10 17:27                         ` Markus Triska
  0 siblings, 1 reply; 29+ messages in thread
From: Glenn Morris @ 2007-04-10  3:53 UTC (permalink / raw)
  To: rms; +Cc: acm, cyd, Markus Triska, emacs-devel

Richard Stallman wrote:

> This is a good improvement.  Would someone please install it?

installed

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-10  3:53                       ` Glenn Morris
@ 2007-04-10 17:27                         ` Markus Triska
  2007-04-11  4:00                           ` Glenn Morris
  0 siblings, 1 reply; 29+ messages in thread
From: Markus Triska @ 2007-04-10 17:27 UTC (permalink / raw)
  To: Glenn Morris; +Cc: acm, cyd, rms, emacs-devel

Glenn Morris <rgm@gnu.org> writes:

> installed

Thank you. Here's the remaining part:

2007-04-10  Markus Triska  <markus.triska@gmx.at>

	* emacs-lisp/byte-opt.el (byte-optimize-backward-char)
	(byte-optimize-backward-word): Remove (move to bytecomp.el)

	* emacs-lisp/bytecomp.el (byte-compile-char-before): Improve
	numeric argument case
	(byte-compile-backward-char, byte-compile-backward-word): New
	functions, performing rewriting previously done in byte-opt.el.
	Fix their "Fixme" item (restriction to numeric arguments).

Index: byte-opt.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/emacs-lisp/byte-opt.el,v
retrieving revision 1.92
diff -c -r1.92 byte-opt.el
*** byte-opt.el	10 Apr 2007 03:55:17 -0000	1.92
--- byte-opt.el	10 Apr 2007 09:04:21 -0000
***************
*** 1117,1142 ****
  	(byte-optimize-predicate form))
      form))
  
- ;; Avoid having to write forward-... with a negative arg for speed.
- ;; Fixme: don't be limited to constant args.
- (put 'backward-char 'byte-optimizer 'byte-optimize-backward-char)
- (defun byte-optimize-backward-char (form)
-   (cond ((and (= 2 (safe-length form))
- 	      (numberp (nth 1 form)))
- 	 (list 'forward-char (eval (- (nth 1 form)))))
- 	((= 1 (safe-length form))
- 	 '(forward-char -1))
- 	(t form)))
- 
- (put 'backward-word 'byte-optimizer 'byte-optimize-backward-word)
- (defun byte-optimize-backward-word (form)
-   (cond ((and (= 2 (safe-length form))
- 	      (numberp (nth 1 form)))
- 	 (list 'forward-word (eval (- (nth 1 form)))))
- 	((= 1 (safe-length form))
- 	 '(forward-word -1))
- 	(t form)))
- 
  ;; Fixme: delete-char -> delete-region (byte-coded)
  ;; optimize string-as-unibyte, string-as-multibyte, string-make-unibyte,
  ;; string-make-multibyte for constant args.
--- 1117,1122 ----


Index: bytecomp.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/emacs-lisp/bytecomp.el,v
retrieving revision 2.197
diff -c -r2.197 bytecomp.el
*** bytecomp.el	10 Apr 2007 03:54:36 -0000	2.197
--- bytecomp.el	10 Apr 2007 09:07:07 -0000
***************
*** 3149,3154 ****
--- 3149,3156 ----
  ;; more complicated compiler macros
  
  (byte-defop-compiler char-before)
+ (byte-defop-compiler backward-char)
+ (byte-defop-compiler backward-word)
  (byte-defop-compiler list)
  (byte-defop-compiler concat)
  (byte-defop-compiler fset)
***************
*** 3162,3171 ****
  
  (defun byte-compile-char-before (form)
    (cond ((= 2 (length form))
!          (byte-compile-form `(char-after (1- ,(nth 1 form)))))
!         ((= 1 (length form))
!          (byte-compile-form '(char-after (1- (point)))))
!         (t (byte-compile-subr-wrong-args form "0-1"))))
  
  (defun byte-compile-list (form)
    (let ((count (length (cdr form))))
--- 3164,3194 ----
  
  (defun byte-compile-char-before (form)
    (cond ((= 2 (length form))
! 	 (byte-compile-form (list 'char-after (if (numberp (nth 1 form))
! 						  (1- (nth 1 form))
! 						`(1- ,(nth 1 form))))))
! 	((= 1 (length form))
! 	 (byte-compile-form '(char-after (1- (point)))))
! 	(t (byte-compile-subr-wrong-args form "0-1"))))
! 
! ;; backward-... ==> forward-... with negated argument.
! (defun byte-compile-backward-char (form)
!   (cond ((= 2 (length form))
! 	 (byte-compile-form (list 'forward-char (if (numberp (nth 1 form))
! 						    (- (nth 1 form))
! 						  `(- ,(nth 1 form))))))
! 	((= 1 (length form))
! 	 (byte-compile-form '(forward-char -1)))
! 	(t (byte-compile-subr-wrong-args form "0-1"))))
! 
! (defun byte-compile-backward-word (form)
!   (cond ((= 2 (length form))
! 	 (byte-compile-form (list 'forward-word (if (numberp (nth 1 form))
! 						    (- (nth 1 form))
! 						  `(- ,(nth 1 form))))))
! 	((= 1 (length form))
! 	 (byte-compile-form '(forward-word -1)))
! 	(t (byte-compile-subr-wrong-args form "0-1"))))
  
  (defun byte-compile-list (form)
    (let ((count (length (cdr form))))

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Nonsensical byte compiler warning.
  2007-04-10 17:27                         ` Markus Triska
@ 2007-04-11  4:00                           ` Glenn Morris
  0 siblings, 0 replies; 29+ messages in thread
From: Glenn Morris @ 2007-04-11  4:00 UTC (permalink / raw)
  To: Markus Triska; +Cc: acm, cyd, rms, emacs-devel

Markus Triska wrote:

> Thank you. Here's the remaining part:

also installed

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2007-04-11  4:00 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-04-01 17:14 Nonsensical byte compiler warning David Kastrup
2007-04-01 18:10 ` Chong Yidong
2007-04-01 20:57   ` Alan Mackenzie
2007-04-02 12:29   ` Richard Stallman
2007-04-04  4:48     ` Markus Triska
2007-04-04  6:15       ` David Kastrup
2007-04-04  8:19         ` Markus Triska
2007-04-04  8:46           ` David Kastrup
2007-04-04  9:50             ` Markus Triska
2007-04-04 10:17               ` David Kastrup
2007-04-04 12:35                 ` Markus Triska
2007-04-04 18:25                 ` Markus Triska
2007-04-04 22:13                   ` David Kastrup
2007-04-05  6:52             ` Richard Stallman
2007-04-05  7:55               ` Markus Triska
2007-04-06 12:56                 ` Richard Stallman
2007-04-06 15:11                   ` Chong Yidong
2007-04-08 20:47                   ` Markus Triska
2007-04-09 15:42                     ` Richard Stallman
2007-04-10  3:53                       ` Glenn Morris
2007-04-10 17:27                         ` Markus Triska
2007-04-11  4:00                           ` Glenn Morris
2007-04-05 18:01               ` Chong Yidong
2007-04-04 20:08           ` Alan Mackenzie
2007-04-04 21:45             ` Markus Triska
2007-04-04 22:11               ` Chong Yidong
2007-04-05  5:44                 ` Markus Triska
2007-04-08  1:21             ` Kim F. Storm
2007-04-08 11:27               ` Alan Mackenzie

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).