Package: emacs;
Reported by: Alan Mackenzie <acm <at> muc.de>
Date: Thu, 10 Oct 2024 10:24:02 UTC
Severity: normal
Tags: patch
Done: Alan Mackenzie <acm <at> muc.de>
View this message in rfc822 format
From: help-debbugs <at> gnu.org (GNU bug Tracking System) To: Alan Mackenzie <acm <at> muc.de> Cc: tracker <at> debbugs.gnu.org Subject: bug#73725: closed (Master: Wrong position for byte compiler warning message.) Date: Mon, 14 Jul 2025 10:14:02 +0000
[Message part 1 (text/plain, inline)]
Your message dated Mon, 14 Jul 2025 10:13:02 +0000 with message-id <aHTYLg9rRKFYsBqx <at> MAC.fritz.box> and subject line Re: bug#73725: Acknowledgement (Master: Wrong position for byte compiler warning message.) has caused the debbugs.gnu.org bug report #73725, regarding Master: Wrong position for byte compiler warning message. to be marked as done. (If you believe you have received this mail in error, please contact help-debbugs <at> gnu.org.) -- 73725: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=73725 GNU Bug Tracking System Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Alan Mackenzie <acm <at> muc.de> To: bug-gnu-emacs <at> gnu.org Subject: Master: Wrong position for byte compiler warning message. Date: Thu, 10 Oct 2024 10:22:49 +0000Hello, Emacs. On the master branch. (i) Create the following file, bad-error-position2.el: ######################################################################### ;; -*- lexical-binding:t -*- (eval-and-compile (defmacro increase () `(let ((foo (point-max))) (cond ((consp foo) (message "consp %s" foo) foo) ((numberp foo) (1+ fooo)) ; Note the misspelling. (t (message "Something else: %s" foo)))))) (defun call-increase (bar) (cond ((not (or (consp bar) (numberp bar))) bar) (t (increase)))) ######################################################################### Note the misspelling of `foo' as `fooo' on line 10. (ii) emacs -Q (iii) M-x byte-compile-file /path/to/bad-error-position2.el. (iv) This gives the warning message: bad-error-position2.el:14:4: Warning: reference to free variable `fooo' (v) The position 14:4 is wrong. That is the position of the `cond' symbol in `call-increase'. (vi) The correct message should be: bad-error-position2.el:18:8: Warning: reference to free variable `fooo' .. 18:8 is the position of `increase' on the last line of the file. ######################################################################### Diagnosis --------- When the byte compiler expands the invocation of `increase' on the last line, `increase' is a symbol with position. The form returned by the macro expander, however, has no position on its first symbol, the `let'. Thus when the warning is being processed, the pertinent entry on byte-compile-form-stack has no symbols with position, so the code takes a SWP from a deeper nested form, the `cond' form on line 14, and derives the warning position from it. ######################################################################### Proposed fix ------------ To fix this I propose amending the macro expander, such that if the first symbol in a form being expanded is a SWP, the position is applied to the expanded form, typically on the car of that form, but possibly elsewhere if that expanded form isn't a list. The following patch appears to fix the bug: diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 29e7882c851..e0a59d19d4e 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -2582,14 +2582,17 @@ byte-compile-flush-pending byte-compile-jump-tables nil)))) (defun byte-compile-preprocess (form &optional _for-effect) - (setq form (macroexpand-all form byte-compile-macro-environment)) + (let ((call-pos (and (consp form) + (symbol-with-pos-p (car form)) + (symbol-with-pos-pos (car form))))) + (setq form (macroexpand-all form byte-compile-macro-environment call-pos)) ;; FIXME: We should run byte-optimize-form here, but it currently does not ;; recurse through all the code, so we'd have to fix this first. ;; Maybe a good fix would be to merge byte-optimize-form into ;; macroexpand-all. ;; (if (memq byte-optimize '(t source)) ;; (setq form (byte-optimize-form form for-effect))) - (cconv-closure-convert form byte-compile-bound-variables)) + (cconv-closure-convert form byte-compile-bound-variables))) ;; byte-hunk-handlers cannot call this! (defun byte-compile-toplevel-file-form (top-level-form) diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el index 4524eccc7ef..44d49bd7757 100644 --- a/lisp/emacs-lisp/macroexp.el +++ b/lisp/emacs-lisp/macroexp.el @@ -237,11 +237,63 @@ macroexpand-1 form)))))))) (t form))) +(defun sub-macroexp--posify-form (form call-pos depth) + "Try to apply the transformation of `macroexp--posify-form' to FORM. +FORM and CALL-POS are as in that function. DEPTH is a small integer, +decremented at each recursive call, to prevent infinite recursion. +Return the changed form, or nil if no change happened." + (let (new-form + ) + (cond + ((zerop depth) nil) + ((and (consp form) + (symbolp (car form)) + (car form)) + (setcar form (position-symbol (car form) call-pos)) + form) + ((consp form) + (or (when (setq new-form (sub-macroexp--posify-form + (car form) call-pos (1- depth))) + (setcar form new-form) + form) + (when (setq new-form (sub-macroexp--posify-form + (cdr form) call-pos (1- depth))) + (setcdr form new-form) + form))) + ((symbolp form) + (if form ; Don't position nil! + (position-symbol form call-pos))) + ((and (or (vectorp form) (recordp form))) + (let ((len (length form)) + (i 0) + ) + (while (and (< i len) + (not (setq new-form (sub-macroexp--posify-form + (aref form i) call-pos (1- depth))))) + (setq i (1+ i))) + (when (< i len) + (aset form i new-form) + form)))))) + +(defun macroexp--posify-form (form call-pos) + "Try to apply the position CALL-POS to the form FORM. +CALL-POS is a buffer position, a number. FORM may be any lisp form, +and is typically the output form returned by macro expansion. +Apply CALL-POS to FORM as a symbol with position, such that +`byte-compile--first-symbol-with-pos' can later return it. Return +the possibly modified FORM." + (let ((new-form (sub-macroexp--posify-form form call-pos 10))) + (or new-form form))) + (defun macroexp-macroexpand (form env) "Like `macroexpand' but checking obsolescence." (let* ((macroexpand-all-environment env) - new-form) + (call-pos (and (consp form) + (symbol-with-pos-p (car form)) + (symbol-with-pos-pos (car form)))) + macroexpanded new-form) (while (not (eq form (setq new-form (macroexpand-1 form env)))) + (setq macroexpanded t) (let ((fun (car-safe form))) (setq form (if (and fun (symbolp fun) @@ -252,6 +304,8 @@ macroexp-macroexpand (if (symbolp (symbol-function fun)) "alias" "macro")) new-form (list 'obsolete fun) nil fun) new-form)))) + (when (and macroexpanded call-pos) + (setq form (macroexp--posify-form form call-pos))) form)) (defun macroexp--unfold-lambda (form &optional name) @@ -516,14 +570,22 @@ macroexp--expand-all (_ form)))))) ;;;###autoload -(defun macroexpand-all (form &optional environment) +(defun macroexpand-all (form &optional environment call-pos) "Return result of expanding macros at all levels in FORM. If no macros are expanded, FORM is returned unchanged. The second optional arg ENVIRONMENT specifies an environment of macro -definitions to shadow the loaded ones for use in file byte-compilation." - (let ((macroexpand-all-environment environment) - (macroexp--dynvars macroexp--dynvars)) - (macroexp--expand-all form))) +definitions to shadow the loaded ones for use in file byte-compilation. +CALL-POS, if non-nil, is a buffer position which will be incorporated +into the result form as a symbol with position, later to be usable by +`byte-compile--warning-source-offset'." + (let* + ((macroexpand-all-environment environment) + (macroexp--dynvars macroexp--dynvars) + (new-form + (macroexp--expand-all form))) + (if call-pos + (macroexp--posify-form new-form call-pos) + new-form))) ;; This function is like `macroexpand-all' but for use with top-level ;; forms. It does not dynbind `macroexp--dynvars' because we want -- Alan Mackenzie (Nuremberg, Germany).
[Message part 3 (message/rfc822, inline)]
From: Alan Mackenzie <acm <at> muc.de> To: 73725-done <at> debbugs.gnu.org Cc: acm <at> muc.de Subject: Re: bug#73725: Acknowledgement (Master: Wrong position for byte compiler warning message.) Date: Mon, 14 Jul 2025 10:13:02 +0000On Thu, Oct 10, 2024 at 10:24:02 +0000, GNU bug Tracking System wrote: > Thank you for filing a new bug report with debbugs.gnu.org. > This is an automatically generated reply to let you know your message > has been received. > Your message is being forwarded to the package maintainers and other > interested parties for their attention; they will reply in due course. > As you requested using X-Debbugs-CC, your message was also forwarded to > Stefan Monnier <monnier <at> iro.umontreal.ca>, Mattias Engdegård <mattias.engdegard <at> gmail.com> > (after having been given a bug report number, if it did not have one). > Your message has been sent to the package maintainer(s): > bug-gnu-emacs <at> gnu.org > If you wish to submit further information on this problem, please > send it to 73725 <at> debbugs.gnu.org. > Please do not send mail to help-debbugs <at> gnu.org unless you wish > to report a problem with the Bug-tracking system. I have finally committed the bug fix. > -- > 73725: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=73725 > GNU Bug Tracking System > Contact help-debbugs <at> gnu.org with problems -- Alan Mackenzie (Nuremberg, Germany).
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.