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> Subject: bug#73725: closed (Re: bug#73725: Acknowledgement (Master: Wrong position for byte compiler warning message.)) Date: Mon, 14 Jul 2025 10:14:02 +0000
[Message part 1 (text/plain, inline)]
Your bug report #73725: Master: Wrong position for byte compiler warning message. which was filed against the emacs package, has been closed. The explanation is attached below, along with your original report. If you require more details, please reply to 73725 <at> debbugs.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: 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).
[Message part 3 (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).
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.