assignment to free variable byte compiler warnings

assignment to free variable byte compiler warnings

[Dan Nicolaescu]

As has been discussed on emacs-devel the byte compiler does not issue
some free variable warnings because of using boundp on variable names
that are let-bound inside the byte compiler.  Renaming some let-bound
variables in the byte compiler reveals these warnings:



Compiling /tmp/emacs/lisp/./tree-widget.el

In tree-widget-lookup-image:
tree-widget.el:412:35:Warning: assignment to free variable `file'
tree-widget.el:413:34:Warning: reference to free variable `file'
Compiling /tmp/emacs/lisp/./vc-cvs.el

In vc-cvs-register:
vc-cvs.el:301:41:Warning: reference to free variable `file'
Compiling /tmp/emacs/lisp/./vc-mcvs.el

In vc-mcvs-register:
vc-mcvs.el:260:25:Warning: reference to free variable `file'
Compiling /tmp/emacs/lisp/./vc-rcs.el

In vc-rcs-register:
vc-rcs.el:265:62:Warning: reference to free variable `file'
Wrote /tmp/emacs/lisp/vc-rcs.elc
Compiling /tmp/emacs/lisp/./vc-sccs.el

In vc-sccs-diff:
vc-sccs.el:316:40:Warning: reference to free variable `file'


Can someone please fix them? 

Thanks

Re: assignment to free variable byte compiler warnings

[martin rudalics]

 > As has been discussed on emacs-devel the byte compiler does not issue
 > some free variable warnings because of using boundp on variable names
 > that are let-bound inside the byte compiler.  Renaming some let-bound
 > variables in the byte compiler reveals these warnings:

Unless you have already done so: Wouldn't it be reasonable if you
installed your re-namings in bytecomp.el?  This way there will be fewer
problems in the future - provided your names are chosen reasonably.

Re: assignment to free variable byte compiler warnings

[Glenn Morris]

Dan Nicolaescu wrote:

> In tree-widget-lookup-image:
> tree-widget.el:412:35:Warning: assignment to free variable `file'
> tree-widget.el:413:34:Warning: reference to free variable `file'

I fixed that one, but left the vc*.el ones. (Why are we having this
conversation on bug-gnu-emacs? I doubt the VC problems exist outside
the trunk.)

Re: assignment to free variable byte compiler warnings

[Dan Nicolaescu]

martin rudalics <rudalics@gmx.at> writes:

  > > As has been discussed on emacs-devel the byte compiler does not issue
  > > some free variable warnings because of using boundp on variable names
  > > that are let-bound inside the byte compiler.  Renaming some let-bound
  > > variables in the byte compiler reveals these warnings:
  > 
  > Unless you have already done so: Wouldn't it be reasonable if you
  > installed your re-namings in bytecomp.el?  

No opinion... If a bytecomp maintainers asks for it, I can install the change.

Re: assignment to free variable byte compiler warnings

[martin rudalics]

>   > Unless you have already done so: Wouldn't it be reasonable if you
>   > installed your re-namings in bytecomp.el?  
> 
> No opinion... If a bytecomp maintainers asks for it, I can install the change.

Why should bytecomp maintainers care about this problem?  It's a
problem they pose on you and me.

Re: assignment to free variable byte compiler warnings

[Glenn Morris]

Dan Nicolaescu wrote:

> No opinion... If a bytecomp maintainers asks for it, I can install
> the change.

Who are the "bytecomp maintainers"?

grep Maintainer byte*.el
byte-opt.el:;; Maintainer: FSF
byte-run.el:;; Maintainer: FSF
bytecomp.el:;; Maintainer: FSF

Re: assignment to free variable byte compiler warnings

[Dan Nicolaescu]

Glenn Morris <rgm@gnu.org> writes:

  > Who are the "bytecomp maintainers"?
  >
  > grep Maintainer byte*.el
  > byte-opt.el:;; Maintainer: FSF
  > byte-run.el:;; Maintainer: FSF
  > bytecomp.el:;; Maintainer: FSF

I'd guess anyone that wants to take responsibility...

For the record, here's the patch in question.  As it's a band-aid, and
it was not intended to be checked in, the new variable names are not
really consistent, just consistently ugly.

