Subject: bug#73725: closed (Re: bug#73725: Acknowledgement (Master: Wrong position for byte compiler warning message.))

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 14 Jul 2025 10:14:02 +0000

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 [email-masked].

-- 
73725: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=73725
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#73725: Acknowledgement (Master: Wrong position for byte compiler warning message.)
From: Alan Mackenzie <[email-masked]>
To: [email-masked]
Cc: [email-masked]
Date: Mon, 14 Jul 2025 10:13:02 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

On 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 <[email-masked]>, Mattias Engdegård <[email-masked]>
> (after having been given a bug report number, if it did not have one).

> Your message has been sent to the package maintainer(s):
>  [email-masked]

> If you wish to submit further information on this problem, please
> send it to [email-masked].

> Please do not send mail to [email-masked] 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 [email-masked] with problems

-- 
Alan Mackenzie (Nuremberg, Germany).

Subject: Master: Wrong position for byte compiler warning message.
From: Alan Mackenzie <[email-masked]>
To: [email-masked]
Date: Thu, 10 Oct 2024 10:22:49 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello, 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).