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: 1636 bytes --]

4 dec. 2023 kl. 19.19 skrev Alan Mackenzie <acm@muc.de>:

> You've undone the bug fix, and the bug is there again.

Oops. An honest mistake, very sorry about that.
On the other hand, these blunders often turn out to be beneficial in the end because they force us to take a better look at the problem.

I ended up maintaining `byte-compile-form-stack` in `cconv-convert` which isn't free but a lot better than doing it in the optimiser. We emit a fair amount of warnings in cconv so this should be useful for other reasons as well.

The warning was also reverted from delayed to immediate, which makes sense in this case (since it's essentially a syntax error) and in fact it wouldn't work otherwise because delayed warnings implicitly use the byte-compile-form-stack we have when traversing the post-optimisation tree in codegen.

I'm inclined to do something about this last problem for good measure (see attached patch). Most of the time the byte-compile-form-stack doesn't matter much because the warning argument contains a symbol with position, but when it does, the stack state in codegen when the warning is emitted is likely to be less useful than when the warning was registered in the front-end. Haven't made up my mind about this yet.

> We do indeed, but here binding the variable simply doesn't work.  Parts
> of the compiler, when they encounter errors, signal an error which gets
> caught by a condition-case somewhere.

Yes, so I noticed. This is rubbish of course; we should do something about it. We have some options.
Meanwhile I made a macro to encapsulate the ugly push-pop logic in one place.


[-- 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)