From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: "Pascal J. Bourguignon" Newsgroups: gmane.emacs.help Subject: Re: Compilation warnings of ELisp seem wrong and misleading Date: Mon, 30 Mar 2015 19:08:26 +0200 Organization: Informatimago Message-ID: <87wq1ywgz9.fsf@kuiper.lan.informatimago.com> References: <87619iy2g9.fsf@kuiper.lan.informatimago.com> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: ger.gmane.org 1427736040 13266 80.91.229.3 (30 Mar 2015 17:20:40 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 30 Mar 2015 17:20:40 +0000 (UTC) To: help-gnu-emacs@gnu.org Original-X-From: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Mon Mar 30 19:20:33 2015 Return-path: Envelope-to: geh-help-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1YcdMK-0003rB-Si for geh-help-gnu-emacs@m.gmane.org; Mon, 30 Mar 2015 19:20:21 +0200 Original-Received: from localhost ([::1]:35312 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YcdMK-0007Av-0z for geh-help-gnu-emacs@m.gmane.org; Mon, 30 Mar 2015 13:20:20 -0400 Original-Path: usenet.stanford.edu!fu-berlin.de!uni-berlin.de!individual.net!not-for-mail Original-Newsgroups: gnu.emacs.help Original-Lines: 333 Original-X-Trace: individual.net a6f8a/77TT8ZqSnk+ITi6g3+aJqL+tyBDQuKVzR0V370wpQpGr Cancel-Lock: sha1:ZTgwNDdjMjY4MzkxY2YyYTU2MDY3Nzg4YmVmZjJmOTYyOWZkMjU2Nw== sha1:Div8poiieqJVxe2cuGHK8cWNnT0= Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwAQMAAABtzGvEAAAABlBMVEUAAAD///+l2Z/dAAAA oElEQVR4nK3OsRHCMAwF0O8YQufUNIQRGIAja9CxSA55AxZgFO4coMgYrEDDQZWPIlNAjwq9 033pbOBPtbXuB6PKNBn5gZkhGa86Z4x2wE67O+06WxGD/HCOGR0deY3f9Ijwwt7rNGNf6Oac l/GuZTF1wFGKiYYHKSFAkjIo1b6sCYS1sVmFhhhahKQssRjRT90ITWUk6vvK3RsPGs+M1RuR mV+hO/VvFAAAAABJRU5ErkJggg== X-Accept-Language: fr, es, en User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) Original-Xref: usenet.stanford.edu gnu.emacs.help:211175 X-BeenThere: help-gnu-emacs@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Users list for the GNU Emacs text editor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Original-Sender: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.help:103457 Archived-At: "Ludwig, Mark" writes: > "Pascal J. Bourguignon" wrote: > >> "Ludwig, Mark" writes: >> >> > Greetings, >> > >> > I've been using an ancient Emacs (19.29) on Solaris and finally >> > got around to installing a current one (24.4). >> > >> > I find my custom Elisp generates warnings that seem pretty stupid. >> > For example: >> > >> > emacs.el:255:10:Warning: reference to free variable `if' >> > emacs.el:219:8:Warning: reference to free variable `save-excursion' >> > emacs.el:331:41:Warning: reference to free variable `forward-char' >> > emacs.el:261:17:Warning: reference to free variable `insert' >> > emacs.el:261:17:Warning: reference to free variable `forward-sexp' >> > >> > Those are all valid functions. For example, here are lines 255-258: >> > >> > (if (not (= ans ?q)) >> > (progn >> > (goto-char found-start) >> > (delete-region found-start found-end))) >> > >> > This is inside a large-ish "let*" form (111 lines). > >> We cannot see either, because the meaning of a sexp is determined by its >> surrounding form, which you didn't provide. >> >> You probably have a parenthesis problem in your let* form which makes >> lisp interpret some parts as being variable names instead of operators. > > No, I know how to balance parentheses. I've been > programming Emacs Lisp for almost 30 years. I rely on Emacs > for its outstanding job of showing structural problems > through its automatic indentation (in all programming > languages, but certainly within its own). By all means, if > there's anything wrong with that (if ...) form, please > explain. > > For context, the "let*" is on line 239; the warning > (mentioned above as being on line 255 column 10) is > referring to the form after the comment "Make the > substitution." If you read the warnings carefully, you'll > note that it warns about two different things on line 261, > column 17. That line is the one with the comment, 'Now it's > "strlcpy" or whatever.' Clearly, the byte-compiler is > pretty confused.... No it is not confused. The warning are correct. However, it reports them at seemingly wrong places. First, the code you gave has indeed 5 free variables. It should not compile and it should not run! I had to add them to be able to compile it, in a surrounding let. Second, the code you gave is ill-formatted. How hard is it to put the closing parentheses on the same line (try to avoid ;-comments on the last expression before a closing parenthesis), and how hard is it to type M-q to have emacs indent your code correctly?!? Next, the warning reported: Compiling file /tmp/e.el at Mon Mar 30 18:38:30 2015 e.el:65:14:Warning: reference to free variable `if' e.el:90:33:Warning: reference to free variable `save-excursion' e.el:99:33:Warning: reference to free variable `forward-char' e.el:32:12:Warning: reference to free variable `insert' e.el:32:12:Warning: reference to free variable `forward-sexp' e.el:110:33:Warning: reference to free variable `c-indent-exp' correspond indeed to those symbols used in variable position, as cond expressions! Which can be clearly seen, when you have your code indented properly, starting from line 85 below: 1 (let ((from "from") 2 (to1 "T") 3 (to2 "t") 4 (found-start 0) 5 (found-end 2)) 6 (let* ((c1 (string-to-char to1)) 7 (c2 (string-to-char to2)) 8 (ans 0)) 9 10 ;; Get one of the first characters from the two substitutions 11 12 (while (and (not (= ans c1)) 13 (not (= ans c2)) 14 (not (= ans ?q))) ; Quit, don't substitute anything 15 (setq ans (read-char-exclusive 16 (concat "Replace " from " with " 17 to1 " (will truncate silently) or " 18 to2 " (dstat)? (" (char-to-string c1) "/" (char-to-string c2) "/q) ")))) 19 20 ;; Make the substitution 21 22 (if (not (= ans ?q)) 23 (progn 24 (goto-char found-start) 25 (delete-region found-start found-end))) 26 27 (cond ((= ans c1) 28 (insert to1) ; Now it's "strlcpy" or whatever 29 ) 30 31 ((= ans c2) 32 (insert "dstat = " to2) ; Now it's "dstat = nlsStrLCpy" or whatever 33 (back-to-indentation) 34 (setq ans 0) 35 (while (and (not (= ans ?d)) 36 (not (= ans ?e)) 37 (not (= ans ?n))) 38 (setq ans (read-char-exclusive "Use DSOK/EXIT/no form? "))) 39 40 (let ((if-pos (point))) 41 42 (cond ((= ans ?d) 43 (insert "if (DSOK) ") 44 (forward-sexp 3) ; After the closing parenthesis for the function call 45 ) 46 47 ((= ans ?e) 48 (insert "if (") 49 (forward-sexp 3) ; After the closing parenthesis for the function call 50 (insert ")") 51 (newline-and-indent) 52 (insert "goto EXIT") ; The semi-colon should end up after "EXIT" 53 (c-indent-command) ; Make the "goto" be correctly indented under the "if" 54 ) 55 56 ((= ans ?n) 57 ;; no form 58 )) 59 60 ;; Try to look ahead, in case the function call we just 61 ;; modified was a one-statement "block" in an "if," so 62 ;; we've just unintentionally modified the meaning of 63 ;; that "if" statement 64 65 (if (not (= ans ?n)) 66 (condition-case 67 nil 68 (progn 69 (forward-sexp 1) ; Skip over "else" if it's there 70 (forward-sexp -1) ; Back in front of "else" if it's there 71 (if (looking-at "else") 72 (progn 73 (goto-char if-pos) ; Go back to where we inserted "if" 74 75 (cond ((= ans ?d) 76 ;; Skip over: "if" "(DSOK)" "dstat" "= ..." "()" 77 (forward-sexp 5) 78 ) 79 80 ((= ans ?e) 81 ;; Skip over: 82 ;; "if" "(dstat = ...)" "goto" "EXIT" 83 (forward-sexp 4)) 84 85 (if (not (looking-at ";")) 86 (error "First expected ';' not: \"%s\"" 87 (buffer-substring (point) 88 (progn (end-of-line 1) 89 (point))))) 90 (save-excursion 91 (goto-char if-pos) ; Go back to where we inserted "if" 92 (insert "{ 93 ")) 94 (if (not (looking-at ";")) 95 (error "Second expected ';' not: \"%s\"" 96 (buffer-substring (point) 97 (progn (end-of-line 1) 98 (point))))) 99 (forward-char 1) 100 101 ;; We should be looking at the end of line 102 (if (not (looking-at "\\s-*$")) 103 (error "Second expected end of line, not: \"%s\"" 104 (buffer-substring (point) 105 (progn (end-of-line 1) 106 (point))))) 107 (insert " 108 }") 109 (forward-sexp -1) 110 (c-indent-exp) 111 )))) 112 (error nil) ;hand-rolled (ignore-errors ...) 113 ))))))) You may indeed report an emacs bug, that the warnings refer the first occurences of the symbol, instead of the erroneous occurences. Finally, you should have expressions so long. You should write short functions, and use them in shorter forms. Your code should be replaced with a single simple function: (defun do-something (from to1 to2 found-start found-end) (let* ((c1 (string-to-char to1)) (c2 (string-to-char to2)) (ans (get-one-of-the-first-characters-from-the-two-substitutions from to1 to2 c1 c2))) (unless (= ans ?q) (make-the-substitution ans from to1 to2 c1 c2 found-start found-end)))) ;; which uses the two simple and clear functions: (defun get-one-of-the-first-characters-from-the-two-substitutions (from to1 to2 c1 c2) (let ((ans 0)) (while (and (/= ans c1) (/= ans c2) (/= ans ?q)) ; Quit, don't substitute anything (setq ans (read-char-exclusive (concat "Replace " from " with " to1 " (will truncate silently) or " to2 " (dstat)? (" (char-to-string c1) "/" (char-to-string c2) "/q) ")))) ans)) (defun make-the-substitution (ans from to1 to2 c1 c2 found-start found-end) (assert (/= ans ?q)) (goto-char found-start) (delete-region found-start found-end) (cond ((= ans c1) (insert to1)) ((= ans c2) (insert-dstat ans from to2 c2 found-start found-end)))) ;; Also notice how they can very easily be tested alone: ;; (get-one-of-the-first-characters-from-the-two-substitutions "from" "T" "t" ?T ?t) ;; (make-the-substitution ?T "from" "T" "t" ?T ?t (point) (point)) ;; With make-the-substitution itself using the following functions: (defun get-dstat-choice () (let ((ans 0)) (while (and (/= ans ?d) (/= ans ?e) (/= ans ?n)) (setq ans (read-char-exclusive "Use DSOK/EXIT/no form? "))) ans)) ;; again, very easily testable: (get-dstat-choice) (defun insert-dstat (ans from to2 c2 found-start found-end) (insert "dstat = " to2) ; Now it's "dstat = nlsStrLCpy" or whatever (back-to-indentation) (let ((ans (get-dstat-choice)) (if-pos (point))) (unless (= ans ?n) (case ans (?d (insert "if (DSOK) ") (forward-sexp 3)) (?e (insert "if (") (forward-sexp 3) ; After the closing parenthesis for the function call (insert ")") (newline-and-indent) (insert "goto EXIT") ; The semi-colon should end up after "EXIT" (c-indent-command))) ;; Try to look ahead, in case the function call we just ;; modified was a one-statement "block" in an "if," so ;; we've just unintentionally modified the meaning of ;; that "if" statement (forward-sexp 1) ; Skip over "else" if it's there (forward-sexp -1) ; Back in front of "else" if it's there (when (looking-at "else") (goto-char if-pos) ; Go back to where we inserted "if" (case ans (?d ;; Skip over: "if" "(DSOK)" "dstat" "= ..." "()" (forward-sexp 5)) (?e ;; Skip over: ;; "if" "(dstat = ...)" "goto" "EXIT" (forward-sexp 4)) (otherwise ;; perhaps you didn't mean otherwise, but end of ;; the case? You'll have to debug it here. (unless (looking-at ";") (error "First expected ';' not: \"%s\"" (buffer-substring (point) (progn (end-of-line 1) (point))))) (save-excursion (goto-char if-pos) ; Go back to where we inserted "if" (insert "{\n")) (unless (looking-at ";") (error "Second expected ';' not: \"%s\"" (buffer-substring (point) (progn (end-of-line 1) (point))))) (forward-char 1) ;; We should be looking at the end of line (unless (looking-at "\\s-*$") (error "Second expected end of line, not: \"%s\"" (buffer-substring (point) (progn (end-of-line 1) (point))))) (insert "\n}") (forward-sexp -1) (c-indent-exp))))))) ;; you an then call your functions from the toplevel: (let ((from "from") (to1 "T") (to2 "t") (found-start 0) (found-end 2)) (do-something from to1 to2 found-start found-end)) -- __Pascal Bourguignon__ http://www.informatimago.com/ “The factory of the future will have only two employees, a man and a dog. The man will be there to feed the dog. The dog will be there to keep the man from touching the equipment.” -- Carl Bass CEO Autodesk