Lexical byte-compilation warnings cleanup

Lexical byte-compilation warnings cleanup

[Daniel Hackney]

[-- Attachment #1: Type: text/plain, Size: 3032 bytes --]

I'm taking the advice Stafan gave in [1] and squashing lexical-scoping
bugs identified by the byte compiler. I've attached a patch which starts
making a dent in the warnings. There were some things which were
relatively easy, such as adding a leading underscore to `&rest _ignore',
but there were a lot what seem to be false positives. I've taken some
notes on what I've found.

 - `condition-case': The byte-compiler doesn't recognize the handler
   forms and so gives warnings about unused lexical arguments. Consider
   the following code (adapted from `widget-sexp-validate'):

   #+BEGIN_SRC emacs-lisp
     ;; -*- lexical-binding: t; -*-

     (defun foo ()
       (let (err)
         (condition-case data
             (skip-syntax-forward "\\s-")
           (end-of-file (setq err "Unbalanced sexp"))      ;; 1
           (error (setq err (error-message-string data)))) ;; 2
         err))
   #+END_SRC

   Byte-compiling that with `byte-compile-force-lexical-warnings' set
   to `t' warns of "Unused lexical argument `data'". Switching lines 1
   and 2 gives the warning "the function `end-of-file' is not known to
   be defined." Removing line 1 makes it compile without errors. It
   would seem that the byte-compiler is not treating `condition-case'
   as specially as it should.

 - I'm seeing a lot of "Argument foo is not a lexical variable", such as
   in vc/ediff-diff.el:532:56. What does this mean and is this something
   I should "fix"? In some cases, this is because `foo' is defined with
   `defvar' but also used as a function argument. What should be done in
   this case? For example, here is the definition of
   `emerge-remote-exit':

   #+BEGIN_SRC emacs-lisp
     (defun emerge-remote-exit (file-out emerge-exit-func)
       (emerge-write-and-delete file-out)
       (kill-buffer emerge-merge-buffer)
       (funcall emerge-exit-func (if emerge-prefix-argument 1 0)))
   #+END_SRC

   Note that `emerge-exit-func' is a `defvar'ed variable. Should it be
   replaced with something like this:

   #+BEGIN_SRC emacs-lisp
     (defun emerge-remote-exit (file-out exit-func)
       (let ((emerge-exit-func exit-func))
         (emerge-write-and-delete file-out)
         (kill-buffer emerge-merge-buffer)
         (funcall emerge-exit-func (if emerge-prefix-argument 1 0))))
   #+END_SRC

 - In `emerge-revisions-with-ancestor', the variable `cmd' is let-bound,
   but does not appear to be used. Could it be safely removed?

 - Similarly, `cvs-fileinfo<' binds variables `subtypea' and `subtypeb'
   but never uses them. Can they be removed?

 - Superfluously-bound variables: In, for example,
   `ange-ftp-file-attributes', the variables `host', `user', and `name'
   are bound but never used. Should these be prefixed with an
   underscore (to make the byte compiler shut up) or removed altogether
   (since they aren't actually used)? It seems like they should just be
   removed, but I don't want to trample on something needed.

[1] http://article.gmane.org/gmane.emacs.devel/162466

--
Daniel Hackney

[-- Attachment #2: byte-silence.patch --]
[-- Type: application/octet-stream, Size: 36150 bytes --]

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 8c9a21d..5b6665e 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,27 @@
+2013-08-19 Daniel Hackney  <dan@haxney.org>
+
+	* dired-x.el:
+	* net/ange-ftp.el:
+	* net/browse-url.el:
+	* net/dbus.el:
+	* net/eudc.el:
+	* net/eudcb-ldap.el:
+	* net/eww.el:
+	* net/imap.el:
+	* printing.el:
+	* vc/ediff-diff.el:
+	* vc/ediff-init.el:
+	* vc/ediff-merg.el:
+	* vc/ediff-mult.el:
+	* vc/ediff-util.el:
+	* vc/ediff-wind.el:
+	* vc/ediff.el:
+	* vc/emerge.el:
+	* vc/pcvs.el:
+	* vc/vc-annotate.el: Prefix unused arguments with `_' to silence
+	byte compiler. Remove some unused let-bound variables.
+
+
 2013-08-17  Michael Albinus  <michael.albinus@gmx.de>
 
 	* net/tramp.el:
diff --git a/lisp/dired-x.el b/lisp/dired-x.el
index 0c43259..3527a3f 100644
--- a/lisp/dired-x.el
+++ b/lisp/dired-x.el
@@ -1185,7 +1185,7 @@ results in
           (setq count (1+ count)
                 start (1+ start)))
         ;; ... and prepend a "../" for each slash found:
-        (dotimes (_n count)
+        (dotimes (n count)
           (setq name1 (concat "../" name1)))))
     (make-symbolic-link
      (directory-file-name name1)        ; must not link to foo/
diff --git a/lisp/net/ange-ftp.el b/lisp/net/ange-ftp.el
index 177fdac..67c74f8 100644
--- a/lisp/net/ange-ftp.el
+++ b/lisp/net/ange-ftp.el
@@ -3733,7 +3733,7 @@ so return the size on the remote host exactly. See RFC 3659."
 ;; next part of copying routine.
 (defun ange-ftp-cf1 (result line
 			    filename newname binary msg
-			    f-parsed f-host f-user f-name f-abbr
+			    f-parsed f-host f-user _f-name f-abbr
 			    t-parsed t-host t-user t-name t-abbr
 			    temp1 temp2 cont nowait)
   (if line
@@ -3835,7 +3835,7 @@ so return the size on the remote host exactly. See RFC 3659."
 
 (defun ange-ftp-copy-file (filename newname &optional ok-if-already-exists
 				    keep-date preserve-uid-gid
-				    preserve-selinux-context)
+				    _preserve-selinux-context)
   (interactive "fCopy file: \nFCopy %s to file: \np")
   (ange-ftp-copy-file-internal filename
 			       newname
@@ -4200,7 +4200,7 @@ directory, so that Emacs will know its current contents."
 	(while (and tryfiles (not copy))
 	  (catch 'ftp-error
 	    (let ((ange-ftp-waiting-flag t))
-	      (condition-case error
+	      (condition-case _error
 		  (setq copy (ange-ftp-file-local-copy (car tryfiles)))
 		(ftp-error nil))))
 	  (setq tryfiles (cdr tryfiles)))
@@ -4214,7 +4214,7 @@ directory, so that Emacs will know its current contents."
     (ange-ftp-real-load file noerror nomessage nosuffix)))
 
 ;; Calculate default-unhandled-directory for a given ange-ftp buffer.
-(defun ange-ftp-unhandled-file-name-directory (filename)
+(defun ange-ftp-unhandled-file-name-directory (_filename)
   nil)
 
 \f
@@ -4605,7 +4605,6 @@ NEWNAME should be the name to give the new compressed or uncompressed file.")
 (defun ange-ftp-shell-command (command &optional output-buffer error-buffer)
   (let* ((parsed (ange-ftp-ftp-name default-directory))
 	 (host (nth 0 parsed))
-	 (user (nth 1 parsed))
 	 (name (nth 2 parsed)))
     (if (not parsed)
 	(ange-ftp-real-shell-command command output-buffer error-buffer)
@@ -5176,7 +5175,7 @@ Other orders of $ and _ seem to all work just fine.")
 		    ;; versions left. If not, then delete the
 		    ;; root entry.
 		    (maphash
-		     (lambda (key val)
+		     (lambda (key _val)
 		       (and (string-match regexp key)
 			    (setq versions t)))
 		     files)
@@ -5358,7 +5357,7 @@ Other orders of $ and _ seem to all work just fine.")
 ;; compressed files. Instead, we turn "FILE.TYPE" into
 ;; "FILE.TYPE-Z". Hope that this is a reasonable thing to do.
 
-(defun ange-ftp-vms-make-compressed-filename (name &optional reverse)
+(defun ange-ftp-vms-make-compressed-filename (name &optional _reverse)
   (cond
    ((string-match "-Z;[0-9]+\\'" name)
     (list nil (substring name 0 (match-beginning 0))))
@@ -5399,7 +5398,7 @@ Other orders of $ and _ seem to all work just fine.")
 ;;	  (cons '(vms . ange-ftp-dired-vms-ls-trim)
 ;;		ange-ftp-dired-ls-trim-alist)))
 
-(defun ange-ftp-vms-sans-version (name &rest args)
+(defun ange-ftp-vms-sans-version (name &rest _args)
   (save-match-data
     (if (string-match ";[0-9]+\\'" name)
 	(substring name 0 (match-beginning 0))
@@ -5920,7 +5919,7 @@ Other orders of $ and _ seem to all work just fine.")
 ;;	  (cons '(cms . ange-ftp-dired-cms-move-to-end-of-filename)
 ;;		ange-ftp-dired-move-to-end-of-filename-alist)))
 
-(defun ange-ftp-cms-make-compressed-filename (name &optional reverse)
+(defun ange-ftp-cms-make-compressed-filename (name &optional _reverse)
   (if (string-match "-Z\\'" name)
       (list nil (substring name 0 -2))
     (list t (concat name "-Z"))))
diff --git a/lisp/net/browse-url.el b/lisp/net/browse-url.el
index 70173db..ff654f2 100644
--- a/lisp/net/browse-url.el
+++ b/lisp/net/browse-url.el
@@ -868,7 +868,7 @@ to use."
 (defvar dos-windows-version)
 (declare-function w32-shell-execute "w32fns.c")    ;; Defined in C.
 
-(defun browse-url-default-windows-browser (url &optional new-window)
+(defun browse-url-default-windows-browser (url &optional _new-window)
   (interactive (browse-url-interactive-arg "URL: "))
   (cond ((eq system-type 'ms-dos)
 	 (if dos-windows-version
@@ -878,7 +878,7 @@ to use."
 	 (call-process "cygstart" nil nil nil url))
 	(t (w32-shell-execute "open" url))))
 
-(defun browse-url-default-macosx-browser (url &optional new-window)
+(defun browse-url-default-macosx-browser (url &optional _new-window)
   (interactive (browse-url-interactive-arg "URL: "))
   (start-process (concat "open " url) nil "open" url))
 
@@ -933,7 +933,7 @@ used instead of `browse-url-new-window-flag'."
     ((executable-find browse-url-xterm-program) 'browse-url-text-xterm)
     ((locate-library "w3") 'browse-url-w3)
     (t
-     (lambda (&rest ignore) (error "No usable browser found"))))
+     (lambda (&rest _ignore) (error "No usable browser found"))))
    url args))
 
 (defun browse-url-can-use-xdg-open ()
@@ -1163,7 +1163,7 @@ URL in a new window."
 	       (append browse-url-firefox-startup-arguments (list url))))))
 
 ;;;###autoload
-(defun browse-url-chromium (url &optional new-window)
+(defun browse-url-chromium (url &optional _new-window)
   "Ask the Chromium WWW browser to load URL.
 Default to the URL around or before point.  The strings in
 variable `browse-url-chromium-arguments' are also passed to
@@ -1272,7 +1272,7 @@ used instead of `browse-url-new-window-flag'."
 (defvar url-handler-regexp)
 
 ;;;###autoload
-(defun browse-url-emacs (url &optional new-window)
+(defun browse-url-emacs (url &optional _new-window)
   "Ask Emacs to load URL into a buffer and show it in another window."
   (interactive (browse-url-interactive-arg "URL: "))
   (require 'url-handlers)
@@ -1413,7 +1413,7 @@ used instead of `browse-url-new-window-flag'."
     (w3-fetch url)))
 
 ;;;###autoload
-(defun browse-url-w3-gnudoit (url &optional new-window)
+(defun browse-url-w3-gnudoit (url &optional _new-window)
   ;; new-window ignored
   "Ask another Emacs running gnuserv to load the URL using the W3 browser.
 The `browse-url-gnudoit-program' program is used with options given by
@@ -1428,7 +1428,7 @@ The `browse-url-gnudoit-program' program is used with options given by
 ;; --- Lynx in an xterm ---
 
 ;;;###autoload
-(defun browse-url-text-xterm (url &optional new-window)
+(defun browse-url-text-xterm (url &optional _new-window)
   ;; new-window ignored
   "Ask a text browser to load URL.
 URL defaults to the URL around or before point.
@@ -1492,7 +1492,7 @@ used instead of `browse-url-new-window-flag'."
            (get-buffer-process buf)
            ;; Don't leave around a dead one (especially because of its
            ;; munged keymap.)
-           (lambda (process event)
+           (lambda (process _event)
              (if (not (memq (process-status process) '(run stop)))
                  (let ((buf (process-buffer process)))
                    (if buf (kill-buffer buf)))))))
@@ -1565,7 +1565,7 @@ used instead of `browse-url-new-window-flag'."
 ;; --- Random browser ---
 
 ;;;###autoload
-(defun browse-url-generic (url &optional new-window)
+(defun browse-url-generic (url &optional _new-window)
   ;; new-window ignored
   "Ask the WWW browser defined by `browse-url-generic-program' to load URL.
 Default to the URL around or before point.  A fresh copy of the
@@ -1580,7 +1580,7 @@ don't offer a form of remote control."
 	 (append browse-url-generic-args (list url))))
 
 ;;;###autoload
-(defun browse-url-kde (url &optional new-window)
+(defun browse-url-kde (url &optional _new-window)
   "Ask the KDE WWW browser to load URL.
 Default to the URL around or before point."
   (interactive (browse-url-interactive-arg "KDE URL: "))
diff --git a/lisp/net/dbus.el b/lisp/net/dbus.el
index 0e9c4fc..637e92f 100644
--- a/lisp/net/dbus.el
+++ b/lisp/net/dbus.el
@@ -818,13 +818,13 @@ association to the service from D-Bus."
 \f
 ;;; D-Bus type conversion.
 
-(defun dbus-string-to-byte-array (string)
-  "Transforms STRING to list (:array :byte c1 :byte c2 ...).
-STRING shall be UTF8 coded."
-  (if (zerop (length string))
+(defun dbus-string-to-byte-array (str)
+  "Transforms STR to list (:array :byte c1 :byte c2 ...).
+STR shall be UTF8 coded."
+  (if (zerop (length str))
       '(:array :signature "y")
     (let (result)
-      (dolist (elt (string-to-list string) (append '(:array) result))
+      (dolist (elt (string-to-list str) (append '(:array) result))
 	(setq result (append result (list :byte elt)))))))
 
 (defun dbus-byte-array-to-string (byte-array)
@@ -1609,7 +1609,6 @@ and \"org.freedesktop.DBus.Properties.GetAll\", which is slow."
 It will be registered for all objects created by `dbus-register-method'."
   (let* ((last-input-event last-input-event)
 	 (bus (dbus-event-bus-name last-input-event))
-	 (service (dbus-event-service-name last-input-event))
 	 (path (dbus-event-path-name last-input-event)))
     ;; "GetManagedObjects" returns "a{oa{sa{sv}}}".
     (let (interfaces result)
@@ -1625,8 +1624,7 @@ It will be registered for all objects created by `dbus-register-method'."
       ;; Check all registered object paths.
       (maphash
        (lambda (key val)
-	 (let ((object (or (nth 2 (car-safe val)) ""))
-	       (interface (nth 2 key)))
+	 (let ((object (or (nth 2 (car-safe val)) "")))
 	   (when (and (equal (butlast key 2) (list :method bus))
 		      (string-prefix-p path object))
 	     (dolist (interface (cons (nth 2 key) interfaces))
diff --git a/lisp/net/eudc.el b/lisp/net/eudc.el
index ef09267..c474ac9 100644
--- a/lisp/net/eudc.el
+++ b/lisp/net/eudc.el
@@ -518,12 +518,12 @@ otherwise they are formatted according to `eudc-user-attribute-names-alist'."
 	   precords))
 	(insert "\n")
 	(widget-create 'push-button
-		       :notify (lambda (&rest ignore)
+		       :notify (lambda (&rest _ignore)
 				 (eudc-query-form))
 		       "New query")
 	(widget-insert " ")
 	(widget-create 'push-button
-		       :notify (lambda (&rest ignore)
+		       :notify (lambda (&rest _ignore)
 				 (kill-this-buffer))
 		       "Quit")
 	(eudc-mode)
@@ -995,17 +995,17 @@ queries the server for the existing fields and displays a corresponding form."
 	  fields)
     (widget-insert "\n\n")
     (widget-create 'push-button
-		   :notify (lambda (&rest ignore)
+		   :notify (lambda (&rest _ignore)
 			     (eudc-process-form))
 		   "Query Server")
     (widget-insert " ")
     (widget-create 'push-button
-		   :notify (lambda (&rest ignore)
+		   :notify (lambda (&rest _ignore)
 			     (eudc-query-form))
 		   "Reset Form")
     (widget-insert " ")
     (widget-create 'push-button
-		   :notify (lambda (&rest ignore)
+		   :notify (lambda (&rest _ignore)
 			     (kill-this-buffer))
 		   "Quit")
     (goto-char pt)
diff --git a/lisp/net/eudcb-ldap.el b/lisp/net/eudcb-ldap.el
index d0ba47a..a678ec3 100644
--- a/lisp/net/eudcb-ldap.el
+++ b/lisp/net/eudcb-ldap.el
@@ -136,7 +136,7 @@ RETURN-ATTRS is a list of attributes to return, defaulting to
 	 result))
     final-result))
 
-(defun eudc-ldap-get-field-list (dummy &optional objectclass)
+(defun eudc-ldap-get-field-list (_dummy &optional objectclass)
   "Return a list of valid attribute names for the current server.
 OBJECTCLASS is the LDAP object class for which the valid
 attribute names are returned. Default to `person'"
diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 34934a0..48e3e26 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -296,7 +296,7 @@ word(s) will be searched for via `eww-search-prefix'."
 				  (list :background (car new-colors))
 				  t))))))
 
-(defun eww-display-raw (charset)
+(defun eww-display-raw (_charset)
   (let ((data (buffer-substring (point) (point-max))))
     (eww-setup-buffer)
     (let ((inhibit-read-only t))
@@ -381,7 +381,7 @@ word(s) will be searched for via `eww-search-prefix'."
 	eww-history))
 
 ;;;###autoload
-(defun eww-browse-url (url &optional new-window)
+(defun eww-browse-url (url &optional _new-window)
   (when (and (equal major-mode 'eww-mode)
 	     eww-current-url)
     (eww-save-history))
@@ -930,8 +930,7 @@ The browser to used is specified by the `shr-external-browser' variable."
       (setq file "!"))
      ((string-match "\\`[.]" file)
       (setq file (concat "!" file))))
-    (let ((base file)
-	  (count 1))
+    (let ((count 1))
       (while (file-exists-p (expand-file-name file directory))
 	(setq file
 	      (if (string-match "\\`\\(.*\\)\\([.][^.]+\\)" file)
diff --git a/lisp/net/imap.el b/lisp/net/imap.el
index 9584ceb..256af72 100644
--- a/lisp/net/imap.el
+++ b/lisp/net/imap.el
@@ -139,7 +139,7 @@
 (eval-when-compile (require 'cl))
 (eval-and-compile
   ;; For Emacs <22.2 and XEmacs.
-  (unless (fboundp 'declare-function) (defmacro declare-function (&rest r)))
+  (unless (fboundp 'declare-function) (defmacro declare-function (&rest _r)))
   (autoload 'starttls-open-stream "starttls")
   (autoload 'starttls-negotiate "starttls")
   (autoload 'sasl-find-mechanism "sasl")
@@ -661,7 +661,7 @@ sure of changing the value of `foo'."
 	      nil)))))
     done))
 
-(defun imap-ssl-p (buffer)
+(defun imap-ssl-p (_buffer)
   nil)
 
 (defun imap-ssl-open (name buffer server port)
@@ -711,7 +711,7 @@ sure of changing the value of `foo'."
       (message "imap: Opening SSL connection with `%s'...failed" cmd)
       nil)))
 
-(defun imap-tls-p (buffer)
+(defun imap-tls-p (_buffer)
   nil)
 
 (defun imap-tls-open (name buffer server port)
@@ -738,7 +738,7 @@ sure of changing the value of `foo'."
       (when (memq (process-status process) '(open run))
 	process))))
 
-(defun imap-network-p (buffer)
+(defun imap-network-p (_buffer)
   t)
 
 (defun imap-network-open (name buffer server port)
@@ -757,7 +757,7 @@ sure of changing the value of `foo'."
       (when (memq (process-status process) '(open run))
 	process))))
 
-(defun imap-shell-p (buffer)
+(defun imap-shell-p (_buffer)
   nil)
 
 (defun imap-shell-open (name buffer server port)
@@ -881,10 +881,10 @@ t if it successfully authenticates, nil otherwise."
       ;;		       passwd nil))))
       ret)))
 
-(defun imap-gssapi-auth-p (buffer)
+(defun imap-gssapi-auth-p (_buffer)
   (eq imap-stream 'gssapi))
 
-(defun imap-gssapi-auth (buffer)
+(defun imap-gssapi-auth (_buffer)
   (message "imap: Authenticating using GSSAPI...%s"
 	   (if (eq imap-stream 'gssapi) "done" "failed"))
   (eq imap-stream 'gssapi))
@@ -893,7 +893,7 @@ t if it successfully authenticates, nil otherwise."
   (and (imap-capability 'AUTH=KERBEROS_V4 buffer)
        (eq imap-stream 'kerberos4)))
 
-(defun imap-kerberos4-auth (buffer)
+(defun imap-kerberos4-auth (_buffer)
   (message "imap: Authenticating using Kerberos 4...%s"
 	   (if (eq imap-stream 'kerberos4) "done" "failed"))
   (eq imap-stream 'kerberos4))
@@ -947,7 +947,7 @@ t if it successfully authenticates, nil otherwise."
 						(imap-quote-specials passwd)
 						"\""))))))
 
-(defun imap-anonymous-p (buffer)
+(defun imap-anonymous-p (_buffer)
   t)
 
 (defun imap-anonymous-auth (buffer)
@@ -1838,7 +1838,7 @@ See `imap-enable-exchange-bug-workaround'."
 	    (and (imap-fetch-safe '("*" . "*:*") "UID")
 		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
 		       (apply 'max (imap-message-map
-				    (lambda (uid prop) uid) 'UID))))
+				    (lambda (uid _prop) uid) 'UID))))
 	  (if old-mailbox
 	      (imap-mailbox-select old-mailbox (eq state 'examine))
 	    (imap-mailbox-unselect)))))))
@@ -1884,7 +1884,7 @@ first element.  The rest of list contains the saved articles' UIDs."
 	    (and (imap-fetch-safe '("*" . "*:*") "UID")
 		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
 		       (apply 'max (imap-message-map
-				    (lambda (uid prop) uid) 'UID))))
+				    (lambda (uid _prop) uid) 'UID))))
 	  (if old-mailbox
 	      (imap-mailbox-select old-mailbox (eq state 'examine))
 	    (imap-mailbox-unselect)))))))
@@ -1893,7 +1893,7 @@ first element.  The rest of list contains the saved articles' UIDs."
   (with-current-buffer (or buffer (current-buffer))
     (imap-message-appenduid-1 (imap-utf7-encode mailbox))))
 
-(defun imap-message-append (mailbox article &optional flags date-time buffer)
+(defun imap-message-append (mailbox article &optional _flags _date-time buffer)
   "Append ARTICLE (a buffer) to MAILBOX on server in BUFFER.
 FLAGS and DATE-TIME is currently not used.  Return a cons holding
 uidvalidity of MAILBOX and UID the newly created article got, or nil
diff --git a/lisp/printing.el b/lisp/printing.el
index 2c807b0..e814c3a 100644
--- a/lisp/printing.el
+++ b/lisp/printing.el
@@ -4627,21 +4627,21 @@ bottom."
 
 
 ;;;###autoload
-(defun pr-customize (&rest ignore)
+(defun pr-customize (&rest _ignore)
   "Customization of the `printing' group."
   (interactive)
   (customize-group 'printing))
 
 
 ;;;###autoload
-(defun lpr-customize (&rest ignore)
+(defun lpr-customize (&rest _ignore)
   "Customization of the `lpr' group."
   (interactive)
   (customize-group 'lpr))
 
 
 ;;;###autoload
-(defun pr-help (&rest ignore)
+(defun pr-help (&rest _ignore)
   "Help for the printing package."
   (interactive)
   (pr-show-setup pr-help-message "*Printing Help*"))
@@ -4675,21 +4675,21 @@ bottom."
 
 
 ;;;###autoload
-(defun pr-show-ps-setup (&rest ignore)
+(defun pr-show-ps-setup (&rest _ignore)
   "Show current ps-print settings."
   (interactive)
   (pr-show-setup (ps-setup) "*PS Setup*"))
 
 
 ;;;###autoload
-(defun pr-show-pr-setup (&rest ignore)
+(defun pr-show-pr-setup (&rest _ignore)
   "Show current printing settings."
   (interactive)
   (pr-show-setup (pr-setup) "*PR Setup*"))
 
 
 ;;;###autoload
-(defun pr-show-lpr-setup (&rest ignore)
+(defun pr-show-lpr-setup (&rest _ignore)
   "Show current lpr settings."
   (interactive)
   (pr-show-setup (lpr-setup) "*LPR Setup*"))
@@ -6125,7 +6125,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
        (pr-insert-checkbox
 	"\n               "
 	'pr-i-region
-	#'(lambda (widget &rest ignore)
+	#'(lambda (widget &rest _ignore)
 	    (let ((region-p (pr-interface-save
 			     (ps-mark-active-p))))
 	      (cond ((null (widget-value widget)) ; widget is nil
@@ -6146,7 +6146,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
        (pr-insert-checkbox
 	"    "
 	'pr-i-mode
-	#'(lambda (widget &rest ignore)
+	#'(lambda (widget &rest _ignore)
 	    (let ((mode-p (pr-interface-save
 			   (pr-mode-alist-p))))
 	      (cond
@@ -6182,7 +6182,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
   (widget-create 'regexp
 		 :size 58
 		 :format "\n      File Regexp : %v\n"
-		 :notify (lambda (widget &rest ignore)
+		 :notify (lambda (widget &rest _ignore)
 			   (setq pr-i-regexp (widget-value widget)))
 		 pr-i-regexp)
   ;;    1b. Directory: List Directory Entry
@@ -6222,7 +6222,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
        (pr-insert-checkbox
 	"    "
 	'pr-i-despool
-	#'(lambda (widget &rest ignore)
+	#'(lambda (widget &rest _ignore)
 	    (if pr-spool-p
 		(setq pr-i-despool (not pr-i-despool))
 	      (ding)
@@ -6259,7 +6259,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
    'integer
    :size 3
    :format "\n  N-Up : %v"
-   :notify (lambda (widget &rest ignore)
+   :notify (lambda (widget &rest _ignore)
 	     (let ((value (if (string= (widget-apply widget :value-get) "")
 			      0
 			    (widget-value widget))))
@@ -6288,7 +6288,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
   ;; 4. Settings:
   ;; 4. Settings: Landscape             Auto Region    Verbose
   (pr-insert-checkbox "\n\n  " 'ps-landscape-mode
-		      #'(lambda (&rest ignore)
+		      #'(lambda (&rest _ignore)
 			  (setq ps-landscape-mode (not ps-landscape-mode)
 				pr-file-landscape ps-landscape-mode))
 		      " Landscape             ")
@@ -6310,7 +6310,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
   (pr-insert-toggle 'ps-zebra-stripes " Zebra Stripes")
   (pr-insert-checkbox "         "
 		      'pr-spool-p
-		      #'(lambda (&rest ignore)
+		      #'(lambda (&rest _ignore)
 			  (setq pr-spool-p (not pr-spool-p))
 			  (unless pr-spool-p
 			    (setq pr-i-despool nil)
@@ -6320,7 +6320,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
   ;; 4. Settings: Duplex                Print with faces
   (pr-insert-checkbox "\n  "
 		      'ps-spool-duplex
-		      #'(lambda (&rest ignore)
+		      #'(lambda (&rest _ignore)
 			  (setq ps-spool-duplex (not ps-spool-duplex)
 				pr-file-duplex  ps-spool-duplex))
 		      " Duplex                ")
@@ -6329,7 +6329,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
   ;; 4. Settings: Tumble                Print via Ghostscript
   (pr-insert-checkbox "\n  "
 		      'ps-spool-tumble
-		      #'(lambda (&rest ignore)
+		      #'(lambda (&rest _ignore)
 			  (setq ps-spool-tumble (not ps-spool-tumble)
 				pr-file-tumble  ps-spool-tumble))
 		      " Tumble                ")
@@ -6352,7 +6352,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
   ;; 5. Customize:
   (pr-insert-italic "\n\nCustomize     :   " 2 11)
   (pr-insert-button 'pr-customize "printing" "   ")
-  (pr-insert-button #'(lambda (&rest ignore) (ps-print-customize))
+  (pr-insert-button #'(lambda (&rest _ignore) (ps-print-customize))
 		    "ps-print" "   ")
   (pr-insert-button 'lpr-customize "lpr"))
 
@@ -6374,7 +6374,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
   (pr-insert-button 'pr-kill-help "Kill All Printing Help Buffer"))
 
 
-(defun pr-kill-help (&rest ignore)
+(defun pr-kill-help (&rest _ignore)
   "Kill all printing help buffer."
   (interactive)
   (let ((help '("*Printing Interface Help*" "*Printing Help*"
@@ -6388,20 +6388,20 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
   (recenter (- (window-height) 2)))
 
 
-(defun pr-interface-quit (&rest ignore)
+(defun pr-interface-quit (&rest _ignore)
   "Kill the printing buffer interface and quit."
   (interactive)
   (kill-buffer pr-buffer-name)
   (set-window-configuration pr-i-window-configuration))
 
 
-(defun pr-interface-help (&rest ignore)
+(defun pr-interface-help (&rest _ignore)
   "printing buffer interface help."
   (interactive)
   (pr-show-setup pr-interface-help-message "*Printing Interface Help*"))
 
 
-(defun pr-interface-txt-print (&rest ignore)
+(defun pr-interface-txt-print (&rest _ignore)
   "Print using lpr package."
   (interactive)
   (condition-case data
@@ -6433,7 +6433,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
      (message "%s" (error-message-string data)))))
 
 
-(defun pr-interface-printify (&rest ignore)
+(defun pr-interface-printify (&rest _ignore)
   "Printify a buffer."
   (interactive)
   (condition-case data
@@ -6458,7 +6458,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
      (message "%s" (error-message-string data)))))
 
 
-(defun pr-interface-ps-print (&rest ignore)
+(defun pr-interface-ps-print (&rest _ignore)
   "Print using ps-print package."
   (interactive)
   (pr-interface-ps 'pr-despool-ps-print 'pr-ps-directory-ps-print
@@ -6467,7 +6467,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
 		   'pr-ps-buffer-ps-print))
 
 
-(defun pr-interface-preview (&rest ignore)
+(defun pr-interface-preview (&rest _ignore)
   "Preview a PostScript file."
   (interactive)
   (pr-interface-ps 'pr-despool-preview 'pr-ps-directory-preview
@@ -6548,7 +6548,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
       (error "Please specify be a readable directory")))
 
 
-(defun pr-interface-directory (widget &rest ignore)
+(defun pr-interface-directory (widget &rest _ignore)
   (and pr-buffer-verbose
        (message "You can use M-TAB or ESC TAB for file completion"))
   (let ((dir (widget-value widget)))
@@ -6557,7 +6557,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
 	 (setq pr-i-directory dir))))
 
 
-(defun pr-interface-infile (widget &rest ignore)
+(defun pr-interface-infile (widget &rest _ignore)
   (and pr-buffer-verbose
        (message "You can use M-TAB or ESC TAB for file completion"))
   (let ((file (widget-value widget)))
@@ -6566,7 +6566,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
 	 (setq pr-i-ps-file file))))
 
 
-(defun pr-interface-outfile (widget &rest ignore)
+(defun pr-interface-outfile (widget &rest _ignore)
   (setq pr-i-answer-yes nil)
   (and pr-buffer-verbose
        (message "You can use M-TAB or ESC TAB for file completion"))
@@ -6602,7 +6602,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
 
 (defun pr-insert-toggle (var-sym label)
   (widget-create 'checkbox
-		 :notify `(lambda (&rest ignore)
+		 :notify `(lambda (&rest _ignore)
 			    (setq ,var-sym (not ,var-sym)))
 		 (symbol-value var-sym))
   (widget-insert label))
@@ -6623,7 +6623,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
 			:format "%v"
 			:inline t
 			:value ,var-sym
-			:notify (lambda (widget &rest ignore)
+			:notify (lambda (widget &rest _ignore)
 				  (setq ,var-sym (widget-value widget))
 				  ,@body)
 			:void '(choice-item :format "%[%t%]"
@@ -6639,7 +6639,7 @@ COMMAND.exe, COMMAND.bat and COMMAND.com in this order."
 		     'radio-button
 		     :format "  %[%v%]"
 		     :value (eq ,var-sym (quote ,sym))
-		     :notify (lambda (&rest ignore)
+		     :notify (lambda (&rest _ignore)
 			       (setq ,var-sym (quote ,sym))
 			       (pr-update-radio-button (quote ,var-sym)))))))
     (put var-sym 'pr-widget-list (cons (cons wid sym) wid-list))))
diff --git a/lisp/vc/ediff-diff.el b/lisp/vc/ediff-diff.el
index 3e64250..3f0ef3a 100644
--- a/lisp/vc/ediff-diff.el
+++ b/lisp/vc/ediff-diff.el
@@ -211,7 +211,7 @@ one optional arguments, diff-number to refine.")
 ;; ediff-setup-diff-regions is called via a funcall to
 ;; ediff-setup-diff-regions-function, which can also have the value
 ;; ediff-setup-diff-regions3, which takes 4 arguments.
-(defun ediff-setup-diff-regions (file-A file-B file-C)
+(defun ediff-setup-diff-regions (file-A file-B _file-C)
   ;; looking for '-c', '-i', '-u', or 'c', 'i', 'u' among clustered non-long options
   (if (string-match "^-[ciu]\\| -[ciu]\\|\\(^\\| \\)-[^- ]+[ciu]"
 		    ediff-diff-options)
@@ -1223,7 +1223,7 @@ delimiter regions"))
 ;; like shell-command-sentinel but doesn't print an exit status message
 ;; we do this because diff always exits with status 1, if diffs are found
 ;; so shell-command-sentinel displays a confusing message to the user
-(defun ediff-process-sentinel (process signal)
+(defun ediff-process-sentinel (process _signal)
   (if (and (memq (process-status process) '(exit signal))
            (buffer-name (process-buffer process)))
       (progn
diff --git a/lisp/vc/ediff-init.el b/lisp/vc/ediff-init.el
index 0757759..c9f3583 100644
--- a/lisp/vc/ediff-init.el
+++ b/lisp/vc/ediff-init.el
@@ -810,7 +810,7 @@ TYPE-OF-EMACS is either 'xemacs or 'emacs."
   (ediff-overlay-put extent 'face face)
   (ediff-overlay-put extent 'help-echo 'ediff-region-help-echo))
 
-(defun ediff-region-help-echo (extent-or-window &optional overlay point)
+(defun ediff-region-help-echo (extent-or-window &optional overlay _point)
   (unless overlay
     (setq overlay extent-or-window))
   (let ((is-current (ediff-overlay-get overlay 'ediff))
@@ -1768,7 +1768,7 @@ Unless optional argument INPLACE is non-nil, return a new string."
   (or n (setq n ediff-current-difference))
   (and (>= n 0) (< n ediff-number-of-differences)))
 
-(defsubst ediff-show-all-diffs (n)
+(defsubst ediff-show-all-diffs (_n)
   "Don't skip difference regions."
   nil)
 
diff --git a/lisp/vc/ediff-merg.el b/lisp/vc/ediff-merg.el
index a1f4d4f..a319d8d 100644
--- a/lisp/vc/ediff-merg.el
+++ b/lisp/vc/ediff-merg.el
@@ -92,6 +92,8 @@ Buffer B."
   )
 (make-variable-buffer-local 'ediff-skip-merge-regions-that-differ-from-default)
 
+(defvar state-of-merge)			; dynamic var
+
 ;; check if there is no clash between the ancestor and one of the variants.
 ;; if it is not a merge job then return true
 (defun ediff-merge-region-is-non-clash (n)
@@ -351,8 +353,6 @@ Combining is done according to the specifications in variable
 	(reverse delim-regs-list)
 	)))
 
-(defvar state-of-merge)			; dynamic var
-
 ;; Check if the non-preferred merge has been modified since originally set.
 ;; This affects only the regions that are marked as default-A/B or combined.
 ;; If PREFERS-TOO is non-nil, then look at the regions marked as prefers-A/B as
diff --git a/lisp/vc/ediff-mult.el b/lisp/vc/ediff-mult.el
index 58e1081..55e9465 100644
--- a/lisp/vc/ediff-mult.el
+++ b/lisp/vc/ediff-mult.el
@@ -1115,7 +1115,7 @@ behavior."
     (setq overl
 	  (if (featurep 'xemacs)
 	      (map-extents
-	       (lambda (ext maparg)
+	       (lambda (ext _maparg)
 		 (if (and
 		      (ediff-overlay-get ext 'ediff-meta-info)
 		      (eq (ediff-overlay-get ext 'ediff-meta-session-number)
@@ -1444,7 +1444,7 @@ Useful commands:
 
 
 ;; argument is ignored
-(defun ediff-redraw-registry-buffer (&optional ignore)
+(defun ediff-redraw-registry-buffer (&optional _ignore)
   (ediff-with-current-buffer ediff-registry-buffer
     (let ((point (point))
 	  elt bufAname bufBname bufCname cur-diff total-diffs pt
diff --git a/lisp/vc/ediff-util.el b/lisp/vc/ediff-util.el
index 2f2c71a..fe791f6 100644
--- a/lisp/vc/ediff-util.el
+++ b/lisp/vc/ediff-util.el
@@ -40,7 +40,7 @@
 (defvar ediff-after-quit-hook-internal nil)
 
 (eval-and-compile
-  (unless (fboundp 'declare-function) (defmacro declare-function (&rest  r))))
+  (unless (fboundp 'declare-function) (defmacro declare-function (&rest  _r))))
 
 ;; end pacifier
 
@@ -1602,7 +1602,7 @@ the width of the A/B/C windows."
 ;;BEG, END show the region to be positioned.
 ;;JOB-NAME holds ediff-job-name.  The ediff-windows job positions regions
 ;;differently.
-(defun ediff-position-region (beg end pos job-name)
+(defun ediff-position-region (beg end pos _job-name)
   (if (> end (point-max))
       (setq end (point-max)))
   (if ediff-windows-job
@@ -1685,7 +1685,7 @@ the width of the A/B/C windows."
 			    'ediff-get-lines-to-region-start)
 			   ((eq op 'scroll-up)
 			    'ediff-get-lines-to-region-end)
-			   (t (lambda (a b c) 0))))
+			   (t (lambda (_a _b _c) 0))))
 	       (max-lines (max (funcall func 'A n ctl-buf)
 			       (funcall func 'B n ctl-buf)
 			       (if (ediff-buffer-live-p ediff-buffer-C)
diff --git a/lisp/vc/ediff-wind.el b/lisp/vc/ediff-wind.el
index 5a14c19..785535b 100644
--- a/lisp/vc/ediff-wind.el
+++ b/lisp/vc/ediff-wind.el
@@ -40,7 +40,7 @@
 
 ;; declare-function does not exist in XEmacs
 (eval-and-compile
-  (unless (fboundp 'declare-function) (defmacro declare-function (&rest  r))))
+  (unless (fboundp 'declare-function) (defmacro declare-function (&rest  _r))))
 
 (require 'ediff-init)
 (require 'ediff-help)
@@ -280,7 +280,7 @@ into icons, regardless of the window manager."
 
 ;;; Functions
 
-(defun ediff-get-window-by-clicking (wind prev-wind wind-number)
+(defun ediff-get-window-by-clicking (_wind _prev-wind wind-number)
   (let (event)
     (message
      "Select windows by clicking.  Please click on Window %d " wind-number)
@@ -289,9 +289,9 @@ into icons, regardless of the window manager."
 	  (beep 1))
       (message "Please click on Window %d " wind-number))
     (ediff-read-event) ; discard event
-    (setq wind (if (featurep 'xemacs)
-		   (event-window event)
-		 (posn-window (event-start event))))))
+    (if (featurep 'xemacs)
+        (event-window event)
+      (posn-window (event-start event)))))
 
 
 ;; Select the lowest window on the frame.
diff --git a/lisp/vc/ediff.el b/lisp/vc/ediff.el
index d35c3e5..9ab5925 100644
--- a/lisp/vc/ediff.el
+++ b/lisp/vc/ediff.el
@@ -114,7 +114,7 @@
 
 ;; Compiler pacifier
 (eval-and-compile
-  (unless (fboundp 'declare-function) (defmacro declare-function (&rest  r))))
+  (unless (fboundp 'declare-function) (defmacro declare-function (&rest  _r))))
 
 (require 'ediff-util)
 ;; end pacifier
diff --git a/lisp/vc/emerge.el b/lisp/vc/emerge.el
index 8d0e8ef..af13665 100644
--- a/lisp/vc/emerge.el
+++ b/lisp/vc/emerge.el
@@ -849,7 +849,7 @@ This is *not* a user option, since Emerge uses it for its own processing.")
 ;;; Functions to start Emerge on files
 
 ;;;###autoload
-(defun emerge-files (arg file-A file-B file-out &optional startup-hooks
+(defun emerge-files (_arg file-A file-B file-out &optional startup-hooks
 		     quit-hooks)
   "Run Emerge on two files."
   (interactive
@@ -869,7 +869,7 @@ This is *not* a user option, since Emerge uses it for its own processing.")
    file-out))
 
 ;;;###autoload
-(defun emerge-files-with-ancestor (arg file-A file-B file-ancestor file-out
+(defun emerge-files-with-ancestor (_arg file-A file-B file-ancestor file-out
 				   &optional startup-hooks quit-hooks)
   "Run Emerge on two files, giving another file as the ancestor."
   (interactive
@@ -1063,7 +1063,7 @@ This is *not* a user option, since Emerge uses it for its own processing.")
        quit-hooks)))
 
 (defun emerge-revisions-internal (file revision-A revision-B &optional
-                                  startup-hooks quit-hooks output-file)
+                                  startup-hooks quit-hooks _output-file)
   (let ((buffer-A (get-buffer-create (format "%s,%s" file revision-A)))
 	(buffer-B (get-buffer-create (format "%s,%s" file revision-B)))
 	(emerge-file-A (emerge-make-temp-file "A"))
diff --git a/lisp/vc/pcvs.el b/lisp/vc/pcvs.el
index fb91185..914eef4 100644
--- a/lisp/vc/pcvs.el
+++ b/lisp/vc/pcvs.el
@@ -648,7 +648,7 @@ If non-nil, NEW means to create a new buffer no matter what."
 			     done))))
 
 
-(defun cvs-sentinel (proc msg)
+(defun cvs-sentinel (proc _msg)
   "Sentinel for the cvs update process.
 This is responsible for parsing the output from the cvs update when
 it is finished."
@@ -981,7 +981,7 @@ The files are stored to DIR."
 ;;;;
 
 (defun-cvs-mode (cvs-mode-revert-buffer . SIMPLE)
-                (&optional ignore-auto noconfirm)
+                (&optional _ignore-auto _noconfirm)
   "Rerun `cvs-examine' on the current directory with the default flags."
   (interactive)
   (cvs-examine default-directory t))
@@ -995,7 +995,7 @@ If in a *cvs* buffer, don't prompt unless a prefix argument is given."
     (read-directory-name prompt nil default-directory nil)))
 
 ;;;###autoload
-(defun cvs-quickdir (dir &optional flags noshow)
+(defun cvs-quickdir (dir &optional _flags noshow)
   "Open a *cvs* buffer on DIR without running cvs.
 With a prefix argument, prompt for a directory to use.
 A prefix arg >8 (ex: \\[universal-argument] \\[universal-argument]),
diff --git a/lisp/vc/vc-annotate.el b/lisp/vc/vc-annotate.el
index 8af4887..e3f7f39 100644
--- a/lisp/vc/vc-annotate.el
+++ b/lisp/vc/vc-annotate.el
@@ -630,7 +630,7 @@ or OFFSET if present."
 	      (vc-call-backend vc-annotate-backend 'annotate-current-time))
 	  next-time))))
 
-(defun vc-default-annotate-current-time (backend)
+(defun vc-default-annotate-current-time (_backend)
   "Return the current time, encoded as fractional days."
   (vc-annotate-convert-time (current-time)))
 

Re: Lexical byte-compilation warnings cleanup

[Juanma Barranquero]

On Tue, Aug 20, 2013 at 1:33 AM, Daniel Hackney <dan@haxney.org> wrote:

>  - `condition-case': The byte-compiler doesn't recognize the handler

>    Byte-compiling that with `byte-compile-force-lexical-warnings' set
>    to `t' warns of "Unused lexical argument `data'". Switching lines 1
>    and 2 gives the warning "the function `end-of-file' is not known to
>    be defined." Removing line 1 makes it compile without errors. It
>    would seem that the byte-compiler is not treating `condition-case'
>    as specially as it should.

IIRC, each error form is compiled as a lambda that gets the error
variable as an argument. So if some of them do not use the arg, you
get the lexical-binding warning. It is definitely something that
should be improved (the lexical-binding implementation of this
feature, I mean).

>  - I'm seeing a lot of "Argument foo is not a lexical variable", such as
>    in vc/ediff-diff.el:532:56. What does this mean and is this something
>    I should "fix"? In some cases, this is because `foo' is defined with
>    `defvar' but also used as a function argument. What should be done in
>    this case? For example, here is the definition of
>    `emerge-remote-exit':

I don't think there's a general answer. It depends on how the argument
/ dynamic variable are used.

>  - In `emerge-revisions-with-ancestor', the variable `cmd' is let-bound,
>    but does not appear to be used. Could it be safely removed?
>
>  - Similarly, `cvs-fileinfo<' binds variables `subtypea' and `subtypeb'
>    but never uses them. Can they be removed?
>
>  - Superfluously-bound variables: In, for example,
>    `ange-ftp-file-attributes', the variables `host', `user', and `name'
>    are bound but never used. Should these be prefixed with an
>    underscore (to make the byte compiler shut up) or removed altogether
>    (since they aren't actually used)? It seems like they should just be
>    removed, but I don't want to trample on something needed.

Same here. The problem is that in some cases, these variables are
bound to be used in other functions called from the ones that let-bind
them. In a few cases, they are *not* used, but they are declared as
part of the user interface, expecting that the user will use
expressions with these variables that will be eval'ed and expect the
variables to be bound  (I think Calc does this, for example).

So, you have to take a hard look, try to find whether these arguments
or let-bindings are used dynamically or not. If not, they can usually
be fixed as you suggest. If they are, in some cases they could also be
fixed (the defvar vs. function arg that you point above), if the
variable name is not part of the defined interface... It's all a big
PITA, and very easy to make a mistake, truth be told.

   J

Re: Lexical byte-compilation warnings cleanup

[Glenn Morris]

Daniel Hackney wrote:

>  - `condition-case': The byte-compiler doesn't recognize the handler
>    forms and so gives warnings about unused lexical arguments.

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15103

Re: Lexical byte-compilation warnings cleanup

[Stefan Monnier]

>  - I'm seeing a lot of "Argument foo is not a lexical variable", such as
>    in vc/ediff-diff.el:532:56. What does this mean and is this something
>    I should "fix"?

It means that the function uses an argument whose name refers to
a dynamically-scoped variable.  This is not allowed in lexically-scoped
code, so you need to rename either the dynbind var or the
function's argument.

>    Note that `emerge-exit-func' is a `defvar'ed variable. Should it be
>    replaced with something like this:

>    #+BEGIN_SRC emacs-lisp
>      (defun emerge-remote-exit (file-out exit-func)
>        (let ((emerge-exit-func exit-func))
>          (emerge-write-and-delete file-out)
>          (kill-buffer emerge-merge-buffer)
>          (funcall emerge-exit-func (if emerge-prefix-argument 1 0))))
>    #+END_SRC

Yes.

>  - In `emerge-revisions-with-ancestor', the variable `cmd' is let-bound,
>    but does not appear to be used.  Could it be safely removed?

Depends on the rest of the code.  Usually, if there's no assignment
or reference to a free variable named "cmd" elsewhere, there's a good
chance that such variables that are "let-bound but not used locally" are
indeed just unused and can be removed.


        Stefan

RE: Lexical byte-compilation warnings cleanup

[Drew Adams]

> It means that the function uses an argument whose name refers to
> a dynamically-scoped variable.  This is not allowed in lexically-scoped
> code, so you need to rename either the dynbind var or the
> function's argument.

Excuse me for not following the thread and perhaps not understanding what
you say here.  Are you saying that if `lexical-binding' is non-nil then
a function parameter whose name is the same as a dynamically scoped
variable is "not allowed" or does not refer to that variable?

That would be counter to how Common Lisp works, and I thought (and I hope)
that our aim was (is) to use the way Common Lisp marries lexical and
dynamic binding as our model.

In Common Lisp, a variable declared to be "special" (dynamically scoped)
can be used as a formal parameter to a function.  See, for example,

http://www.ai.mit.edu/projects/iiip/doc/CommonLISP/HyperSpec/Body/dec_special.html

Again, sorry for the noise if I misunderstand.  If not, please clarify.
Thx.

Re: Lexical byte-compilation warnings cleanup

[Stefan Monnier]

> Excuse me for not following the thread and perhaps not understanding what
> you say here.  Are you saying that if `lexical-binding' is non-nil then
> a function parameter whose name is the same as a dynamically scoped
> variable is "not allowed" or does not refer to that variable?

Indeed, it does not refer to the dynamically bound variable.

> That would be counter to how Common Lisp works, and I thought (and I hope)
> that our aim was (is) to use the way Common Lisp marries lexical and
> dynamic binding as our model.

Elisp is not Common-Lisp.


        Stefan

RE: Lexical byte-compilation warnings cleanup

[Drew Adams]

> > Excuse me for not following the thread and perhaps not understanding what
> > you say here.  Are you saying that if `lexical-binding' is non-nil then
> > a function parameter whose name is the same as a dynamically scoped
> > variable is "not allowed" or does not refer to that variable?
> 
> Indeed, it does not refer to the dynamically bound variable.

Why is that?  Will this be fixed, or is this the intended design?

> > That would be counter to how Common Lisp works, and I thought (and I hope)
> > that our aim was (is) to use the way Common Lisp marries lexical and
> > dynamic binding as our model.
> 
> Elisp is not Common-Lisp.

That's obvious.  No one will argue the contrary.

But is there a reason not to follow the CL design in general wrt the
cohabitation of lexical and dynamic binding?  Even if things are currently
a work in progress, is that the direction you intend to head, or are you
aiming elsewhere?

Is there a description somewhere of where the design is headed in this
regard?  Is there a spec or a proposal?  What are the intentions?

There was a lot of discussion behind the CL design.  What are the proposals
here and their supporting arguments?

Re: Lexical byte-compilation warnings cleanup

[Stefan Monnier]

>> Indeed, it does not refer to the dynamically bound variable.
> Why is that?  Will this be fixed, or is this the intended design?

Intended design.

> Even if things are currently a work in progress, is that the direction
> you intend to head, or are you aiming elsewhere?

I have never aimed to do what CL does.  As language designers, we just
take idea from here and there.

Elisp's design is different from CL's design partly for historical
reasons but also for technical reasons: Elisp needs to be interpretable
efficiently, for example.


        Stefan

RE: Lexical byte-compilation warnings cleanup

[Drew Adams]

> >> Indeed, it does not refer to the dynamically bound variable.
> > Why is that?  Will this be fixed, or is this the intended design?
    ^^^^^^^^^^^^
> 
> Intended design.

And the intention is?  The design is?  The reason is?

> > Even if things are currently a work in progress, is that the direction
> > you intend to head, or are you aiming elsewhere?
> 
> I have never aimed to do what CL does.  As language designers, we just
> take idea from here and there.
> 
> Elisp's design is different from CL's design partly for historical
> reasons but also for technical reasons: Elisp needs to be interpretable
> efficiently, for example.

OK, and how does the need for efficient interpretation enter into the
design decision that a function parameter whose name is the same as a
dynamically scoped variable is "not allowed" or does not refer to that
variable?

RE: Lexical byte-compilation warnings cleanup

[Stephen J. Turnbull]

Drew Adams writes:
 > Stefan wrote:
 >> Drew wrote:
 >>> Stefan wrote:

 >>>> Indeed, it does not refer to the dynamically bound variable.
 >>> Why is that?  Will this be fixed, or is this the intended design?
 >   ^^^^^^^^^^^^

 >> Intended design.

 > And the intention is?  The design is?  The reason is?

Give it up, Drew.  RMS has never been one to prefer "a foolish
consistency" (ie, adherence to abstract principles) where consistency
collides with his intuition or hacking convenience, and that same
reliance on intuition appeals to current maintainers as well:

 > > As language designers, we just take idea from here and there.

I think that is the last word on Elisp design principles.  Abstractly
I don't like it, myself, but surely you don't deny that it has proven
to be a workable strategy.

Nevertheless there is hope for those who prefer care-full design:
Guilemacs!

Steve

RE: Lexical byte-compilation warnings cleanup

[Drew Adams]

>  >>>> Indeed, it does not refer to the dynamically bound variable.
>  >>> Why is that? Will this be fixed, or is this the intended design?
>  >> Intended design.
>  > And the intention is?  The design is?  The reason is?
> 
> Give it up, Drew.  RMS has never been one to prefer "a foolish
> consistency"

No one has argued for foolish consistency.  Or even for consistency
in this context.

> (ie, adherence to abstract principles)

Or for adherence to any abstract principles. 

> where consistency collides with his intuition or hacking convenience,
> and that same reliance on intuition appeals to current maintainers
> as well:

Nor has anyone argued against using intuition.  That can sometimes
be helpful.

But FWIW I don't think that RMS was particularly one to shy from
presenting his design choices explicitly and giving reasons for them.

Anyway, that was then and this is now - I don't see much point in
comparing with RMS here, either wrt the design or the process.  It's
not clear that RMS would have moved so quickly and cleanly toward
integrating lexical scoping - and I'm glad that has been done.

(No, sorry for the passive voice - I'm glad that Stefan did that.)

But I would like to understand more (more than zero, at least) about
the design intentions in this regard, and the reasons.

Since there are 35 years of big brother Common Lisp's experience
mixing lexical and dynamic binding in the same language, not to
mention years of high-level discussion by Lisp implementors of all
stripes to come up with that design in the first place, I would think
that doing something quite different would prompt at least some
discussion of alternatives, pros & cons, etc.

That was admittedly a long time ago, and no doubt knowledge and
technique have progressed since then.  All the more reason why I
would like to know something about what is behind this new approach.

Aren't you curious at all why this difference wrt function parameters?
That possibility never even occurred to me.  I'm sure there are some
reasons for the choices made, but they don't seem to be forthcoming.

>  > > As language designers, we just take idea from here and there.
> 
> I think that is the last word on Elisp design principles.  Abstractly
> I don't like it, myself, but surely you don't deny that it has proven
> to be a workable strategy.

Again, one can take ideas from here and there - no problem with that.
But one takes this idea instead of that one for a reason.  And that's
what I'm asking about.

Is it a good thing or a bad thing for more people to have an idea
what the design is about and what motivates it?

> Nevertheless there is hope for those who prefer care-full design:
> Guilemacs!

For the record, Guilemacs is not my hope.  (Not that I'm sure what
you mean by that term.)

So far, the addition of lexical scoping to Emacs has not been far
from the way it is mixed in with dynamic binding in Common Lisp,
AFAICT.  Naively, I would hope for Emacs Lisp to become more similar
in this regard, not more different.

But I'm no expert on any of this.  Which is why I asked the question.
And that's all I've done: ask why.

I ask it again: why?  The answer so far is to enable efficient
interpretation.  Sounds like that might be a promising reason, but
so far that slogan means nothing to me.

Does it mean something particular to you?  Don't you want to know
more about what is intended, and why, wrt this design which is
presumably so important to Emacs and Emacs-Lisp programmers?

Re: Lexical byte-compilation warnings cleanup

[Stefan Monnier]

>> > And the intention is?  The design is?  The reason is?

Check the calling convention used by the new byte-code representation
(used for lexically scoped code) and you'll see that there's no support
for dynamic scoping in it.  Adding such support "natively" would
slow down the common case too much.

We could support dynamically-scoped args in lexically-scoped byte-code
by tweaking the byte-compiler, but then the byte-code that the
byte-compiler needs to generate is the same as the one you get with:

   (defun foo (lexarg)
     (let ((dynvar lexarg))
       ...))

So I prefer to make the extra cost of the separate `let' binding
explicit in the source code.  Especially since it has various beneficial
side-effects:
- simpler byte-compiler.
- slightly more efficient handling of function calls for
  lexically-scoped interpreted code (this is actually less beneficial
  than I thought at first, tho).
- IMNHO better style (I have always found it confusing when dynvars were
  bound as function arguments rather than by a let-binding).
- having a separate `let' often allows you to move it to a better place,
  and/or to add useful code between the function header and the let
  binding (such as the defvar that causes the variable to be
  dynamically bound).

BTW, I have never seen a dynamically-scoped argument used in
Common Lisp.


        Stefan

Re: Lexical byte-compilation warnings cleanup

[Dmitri Paduchikh]

Stefan Monnier wrote replying to Drew Adams:

>> Excuse me for not following the thread and perhaps not understanding what
>> you say here.  Are you saying that if `lexical-binding' is non-nil then
>> a function parameter whose name is the same as a dynamically scoped
>> variable is "not allowed" or does not refer to that variable?

> Indeed, it does not refer to the dynamically bound variable.

The warning seems to say opposite:

In toplevel form:
test.el:3:1:Warning: Argument load-path is not a lexical variable

This is for the following simple test case compiled by Emacs 24.3:

;;; -*- lexical-binding: t -*-

(defun my-test (load-path)
  (car load-path))

-- 
With best regards
Dmitri Paduchikh

Re: Lexical byte-compilation warnings cleanup

[Stefan Monnier]

> The warning seems to say opposite:

Yup.  The way I thought about it when I wrote it is that `load-path' is
a dynamically-scoped var, not a lexical var.  But indeed, here it will
(locally) be a lexical var, so the wording is rather poor.

Suggestions for better wording?


        Stefan

Re: Lexical byte-compilation warnings cleanup

[Barry OReilly]

[-- Attachment #1: Type: text/plain, Size: 373 bytes --]

>> The warning seems to say opposite:

> Yup. The way I thought about it when I wrote it is that `load-path'
> is a dynamically-scoped var, not a lexical var. But indeed, here it
> will (locally) be a lexical var, so the wording is rather poor.
>
> Suggestions for better wording?

How about:

  Warning: Lexically bound argument %s is already defined as dynamically
bound

[-- Attachment #2: Type: text/html, Size: 463 bytes --]

Re: Lexical byte-compilation warnings cleanup

[Dmitri Paduchikh]

Barry OReilly wrote:

>>> The warning seems to say opposite:

>> Yup. The way I thought about it when I wrote it is that `load-path'
>> is a dynamically-scoped var, not a lexical var. But indeed, here it
>> will (locally) be a lexical var, so the wording is rather poor.
>>
>> Suggestions for better wording?

> How about:

>   Warning: Lexically bound argument %s is already defined as dynamically
> bound

IMO, this does not explain what happens actually. I would rather prefer
such variant:

Warning: Lexically bound argument shadows dynamic variable %s

-- 
With best regards
Dmitri Paduchikh

Re: Lexical byte-compilation warnings cleanup

[Stefan Monnier]

> IMO, this does not explain what happens actually. I would rather prefer
> such variant:
> Warning: Lexically bound argument shadows dynamic variable %s

Better, indeed, thank you,


        Stefan


=== modified file 'lisp/emacs-lisp/cconv.el'
--- lisp/emacs-lisp/cconv.el	2013-06-14 02:31:28 +0000
+++ lisp/emacs-lisp/cconv.el	2013-08-22 20:41:05 +0000
@@ -552,7 +552,8 @@
       (cond
        ((byte-compile-not-lexical-var-p arg)
         (byte-compile-log-warning
-         (format "Argument %S is not a lexical variable" arg)))
+         (format "Lexical argument shadows the dynamic variable %S"
+                 arg)))
        ((eq ?& (aref (symbol-name arg) 0)) nil) ;Ignore &rest, &optional, ...
        (t (let ((varstruct (list arg nil nil nil nil)))
             (cl-pushnew arg byte-compile-lexical-variables)

Re: Lexical byte-compilation warnings cleanup

[Daniel Hackney]

Has anyone taken a look at my patch? It does some cleaning of some of
these things.

-- 
Daniel Hackney

Re: Lexical byte-compilation warnings cleanup

[Stefan Monnier]

>  - I'm seeing a lot of "Argument foo is not a lexical variable", such as
>    in vc/ediff-diff.el:532:56. What does this mean and is this something

I've noticed that there's a bug in byte-compile-force-lexical-warnings
which causes extra spurious warnings of this kind, so when using
byte-compile-force-lexical-warnings, double check those warnings (which
have been changed in the mean time to use a different formulation) since
they may just be spurious.

> -(defun dbus-string-to-byte-array (string)
> -  "Transforms STRING to list (:array :byte c1 :byte c2 ...).
> -STRING shall be UTF8 coded."
> -  (if (zerop (length string))
> +(defun dbus-string-to-byte-array (str)
> +  "Transforms STR to list (:array :byte c1 :byte c2 ...).
> +STR shall be UTF8 coded."
> +  (if (zerop (length str))

E.g. your renaming of a `string' argument to `str' is probably the
result of such a spurious warning.  So I didn't apply this hunk.

> -(defun eww-display-raw (charset)
> +(defun eww-display-raw (_charset)
>    (let ((data (buffer-substring (point) (point-max))))
>      (eww-setup-buffer)
>      (let ((inhibit-read-only t))

When unused function arguments can be removed (as is the case here),
it's generally a better choice than to mark them as unused.

I just installed your patch, thank you, and sorry for the delay,


        Stefan

Re: Lexical byte-compilation warnings cleanup

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 946 bytes --]

On 8/20/13 9:49 PM, Stefan Monnier wrote:
>>>> And the intention is?  The design is?  The reason is?
> 
> Check the calling convention used by the new byte-code representation
> (used for lexically scoped code) and you'll see that there's no support
> for dynamic scoping in it.  Adding such support "natively" would
> slow down the common case too much.

I think the current design is reasonable, and the byte-compiler warning
provides some safety. We do, however, need to provide some warnings in
cl-lib, because in some cases, we _can_ dynamically bind arguments.
Consider this code:

(defvar test (cons 1 2))
(defun* bar3 () (car test))
(defun* bar2 (&key test) (bar3))
(defun* bar1 () (bar2 :test (cons 3 4)))

When bar2 parses its argument list, is _dynamically_ binds test, which
test then picks up.  bar1 returns 3. If we change test from a keyword
argument to a regular argument, bar1 then returns 1, as expected.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]