[bojohan+mail@dd.chalmers.se: The `cl-functions' byte compiler warning doesn't work]

[bojohan+mail@dd.chalmers.se: The `cl-functions' byte compiler warning doesn't work]

[Richard Stallman]

Would someone please fix this in the trunk, then ack?

------- Start of forwarded message -------
X-Spam-Status: No, score=0.0 required=5.0 tests=UNPARSEABLE_RELAY 
	autolearn=failed version=3.1.0
From: bojohan+mail@dd.chalmers.se (Johan =?utf-8?Q?Bockg=C3=A5rd?=)
To: emacs-pretest-bug@gnu.org
Date: Fri, 08 Jun 2007 16:43:46 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Subject: The `cl-functions' byte compiler warning doesn't work


The file names in `load-history' are absolute nowadays, but
bytecomp.el expects relative names:

    (defun byte-compile-find-cl-functions ()
      (unless byte-compile-cl-functions
        (dolist (elt load-history)
          (when (and (stringp (car elt))
                     (string-match "^cl\\>" (car elt)))
            [...]

Also, there's a typo in the comment on line 101 of bytecomp.el.
"cl-warnings" should be "cl-functions".

- -- 
Johan Bockgård


_______________________________________________
emacs-pretest-bug mailing list
emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
------- End of forwarded message -------

Re: [bojohan+mail@dd.chalmers.se: The `cl-functions' byte compiler warning doesn't work]

[martin rudalics]

> Would someone please fix this in the trunk, then ack?
> 
> ------- Start of forwarded message -------
> X-Spam-Status: No, score=0.0 required=5.0 tests=UNPARSEABLE_RELAY 
> 	autolearn=failed version=3.1.0
> From: bojohan+mail@dd.chalmers.se (Johan =?utf-8?Q?Bockg=C3=A5rd?=)
> To: emacs-pretest-bug@gnu.org
> Date: Fri, 08 Jun 2007 16:43:46 +0200
> MIME-Version: 1.0
> Content-Type: text/plain; charset=utf-8
> Subject: The `cl-functions' byte compiler warning doesn't work
> 
> 
> The file names in `load-history' are absolute nowadays, but
> bytecomp.el expects relative names:
> 
>     (defun byte-compile-find-cl-functions ()
>       (unless byte-compile-cl-functions
>         (dolist (elt load-history)
>           (when (and (stringp (car elt))
>                      (string-match "^cl\\>" (car elt)))
>             [...]
> 
> Also, there's a typo in the comment on line 101 of bytecomp.el.
> "cl-warnings" should be "cl-functions".

I've checked in a fix.  Please try.

Re: [bojohan+mail@dd.chalmers.se: The `cl-functions' byte compiler warning doesn't work]

[Johan Bockgård]

martin rudalics <rudalics@gmx.at> writes:

> I've checked in a fix.  Please try.


The change is correct. However, I've found two further problems
in this function:

    (defun byte-compile-find-cl-functions ()
      (unless byte-compile-cl-functions
        (dolist (elt load-history)
          (when (and (stringp (car elt))
                     (string-match
                      "^cl\\>" (file-name-nondirectory (car elt))))
            (setq byte-compile-cl-functions
                  (append byte-compile-cl-functions
                          (cdr elt)))))
        (let ((tail byte-compile-cl-functions))
          (while tail
            (if (and (consp (car tail))
                     (eq (car (car tail)) 'autoload))
                (setcar tail (cdr (car tail))))
            (setq tail (cdr tail))))))

1. It looks for `(autoload . FOO)' entries in load-history, but not
for `(defun . FOO)' (it wrongly assumes that functions are represented
by plain symbols).

2. It modifies load-history as a side-effect.

(And byte-compile-cl-functions contains useless non-function
(require/t/provide) entries)

This illustrates the problems:

    $ emacs -Q

    (require 'cl)

    (byte-compile (lambda () (subst nil nil nil)))
      => no warning

    (byte-compile (lambda () (subst-if nil nil nil)))
      => Warning: Function `subst-if' from cl package called at runtime

    ;; The `(autoload . CL-FUNCTION)' entries in `load-history' have
       been replaced by `CL-FUNCTION'.

In the first case, the (defun . subst) entry is not respected.

In the second case, the `(autoload . subst-if)' entry is found. This
is transformed into a symbol by `setcar'ing structure shared between
byte-compile-cl-functions and load-history.


This patch uses the `(defun . FOO)' entry format, and avoids modifying
load-history:


--- bytecomp.el	13 Jun 2007 01:27:08 +0200	2.201
+++ bytecomp.el	13 Jun 2007 01:02:49 +0200	
@@ -1359,12 +1359,12 @@
 		 (string-match
 		  "^cl\\>" (file-name-nondirectory (car elt))))
 	(setq byte-compile-cl-functions
-	      (append byte-compile-cl-functions
-		      (cdr elt)))))
+	      (append (cdr elt)
+		      byte-compile-cl-functions))))
     (let ((tail byte-compile-cl-functions))
       (while tail
 	(if (and (consp (car tail))
-		 (eq (car (car tail)) 'autoload))
+		 (memq (car (car tail)) '(autoload defun)))
 	    (setcar tail (cdr (car tail))))
 	(setq tail (cdr tail))))))


-- 
Johan Bockgård

Re: [bojohan+mail@dd.chalmers.se: The `cl-functions' byte compiler warning doesn't work]

[Johan Bockgård]

So, byte-compile-find-cl-functions still has a few problems:

1. It expects functions to be represented by plain symbols
rather than (defun . FOO) in load-history.

2. It modifies load-history as a side-effect.

3.  byte-compile-cl-functions contains useless `(require/t/provide
    ...)' entries.


This illustrates the problem:

    $ emacs -Q

    (require 'cl)

    (byte-compile (lambda () (subst nil nil nil)))
      => no warning

    (byte-compile (lambda () (subst-if nil nil nil)))
      => Warning: Function `subst-if' from cl package called at runtime

    ;; The (autoload . <CL-FUNCTION>) entries in `load-history' are
       gone.


How about this version:

(defun byte-compile-find-cl-functions ()
  (unless byte-compile-cl-functions
    (dolist (elt load-history)
      (when (and (stringp (car elt))
		 (string-match
		  "^cl\\>" (file-name-nondirectory (car elt))))
	(dolist (e (cdr elt))
	  (when (memq (car-safe e) '(autoload defun))
	    (push (cdr e) byte-compile-cl-functions)))))))

2008-03-03 Johan Bockgård <bojohan@gnu.org>

	* emacs-lisp/bytecomp.el (byte-compile-find-cl-functions):
	Simplify.  Collect `defun' and `autoload' entries.  Avoid
	side-effecting load-history.



And I see now that Martin's change wasn't checked in on the branch,
which means that the original problem in this thread exists there (the
compiler never warns about cl functions):


2007-06-10  Martin Rudalics  <rudalics@gmx.at>

        * emacs-lisp/bytecomp.el (byte-compile-find-cl-functions):
        Match against file-name-nondirectory.


-- 
Johan Bockgård