Index: bytecomp.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/bytecomp.el,v
retrieving revision 2.230
diff -u -3 -p -r2.230 bytecomp.el
--- bytecomp.el	12 Feb 2008 11:21:31 -0000	2.230
+++ bytecomp.el	24 Feb 2008 20:31:59 -0000
@@ -1574,34 +1574,34 @@ Files in subdirectories of DIRECTORY are
   (byte-recompile-directory directory nil t))
 
 ;;;###autoload
-(defun byte-recompile-directory (directory &optional arg force)
+(defun byte-recompile-directory (directory__ &optional arg__ force__)
   "Recompile every `.el' file in DIRECTORY that needs recompilation.
 This is if a `.elc' file exists but is older than the `.el' file.
 Files in subdirectories of DIRECTORY are processed also.
 
 If the `.elc' file does not exist, normally this function *does not*
 compile the corresponding `.el' file.  However,
-if ARG (the prefix argument) is 0, that means do compile all those files.
-A nonzero ARG means ask the user, for each such `.el' file,
+if ARG__ (the prefix argument) is 0, that means do compile all those files.
+A nonzero ARG__ means ask the user, for each such `.el' file,
 whether to compile it.
 
-A nonzero ARG also means ask about each subdirectory before scanning it.
+A nonzero ARG__ also means ask about each subdirectory before scanning it.
 
-If the third argument FORCE is non-nil,
+If the third argument FORCE__ is non-nil,
 recompile every `.el' file that already has a `.elc' file."
   (interactive "DByte recompile directory: \nP")
-  (if arg
-      (setq arg (prefix-numeric-value arg)))
+  (if arg__
+      (setq arg__ (prefix-numeric-value arg__)))
   (if noninteractive
       nil
     (save-some-buffers)
     (force-mode-line-update))
   (with-current-buffer (get-buffer-create "*Compile-Log*")
-    (setq default-directory (expand-file-name directory))
+    (setq default-directory (expand-file-name directory__))
     ;; compilation-mode copies value of default-directory.
     (unless (eq major-mode 'compilation-mode)
       (compilation-mode))
-    (let ((directories (list default-directory))
+    (let ((directories__ (list default-directory))
 	  (default-directory default-directory)
 	  (skip-count 0)
 	  (fail-count 0)
@@ -1609,51 +1609,51 @@ recompile every `.el' file that already 
 	  (dir-count 0)
 	  last-dir)
       (displaying-byte-compile-warnings
-       (while directories
-	 (setq directory (car directories))
-	 (message "Checking %s..." directory)
-	 (let ((files (directory-files directory))
-	       source dest)
-	   (dolist (file files)
-	     (setq source (expand-file-name file directory))
-	     (if (and (not (member file '("RCS" "CVS")))
-		      (not (eq ?\. (aref file 0)))
-		      (file-directory-p source)
-		      (not (file-symlink-p source)))
+       (while directories__
+	 (setq directory__ (car directories__))
+	 (message "Checking %s..." directory__)
+	 (let ((files_ (directory-files directory__))
+	       source__ dest__)
+	   (dolist (file_ files_)
+	     (setq source__ (expand-file-name file_ directory__))
+	     (if (and (not (member file_ '("RCS" "CVS")))
+		      (not (eq ?\. (aref file_ 0)))
+		      (file-directory-p source__)
+		      (not (file-symlink-p source__)))
 		 ;; This file is a subdirectory.  Handle them differently.
-		 (when (or (null arg)
-			   (eq 0 arg)
-			   (y-or-n-p (concat "Check " source "? ")))
-		   (setq directories
-			 (nconc directories (list source))))
+		 (when (or (null arg__)
+			   (eq 0 arg__)
+			   (y-or-n-p (concat "Check " source__ "? ")))
+		   (setq directories__
+			 (nconc directories__ (list source__))))
 	       ;; It is an ordinary file.  Decide whether to compile it.
-	       (if (and (string-match emacs-lisp-file-regexp source)
-			(file-readable-p source)
-			(not (auto-save-file-name-p source))
-			(setq dest (byte-compile-dest-file source))
-			(if (file-exists-p dest)
+	       (if (and (string-match emacs-lisp-file-regexp source__)
+			(file-readable-p source__)
+			(not (auto-save-file-name-p source__))
+			(setq dest__ (byte-compile-dest-file source__))
+			(if (file-exists-p dest__)
 			    ;; File was already compiled.
-			    (or force (file-newer-than-file-p source dest))
+			    (or force__ (file-newer-than-file-p source__ dest__))
 			  ;; No compiled file exists yet.
-			  (and arg
-			       (or (eq 0 arg)
-				   (y-or-n-p (concat "Compile " source "? "))))))
+			  (and arg__
+			       (or (eq 0 arg__)
+				   (y-or-n-p (concat "Compile " source__ "? "))))))
 		   (progn (if (and noninteractive (not byte-compile-verbose))
-			      (message "Compiling %s..." source))
-			  (let ((res (byte-compile-file source)))
-			    (cond ((eq res 'no-byte-compile)
+			      (message "Compiling %s..." source__))
+			  (let ((res__ (byte-compile-file source__)))
+			    (cond ((eq res__ 'no-byte-compile)
 				   (setq skip-count (1+ skip-count)))
-				  ((eq res t)
+				  ((eq res__ t)
 				   (setq file-count (1+ file-count)))
-				  ((eq res nil)
+				  ((eq res__ nil)
 				   (setq fail-count (1+ fail-count)))))
 			  (or noninteractive
-			      (message "Checking %s..." directory))
-			  (if (not (eq last-dir directory))
-			      (setq last-dir directory
+			      (message "Checking %s..." directory__))
+			  (if (not (eq last-dir directory__))
+			      (setq last-dir directory__
 				    dir-count (1+ dir-count)))
 			  )))))
-	 (setq directories (cdr directories))))
+	 (setq directories__ (cdr directories__))))
       (message "Done (Total of %d file%s compiled%s%s%s)"
 	       file-count (if (= file-count 1) "" "s")
 	       (if (> fail-count 0) (format ", %d failed" fail-count) "")
@@ -1668,45 +1668,45 @@ This is normally set in local file varia
 ;;;###autoload(put 'no-byte-compile 'safe-local-variable 'booleanp)
 
 ;;;###autoload
-(defun byte-compile-file (filename &optional load)
-  "Compile a file of Lisp code named FILENAME into a file of byte code.
-The output file's name is generated by passing FILENAME to the
+(defun byte-compile-file (filename__bcf &optional load)
+  "Compile a file of Lisp code named FILENAME__BCF into a file of byte code.
+The output file's name is generated by passing FILENAME__BCF to the
 `byte-compile-dest-file' function (which see).
 With prefix arg (noninteractively: 2nd arg), LOAD the file after compiling.
 The value is non-nil if there were no errors, nil if errors."
 ;;  (interactive "fByte compile file: \nP")
   (interactive
-   (let ((file buffer-file-name)
-	 (file-name nil)
-	 (file-dir nil))
-     (and file
+   (let ((file__bcf buffer-file-name)
+	 (file-name__bcf nil)
+	 (file-dir__bcf nil))
+     (and file__bcf
 	  (eq (cdr (assq 'major-mode (buffer-local-variables)))
 	      'emacs-lisp-mode)
-	  (setq file-name (file-name-nondirectory file)
-		file-dir (file-name-directory file)))
+	  (setq file-name__bcf (file-name-nondirectory file__bcf)
+		file-dir__bcf (file-name-directory file__bcf)))
      (list (read-file-name (if current-prefix-arg
 			       "Byte compile and load file: "
 			     "Byte compile file: ")
-			   file-dir file-name nil)
+			   file-dir__bcf file-name__bcf nil)
 	   current-prefix-arg)))
   ;; Expand now so we get the current buffer's defaults
-  (setq filename (expand-file-name filename))
+  (setq filename__bcf (expand-file-name filename__bcf))
 
   ;; If we're compiling a file that's in a buffer and is modified, offer
   ;; to save it first.
   (or noninteractive
-      (let ((b (get-file-buffer (expand-file-name filename))))
+      (let ((b (get-file-buffer (expand-file-name filename__bcf))))
 	(if (and b (buffer-modified-p b)
 		 (y-or-n-p (format "Save buffer %s first? " (buffer-name b))))
 	    (with-current-buffer b (save-buffer)))))
 
   ;; Force logging of the file name for each file compiled.
   (setq byte-compile-last-logged-file nil)
-  (let ((byte-compile-current-file filename)
+  (let ((byte-compile-current-file filename__bcf)
 	(set-auto-coding-for-load t)
 	target-file input-buffer output-buffer
 	byte-compile-dest-file)
-    (setq target-file (byte-compile-dest-file filename))
+    (setq target-file (byte-compile-dest-file filename__bcf))
     (setq byte-compile-dest-file target-file)
     (with-current-buffer
         (setq input-buffer (get-buffer-create " *Compiler Input*"))
@@ -1715,7 +1715,7 @@ The value is non-nil if there were no er
       ;; Always compile an Emacs Lisp file as multibyte
       ;; unless the file itself forces unibyte with -*-coding: raw-text;-*-
       (set-buffer-multibyte t)
-      (insert-file-contents filename)
+      (insert-file-contents filename__bcf)
       ;; Mimic the way after-insert-file-set-coding can make the
       ;; buffer unibyte when visiting this file.
       (when (or (eq last-coding-system-used 'no-conversion)
@@ -1725,7 +1725,7 @@ The value is non-nil if there were no er
 	(set-buffer-multibyte nil))
       ;; Run hooks including the uncompression hook.
       ;; If they change the file name, then change it for the output also.
-      (let ((buffer-file-name filename)
+      (let ((buffer-file-name filename__bcf)
 	    (default-major-mode 'emacs-lisp-mode)
 	    ;; Ignore unsafe local variables.
 	    ;; We only care about a few of them for our purposes.
@@ -1733,15 +1733,15 @@ The value is non-nil if there were no er
 	    (enable-local-eval nil))
 	;; Arg of t means don't alter enable-local-variables.
         (normal-mode t)
-        (setq filename buffer-file-name))
+        (setq filename__bcf buffer-file-name))
       ;; Set the default directory, in case an eval-when-compile uses it.
-      (setq default-directory (file-name-directory filename)))
+      (setq default-directory (file-name-directory filename__bcf)))
     ;; Check if the file's local variables explicitly specify not to
     ;; compile this file.
     (if (with-current-buffer input-buffer no-byte-compile)
 	(progn
 	  ;; (message "%s not compiled because of `no-byte-compile: %s'"
-	  ;; 	   (file-relative-name filename)
+	  ;; 	   (file-relative-name filename__bcf)
 	  ;; 	   (with-current-buffer input-buffer no-byte-compile))
 	  (when (file-exists-p target-file)
 	    (message "%s deleted because of `no-byte-compile: %s'"
@@ -1751,18 +1751,18 @@ The value is non-nil if there were no er
 	  ;; We successfully didn't compile this file.
 	  'no-byte-compile)
       (when byte-compile-verbose
-	(message "Compiling %s..." filename))
+	(message "Compiling %s..." filename__bcf))
       (setq byte-compiler-error-flag nil)
       ;; It is important that input-buffer not be current at this call,
       ;; so that the value of point set in input-buffer
       ;; within byte-compile-from-buffer lingers in that buffer.
       (setq output-buffer
 	    (save-current-buffer
-	      (byte-compile-from-buffer input-buffer filename)))
+	      (byte-compile-from-buffer input-buffer filename__bcf)))
       (if byte-compiler-error-flag
 	  nil
 	(when byte-compile-verbose
-	  (message "Compiling %s...done" filename))
+	  (message "Compiling %s...done" filename__bcf))
 	(kill-buffer input-buffer)
 	(with-current-buffer output-buffer
 	  (goto-char (point-max))
@@ -1791,9 +1791,9 @@ The value is non-nil if there were no er
 	  (kill-buffer (current-buffer)))
 	(if (and byte-compile-generate-call-tree
 		 (or (eq t byte-compile-generate-call-tree)
-		     (y-or-n-p (format "Report call tree for %s? " filename))))
+		     (y-or-n-p (format "Report call tree for %s? " filename__bcf))))
 	    (save-excursion
-	      (display-call-tree filename)))
+	      (display-call-tree filename__bcf)))
 	(if load
 	    (load target-file))
 	t))))
@@ -4225,50 +4225,50 @@ already up-to-date."
     (while command-line-args-left
       (if (file-directory-p (expand-file-name (car command-line-args-left)))
 	  ;; Directory as argument.
-	  (let ((files (directory-files (car command-line-args-left)))
-		source dest)
-	    (dolist (file files)
-	      (if (and (string-match emacs-lisp-file-regexp file)
-		       (not (auto-save-file-name-p file))
-		       (setq source (expand-file-name file
+	  (let ((files__ (directory-files (car command-line-args-left)))
+		source__bc dest__bc)
+	    (dolist (file__ files__)
+	      (if (and (string-match emacs-lisp-file-regexp file__)
+		       (not (auto-save-file-name-p file__))
+		       (setq source__bc (expand-file-name file__
 						      (car command-line-args-left)))
-		       (setq dest (byte-compile-dest-file source))
-		       (file-exists-p dest)
-		       (file-newer-than-file-p source dest))
-		  (if (null (batch-byte-compile-file source))
+		       (setq dest__bc (byte-compile-dest-file source__bc))
+		       (file-exists-p dest__bc)
+		       (file-newer-than-file-p source__bc dest__bc))
+		  (if (null (batch-byte-compile-file source__bc))
 		      (setq error t)))))
 	;; Specific file argument
 	(if (or (not noforce)
-		(let* ((source (car command-line-args-left))
-		       (dest (byte-compile-dest-file source)))
-		  (or (not (file-exists-p dest))
-		      (file-newer-than-file-p source dest))))
+		(let* ((source__bc (car command-line-args-left))
+		       (dest__bc (byte-compile-dest-file source__bc)))
+		  (or (not (file-exists-p dest__bc))
+		      (file-newer-than-file-p source__bc dest__bc))))
 	    (if (null (batch-byte-compile-file (car command-line-args-left)))
 		(setq error t))))
       (setq command-line-args-left (cdr command-line-args-left)))
     (kill-emacs (if error 1 0))))
 
-(defun batch-byte-compile-file (file)
+(defun batch-byte-compile-file (crt-bc-file)
   (if debug-on-error
-      (byte-compile-file file)
+      (byte-compile-file crt-bc-file)
     (condition-case err
-	(byte-compile-file file)
+	(byte-compile-file crt-bc-file)
       (file-error
        (message (if (cdr err)
 		    ">>Error occurred processing %s: %s (%s)"
 		  ">>Error occurred processing %s: %s")
-		file
+		crt-bc-file
 		(get (car err) 'error-message)
 		(prin1-to-string (cdr err)))
-       (let ((destfile (byte-compile-dest-file file)))
-	 (if (file-exists-p destfile)
-	     (delete-file destfile)))
+       (let ((destfile__ (byte-compile-dest-file crt-bc-file)))
+	 (if (file-exists-p destfile__)
+	     (delete-file destfile__)))
        nil)
       (error
        (message (if (cdr err)
 		    ">>Error occurred processing %s: %s (%s)"
 		  ">>Error occurred processing %s: %s")
-		file
+		crt-bc-file
 		(get (car err) 'error-message)
 		(prin1-to-string (cdr err)))
        nil))))

Re: assignment to free variable byte compiler warnings

[Richard Stallman]

Renaming those variables in bytecomp is a good idea
if nobody can find a more elegant solution.

Re: assignment to free variable byte compiler warnings

[martin rudalics]

> For the record, here's the patch in question.  As it's a band-aid, and
> it was not intended to be checked in, the new variable names are not
> really consistent, just consistently ugly.

Please make them consistently beautiful.

Re: assignment to free variable byte compiler warnings

[Glenn Morris]

Dan Nicolaescu wrote:

> In vc-sccs-diff:
> vc-sccs.el:316:40:Warning: reference to free variable `file'

I believe I fixed all except this one.

Re: assignment to free variable byte compiler warnings

[Dan Nicolaescu]

martin rudalics <rudalics@gmx.at> writes:

  > > For the record, here's the patch in question.  As it's a band-aid, and
  > > it was not intended to be checked in, the new variable names are not
  > > really consistent, just consistently ugly.
  > 
  > Please make them consistently beautiful.

They are ugly on purpose so that nobody in his right mind would use such
names.  But if you have an interest in this feel free to change the
patch and commit it.  I don't plan to do any more work on it.

Re: assignment to free variable byte compiler warnings

[Andreas Schwab]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> martin rudalics <rudalics@gmx.at> writes:
>
>   > > For the record, here's the patch in question.  As it's a band-aid, and
>   > > it was not intended to be checked in, the new variable names are not
>   > > really consistent, just consistently ugly.
>   > 
>   > Please make them consistently beautiful.
>
> They are ugly on purpose so that nobody in his right mind would use such
> names.

Using byte-compile- as prefix should be good enough.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: assignment to free variable byte compiler warnings

[Glenn Morris]

Glenn Morris wrote:

> Dan Nicolaescu wrote:
>
>> In vc-sccs-diff:
>> vc-sccs.el:316:40:Warning: reference to free variable `file'
>
> I believe I fixed all except this one.

That one is done now as well.