bug#67483: Wrong warning position given by the byte compiler for a malformed function

["Mattias Engdegård" <mattias.engdegard@gmail.com>]

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

22 dec. 2023 kl. 12.24 skrev Alan Mackenzie <acm@muc.de>:

> What's the process of converting to a closure
> got to do with maintaining the stack of forms for error processing?

Not much, but cconv has evolved to one major part of the compiler front-end (the other being macroexp-all) and isn't restricted to just closure conversion. In fact, it's now used for dynbound code as well.

In particular it's a natural place for various front-end checks and transforms, so don't let the place and name distract you. There are plans to refactor it later on for other reasons.

> Whoops!  There was no patch.

Attached it now, sorry.

> You've put the new macro into macroexp.el.  This file is purely about
> macro handling.

Actually macroexp.el does more than that, and in any case the file isn't very important; the macro ended up there to be next to byte-compile-form-stack. Nor is the name; it can be changed at any time.

However, it probably needs to be in that file for bootstrap reasons.

> And is the "--" in the name
> appropriate, given that the macro is used by several files?  I'm not
> sure about that rule.

The double-dash just means that users shouldn't get any funny ideas. (The converse isn't true: a name without double-dash isn't automatically fair game.)

> Also, byte-compile-form-stack gets bound in cconv-closure-convert.  Why?

It's just a backstop. Not strictly needed. It's probably fine to remove it if you are worried, but then again there shouldn't be any (non-bug) error signalling here. I'll have a look.


[-- Attachment #2: 0001-Capture-byte-compile-form-stack-in-delayed-warnings-.patch --]
[-- Type: application/octet-stream, Size: 1744 bytes --]

From 0a4222090ee3b6690b3097a06dacd23546c7350b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Wed, 20 Dec 2023 11:11:56 +0100
Subject: [PATCH] Capture byte-compile-form-stack in delayed warnings
 (bug#67483)

* lisp/emacs-lisp/macroexp.el (macroexp--warn-wrap):
When recording a delayed warning, capture the current value of
`byte-compile-form-stack` because it is more likely to contain a
relevant source location than whatever we are traversing in codegen.
---
 lisp/emacs-lisp/macroexp.el | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el
index 2a646be9725..8cf6ad7256d 100644
--- a/lisp/emacs-lisp/macroexp.el
+++ b/lisp/emacs-lisp/macroexp.el
@@ -168,12 +168,14 @@ macroexp-file-name
 (defvar macroexp--warned (make-hash-table :test #'equal :weakness 'key))
 
 (defun macroexp--warn-wrap (arg msg form category)
-  (let ((when-compiled
-	 (lambda ()
-           (when (if (consp category)
-                     (apply #'byte-compile-warning-enabled-p category)
-                   (byte-compile-warning-enabled-p category))
-             (byte-compile-warn-x arg "%s" msg)))))
+  (let* ((stack byte-compile-form-stack)
+         (when-compiled
+	  (lambda ()
+            (when (if (consp category)
+                      (apply #'byte-compile-warning-enabled-p category)
+                    (byte-compile-warning-enabled-p category))
+              (let ((byte-compile-form-stack stack))
+                (byte-compile-warn-x arg "%s" msg))))))
     `(progn
        (macroexp--funcall-if-compiled ',when-compiled)
        ,form)))
-- 
2.32.0 (Apple Git-132)