Nonsensical byte compiler warning.

Nonsensical byte compiler warning.

[David Kastrup]

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

Re: Nonsensical byte compiler warning.

[Chong Yidong]

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)

Re: Nonsensical byte compiler warning.

[Alan Mackenzie]

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.

Re: Nonsensical byte compiler warning.

[Richard Stallman]

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

Re: Nonsensical byte compiler warning.

[Markus Triska]

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.

Re: Nonsensical byte compiler warning.

[David Kastrup]

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

Re: Nonsensical byte compiler warning.

[Markus Triska]

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.

Re: Nonsensical byte compiler warning.

[David Kastrup]

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

Re: Nonsensical byte compiler warning.

[Markus Triska]

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.

Re: Nonsensical byte compiler warning.

[David Kastrup]

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

Re: Nonsensical byte compiler warning.

[Markus Triska]

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.

Re: Nonsensical byte compiler warning.

[Markus Triska]

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.

Re: Nonsensical byte compiler warning.

[David Kastrup]

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

Re: Nonsensical byte compiler warning.

[Richard Stallman]

    > 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!

Re: Nonsensical byte compiler warning.

[Markus Triska]

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?

Re: Nonsensical byte compiler warning.

[Richard Stallman]

    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.

Re: Nonsensical byte compiler warning.

[Chong Yidong]

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.

Re: Nonsensical byte compiler warning.

[Markus Triska]

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)

Re: Nonsensical byte compiler warning.

[Richard Stallman]

    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.

Re: Nonsensical byte compiler warning.

[Glenn Morris]

Richard Stallman wrote:

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

installed

Re: Nonsensical byte compiler warning.

[Markus Triska]

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))))

Re: Nonsensical byte compiler warning.

[Glenn Morris]

Markus Triska wrote:

> Thank you. Here's the remaining part:

also installed

Re: Nonsensical byte compiler warning.

[Chong Yidong]

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).

Re: Nonsensical byte compiler warning.

[Alan Mackenzie]

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).

Re: Nonsensical byte compiler warning.

[Markus Triska]

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.

Re: Nonsensical byte compiler warning.

[Chong Yidong]

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?

Re: Nonsensical byte compiler warning.

[Markus Triska]

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.

Re: Nonsensical byte compiler warning.

[Kim F. Storm]

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

Re: Nonsensical byte compiler warning.

[Alan Mackenzie]

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.