Bogus byte-compiler warnings

Bogus byte-compiler warnings

[Reiner Steib]

Hi,

when byte-compiling the following file
(cf. `gnus-display-time-event-handler' in `gnus-start.el')...

--8<---------------cut here---------------start------------->8---
(defun foo-func-1 ()
  (when (and (boundp 'foo-var)
	     (fboundp 'foo-1))
    (foo-1)))

(defun foo-func-2 ()
  (and (boundp 'foo-var)
       (fboundp 'foo-2)
       (foo-2)))

;; Cf. `gnus-display-time-event-handler' in `gnus-start.el'
--8<---------------cut here---------------end--------------->8---

... I get:

,----
| In end of data:
| rs-byte-compile-warnings.el:11:1:Warning: the function `foo-1' is not known to
|     be defined.
`----

foo-1 will not be called if it's not fbound.  (Emacs complains about
both, foo-1 and foo-2).  Maybe this warning could be avoided (after
the release?).

Does it make sense to change `gnus-display-time-event-handler' in
`gnus-start.el' to the form of `foo-func-2' just to avoid the warning?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

re: Bogus byte-compiler warnings

[Jonathan Yavner]

Reiner Steib writes:
> when byte-compiling the following file
>
> (defun foo-func-1 ()
>   (when (and (boundp 'foo-var)
> 	     (fboundp 'foo-1))
>     (foo-1)))
>
> | In end of data:
> | rs-byte-compile-warnings.el:11:1:Warning: the function `foo-1' is
> | not known to be defined.

Bytecomp has a documented hack for use in such cases:
  (defun foo-func-1 ()
    (when (fboundp 'foo-1)
      (when (boundp 'foo-var)
        (foo-1))))

The structure being looked for is "(if (fboundp 'X) BODY)" which 
suppresses the warning for X within BODY.  In your example, the 
presence of 'and' prevents the hack from matching the code.

RE: Bogus byte-compiler warnings

[Drew Adams]

> > when byte-compiling the following file
> >
> > (defun foo-func-1 ()
> >   (when (and (boundp 'foo-var)
> > 	     (fboundp 'foo-1))
> >     (foo-1)))
> >
> > | In end of data:
> > | rs-byte-compile-warnings.el:11:1:Warning: the function `foo-1' is
> > | not known to be defined.
>
> Bytecomp has a documented hack for use in such cases:
>   (defun foo-func-1 ()
>     (when (fboundp 'foo-1)
>       (when (boundp 'foo-var)
>         (foo-1))))
>
> The structure being looked for is "(if (fboundp 'X) BODY)" which
> suppresses the warning for X within BODY.  In your example, the
> presence of 'and' prevents the hack from matching the code.

Good to know, but what a shame that people would write less clear code just
to inhibit such spurious warnings.

I won't, though it means I now need to field emails from users wondering
about such messages in code I write (or else fill the code with comments
explaining which messages can be ignored in which Emacs versions).

I'm not suggesting things were better before the new crop of warning
messages, but those messages can be bothersome. The byte compiler is smarter
than before, but not yet smart enough to know when it's acting too smart.

Re: Bogus byte-compiler warnings

[Richard Stallman]

    foo-1 will not be called if it's not fbound.  (Emacs complains about
    both, foo-1 and foo-2).

What does that last sentence mean?  When I compile that file, there is
a warning for foo-1 but NO warning for foo-2.  In what sense does Emacs
"complain" about both?

The reason there is a warning for foo-1 is that the code in the
compiler to avoid such warnings recognizes only the simpler case (such
as foo-2) and not the more complex case that foo-1 is.

I would not mind if that code were made smarter.

Re: Bogus byte-compiler warnings

[Reiner Steib]

On Mon, Nov 13 2006, Richard Stallman wrote:

>     foo-1 will not be called if it's not fbound.  (Emacs complains about
>     both, foo-1 and foo-2).
>
> What does that last sentence mean?  When I compile that file, there is
> a warning for foo-1 but NO warning for foo-2.  In what sense does Emacs
> "complain" about both?

Sorry, it should read "Emacs 21 complains about both, foo-1 and foo-2".

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: Bogus byte-compiler warnings

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> > (defun foo-func-1 ()
> >   (when (and (boundp 'foo-var)
> > 	     (fboundp 'foo-1))
> >     (foo-1)))
>
>     foo-1 will not be called if it's not fbound.  (Emacs complains about
>     both, foo-1 and foo-2).
>
> The reason there is a warning for foo-1 is that the code in the
> compiler to avoid such warnings recognizes only the simpler case (such
> as foo-2) and not the more complex case that foo-1 is.
>
> I would not mind if that code were made smarter.

How bout this patch?

*** emacs/lisp/emacs-lisp/bytecomp.el.~2.186.~	2006-07-07 13:45:48.000000000 -0400
--- emacs/lisp/emacs-lisp/bytecomp.el	2006-11-16 12:27:07.000000000 -0500
***************
*** 3398,3432 ****
  If CONDITION's value is (not (featurep 'emacs)) or (featurep 'xemacs),
  that suppresses all warnings during execution of BODY."
    (declare (indent 1) (debug t))
!   `(let* ((fbound
! 	   (if (eq 'fboundp (car-safe ,condition))
! 	       (and (eq 'quote (car-safe (nth 1 ,condition)))
! 		    ;; Ignore if the symbol is already on the
! 		    ;; unresolved list.
! 		    (not (assq (nth 1 (nth 1 ,condition)) ; the relevant symbol
! 			       byte-compile-unresolved-functions))
! 		    (nth 1 (nth 1 ,condition)))))
! 	  (bound (if (or (eq 'boundp (car-safe ,condition))
! 			 (eq 'default-boundp (car-safe ,condition)))
! 		     (and (eq 'quote (car-safe (nth 1 ,condition)))
! 			  (nth 1 (nth 1 ,condition)))))
! 	  ;; Maybe add to the bound list.
! 	  (byte-compile-bound-variables
! 	   (if bound
! 	       (cons bound byte-compile-bound-variables)
! 	     byte-compile-bound-variables))
! 	  ;; Suppress all warnings, for code not used in Emacs.
! 	  (byte-compile-warnings
  	   (if (member ,condition '((featurep 'xemacs)
  				    (not (featurep 'emacs))))
! 	       nil byte-compile-warnings)))
       (unwind-protect
  	 (progn ,@body)
         ;; Maybe remove the function symbol from the unresolved list.
!        (if fbound
! 	   (setq byte-compile-unresolved-functions
! 		 (delq (assq fbound byte-compile-unresolved-functions)
! 		       byte-compile-unresolved-functions))))))
  
  (defun byte-compile-if (form)
    (byte-compile-form (car (cdr form)))
--- 3398,3438 ----
  If CONDITION's value is (not (featurep 'emacs)) or (featurep 'xemacs),
  that suppresses all warnings during execution of BODY."
    (declare (indent 1) (debug t))
!   `(let* ((byte-compile-warnings
! 	   ;; Suppress all warnings, for code not used in Emacs.
  	   (if (member ,condition '((featurep 'xemacs)
  				    (not (featurep 'emacs))))
! 	       nil byte-compile-warnings))
! 	  (byte-compile-bound-variables byte-compile-bound-variables)
! 	  binding fbound-list)
!      (mapc (lambda (subcondition)
! 	     (cond ((eq 'fboundp (car-safe subcondition))
! 		    (setq binding (and (eq 'quote (car-safe (nth 1 subcondition)))
! 				       ;; Ignore if the symbol is already on the
! 				       ;; unresolved list.
! 				       (not (assq (nth 1 (nth 1 subcondition))
! 						  byte-compile-unresolved-functions))
! 				       (nth 1 (nth 1 subcondition))))
! 		    (if binding (setq fbound-list (cons binding fbound-list))))
! 		   ((or (eq 'boundp (car-safe subcondition))
! 			(eq 'default-boundp (car-safe subcondition)))
! 		    (setq binding (and (eq 'quote (car-safe (nth 1 subcondition)))
! 				       (nth 1 (nth 1 subcondition))))
! 		    (if binding (setq byte-compile-bound-variables
! 				      (cons binding byte-compile-bound-variables))))))
! 	   ;; Inspect each element in an `and' condition; otherwise,
! 	   ;; inspect the condition itself.
! 	   (if (eq 'and (car-safe ,condition))
! 	       (cdr-safe ,condition)
! 	     (list ,condition)))
       (unwind-protect
  	 (progn ,@body)
         ;; Maybe remove the function symbol from the unresolved list.
!        (mapc (lambda (fun)
! 	       (setq byte-compile-unresolved-functions
! 		     (delq (assq fun byte-compile-unresolved-functions)
! 			   byte-compile-unresolved-functions)))
! 	     fbound-list))))
  
  (defun byte-compile-if (form)
    (byte-compile-form (car (cdr form)))

Re: Bogus byte-compiler warnings

[Davis Herring]

Chong Yidong <cyd <at> stupidchicken.com> writes:

> ! 	   (if (eq 'and (car-safe ,condition))
> ! 	       (cdr-safe ,condition)


Tiny point: you can just use `cdr' here, since `car-safe' returned non-nil.

Davis

Re: Bogus byte-compiler warnings

[Richard Stallman]

    > The reason there is a warning for foo-1 is that the code in the
    > compiler to avoid such warnings recognizes only the simpler case (such
    > as foo-2) and not the more complex case that foo-1 is.
    >
    > I would not mind if that code were made smarter.

    How bout this patch?

I think it would be better to save this for after the release.
Changes in this code are somewhat risky, and even if you have
been rather careful, you could easily break something.

Re: Bogus byte-compiler warnings

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     > The reason there is a warning for foo-1 is that the code in the
>     > compiler to avoid such warnings recognizes only the simpler case (such
>     > as foo-2) and not the more complex case that foo-1 is.
>     >
>     > I would not mind if that code were made smarter.
>
>     How bout this patch?
>
> I think it would be better to save this for after the release.
> Changes in this code are somewhat risky, and even if you have
> been rather careful, you could easily break something.

I had already checked it into CVS (since you put it in FOR-RELEASE, I
assumed you wanted it fixed).  But I agree that now is not the time
for such changes, so I've reverted the change.

Re: Bogus byte-compiler warnings

[Reiner Steib]

On Sun, Nov 19 2006, Chong Yidong wrote:
        ^^^^^^^^^^^

[ See <http://thread.gmane.org/gmane.emacs.devel/62115> for the
  complete thread. ]

> Richard Stallman <rms@gnu.org> writes:
>
>>     > The reason there is a warning for foo-1 is that the code in the
>>     > compiler to avoid such warnings recognizes only the simpler case (such
>>     > as foo-2) and not the more complex case that foo-1 is.
>>     >
>>     > I would not mind if that code were made smarter.
>>
>>     How bout this patch?
>>
>> I think it would be better to save this for after the release.
>> Changes in this code are somewhat risky, and even if you have
>> been rather careful, you could easily break something.
>
> I had already checked it into CVS (since you put it in FOR-RELEASE, I
> assumed you wanted it fixed).  But I agree that now is not the time
> for such changes, so I've reverted the change.

The patch below has been reverted because Richard didn't want to
install it before the release of Emacs 22.1.  Maybe it should be
installed now in the trunk?

--8<---------------cut here---------------start------------->8---
--- bytecomp.el	7 Jul 2006 16:34:44 -0000	2.186
+++ bytecomp.el	18 Nov 2006 21:07:17 -0000	2.187
@@ -3398,35 +3398,42 @@
 If CONDITION's value is (not (featurep 'emacs)) or (featurep 'xemacs),
 that suppresses all warnings during execution of BODY."
   (declare (indent 1) (debug t))
-  `(let* ((fbound
-	   (if (eq 'fboundp (car-safe ,condition))
-	       (and (eq 'quote (car-safe (nth 1 ,condition)))
-		    ;; Ignore if the symbol is already on the
-		    ;; unresolved list.
-		    (not (assq (nth 1 (nth 1 ,condition)) ; the relevant symbol
-			       byte-compile-unresolved-functions))
-		    (nth 1 (nth 1 ,condition)))))
-	  (bound (if (or (eq 'boundp (car-safe ,condition))
-			 (eq 'default-boundp (car-safe ,condition)))
-		     (and (eq 'quote (car-safe (nth 1 ,condition)))
-			  (nth 1 (nth 1 ,condition)))))
-	  ;; Maybe add to the bound list.
-	  (byte-compile-bound-variables
-	   (if bound
-	       (cons bound byte-compile-bound-variables)
-	     byte-compile-bound-variables))
-	  ;; Suppress all warnings, for code not used in Emacs.
-	  (byte-compile-warnings
+  `(let* ((byte-compile-warnings
+	   ;; Suppress all warnings, for code not used in Emacs.
 	   (if (member ,condition '((featurep 'xemacs)
 				    (not (featurep 'emacs))))
-	       nil byte-compile-warnings)))
+	       nil
+	     byte-compile-warnings))
+	  (byte-compile-bound-variables byte-compile-bound-variables)
+	  binding fbound-list)
+     (mapc (lambda (subcondition)
+	     (cond ((eq 'fboundp (car-safe subcondition))
+		    (setq binding (and (eq 'quote (car-safe (nth 1 subcondition)))
+				       ;; Ignore if the symbol is already on the
+				       ;; unresolved list.
+				       (not (assq (nth 1 (nth 1 subcondition))
+						  byte-compile-unresolved-functions))
+				       (nth 1 (nth 1 subcondition))))
+		    (if binding (setq fbound-list (cons binding fbound-list))))
+		   ((or (eq 'boundp (car-safe subcondition))
+			(eq 'default-boundp (car-safe subcondition)))
+		    (setq binding (and (eq 'quote (car-safe (nth 1 subcondition)))
+				       (nth 1 (nth 1 subcondition))))
+		    (if binding (setq byte-compile-bound-variables
+				      (cons binding byte-compile-bound-variables))))))
+	   ;; Inspect each element in an `and' condition; otherwise,
+	   ;; inspect the condition itself.
+	   (if (eq 'and (car-safe ,condition))
+	       (cdr ,condition)
+	     (list ,condition)))
      (unwind-protect
 	 (progn ,@body)
        ;; Maybe remove the function symbol from the unresolved list.
-       (if fbound
-	   (setq byte-compile-unresolved-functions
-		 (delq (assq fbound byte-compile-unresolved-functions)
-		       byte-compile-unresolved-functions))))))
+       (mapc (lambda (fun)
+	       (setq byte-compile-unresolved-functions
+		     (delq (assq fun byte-compile-unresolved-functions)
+			   byte-compile-unresolved-functions)))
+	     fbound-list))))
 
 (defun byte-compile-if (form)
   (byte-compile-form (car (cdr form)))
--8<---------------cut here---------------end--------------->8---

Davis Herring commented:

> Chong Yidong <cyd <at> stupidchicken.com> writes:
>> ! 	   (if (eq 'and (car-safe ,condition))
>> ! 	       (cdr-safe ,condition)
> Tiny point: you can just use `cdr' here, since `car-safe' returned non-nil.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: Bogus byte-compiler warnings

[Dan Nicolaescu]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

  > On Sun, Nov 19 2006, Chong Yidong wrote:
  >         ^^^^^^^^^^^
  > 
  > [ See <http://thread.gmane.org/gmane.emacs.devel/62115> for the
  >   complete thread. ]
  > 
  > > Richard Stallman <rms@gnu.org> writes:
  > >
  > >>     > The reason there is a warning for foo-1 is that the code in the
  > >>     > compiler to avoid such warnings recognizes only the simpler case (such
  > >>     > as foo-2) and not the more complex case that foo-1 is.
  > >>     >
  > >>     > I would not mind if that code were made smarter.
  > >>
  > >>     How bout this patch?
  > >>
  > >> I think it would be better to save this for after the release.
  > >> Changes in this code are somewhat risky, and even if you have
  > >> been rather careful, you could easily break something.
  > >
  > > I had already checked it into CVS (since you put it in FOR-RELEASE, I
  > > assumed you wanted it fixed).  But I agree that now is not the time
  > > for such changes, so I've reverted the change.
  > 
  > The patch below has been reverted because Richard didn't want to
  > install it before the release of Emacs 22.1.  Maybe it should be
  > installed now in the trunk?

The example given at the start of this thread does not produce any
warning now (I fixed it on 2007-11-10). So unless there are other cases
that produce bogus warnings, this patch should not be needed anymore.

Re: Bogus byte-compiler warnings

[Reiner Steib]

On Sat, Dec 01 2007, Dan Nicolaescu wrote:

> The example given at the start of this thread does not produce any
> warning now (I fixed it on 2007-11-10). So unless there are other cases
> that produce bogus warnings, this patch should not be needed anymore.

Oops, I should have checked this myself.  Thanks.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/