bug#58396: 29.0.50; Optimization failure for add-to-list

[Jens Schmidt via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I also wanted to ask whether to extend that approach to the other advice
>> installed in `elisp--local-variables´
>
> That would be nice, but...
>
>> but then noticed that `macroexpand´ is actually a built-in.
>
> `elisp--local-variables´ doesn't use that built-in any more, it uses
> `macroexpand-1` instead, which is implemented in `macroexp.el` :-)
>
> But the problem is that the advice wraps the call within
> a `condition-case` and that's inconvenient to do in a way which
> doesn't interfere too much with "normal use" (e.g. doesn't hide/catch
> errors when we don't want to, and doesn't impose too much of
> a performance cost either).

Agreed.

Anyway, here are two patches: One for Emacs 29 and one for master.

The patch for Emacs 29:

- does not contain the rather non-local change for avoiding compiler
  macros.  But it fixes this bug, anyway, by just ignoring all errors
  during the `macroexpand-all' call.

- contains a "don't merge to master" maker in the commit message.

What do you think?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-29-Silence-macro-expansion-during-completion-at-point.patch --]
[-- Type: text/x-diff, Size: 1854 bytes --]

From cc663ccb14f3fae361733fe9742d7c7d07274d54 Mon Sep 17 00:00:00 2001
From: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
Date: Tue, 26 Sep 2023 22:26:15 +0200
Subject: [PATCH] Silence macro expansion during completion-at-point

* lisp/progmodes/elisp-mode.el (elisp--local-variables): Silence
messages.  Suppress all errors during macro expansion.  (Bug#58396)

Do not merge to master.
---
 lisp/progmodes/elisp-mode.el | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index bd3916ce108..bc01058ff3c 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -445,11 +445,19 @@ elisp--local-variables
                                    (condition-case nil
                                        (apply expander form args)
                                      (error form))))
+             ;; Avoid any macro expansion errors when attempting
+             ;; completion at point (bug#58148).  As Stefan suggested
+             ;; there: Silence messages [1] and suppress all errors
+             ;; [3].  (Not avoiding compiler macros in the current
+             ;; release branch, though.)
              (sexp
               (unwind-protect
-                  (let ((warning-minimum-log-level :emergency))
+                  (let ((inhibit-message t)     ;[1]
+                        (warning-minimum-log-level :emergency))
                     (advice-add 'macroexpand :around macroexpand-advice)
-                    (macroexpand-all sexp))
+                    (condition-case nil         ;[3]
+                        (macroexpand-all sexp)
+                      (t sexp)))
                 (advice-remove 'macroexpand macroexpand-advice)))
              (vars (elisp--local-variables-1 nil sexp)))
         (delq nil
-- 
2.30.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-30-Silence-macro-expansion-during-completion-at-point.patch --]
[-- Type: text/x-diff, Size: 3341 bytes --]

From feb9906cece4a1fbce6417e9225a937aa7e8c830 Mon Sep 17 00:00:00 2001
From: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
Date: Fri, 29 Sep 2023 22:04:43 +0200
Subject: [PATCH] Silence macro expansion during completion-at-point

* lisp/emacs-lisp/macroexp.el (macroexp-inhibit-compiler-macros): Add
variable.
(macroexp--compiler-macro): Inspect that new variable and, if it is
non-nil, return the input form unchanged.
* lisp/progmodes/elisp-mode.el (elisp--local-variables): Silence
messages.  Avoid compiler macros.  Suppress all errors during macro
expansion.  (Bug#58396)
---
 lisp/emacs-lisp/macroexp.el  | 20 ++++++++++++++------
 lisp/progmodes/elisp-mode.el | 12 ++++++++++--
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el
index 3ef924a5c73..6eb670d6dc1 100644
--- a/lisp/emacs-lisp/macroexp.el
+++ b/lisp/emacs-lisp/macroexp.el
@@ -105,13 +105,21 @@ macroexp--all-clauses
 	(macroexp--all-forms clause skip)
       clause)))
 
+(defvar macroexp-inhibit-compiler-macros nil
+  "Inhibit application of compiler macros if non-nil.")
+
 (defun macroexp--compiler-macro (handler form)
-  (condition-case-unless-debug err
-      (apply handler form (cdr form))
-    (error
-     (message "Warning: Optimization failure for %S: Handler: %S\n%S"
-              (car form) handler err)
-     form)))
+  "Apply compiler macro HANDLER to FORM and return the result.
+Unless `macroexp-inhibit-compiler-macros' is non-nil, in which
+case return FORM unchanged."
+  (if macroexp-inhibit-compiler-macros
+      form
+    (condition-case-unless-debug err
+        (apply handler form (cdr form))
+      (error
+       (message "Warning: Optimization failure for %S: Handler: %S\n%S"
+                (car form) handler err)
+       form))))
 
 (defun macroexp--funcall-if-compiled (_form)
   "Pseudo function used internally by macroexp to delay warnings.
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 664299df288..434b493ed55 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -458,11 +458,19 @@ elisp--local-variables
                     (apply expander form args)
                   ((debug error)
                    (message "Ignoring macroexpansion error: %S" err) form))))
+             ;; Avoid any macro expansion errors when attempting
+             ;; completion at point (bug#58148).  As Stefan suggested
+             ;; there: Silence messages [1], avoid compiler macros
+             ;; [2], and suppress all errors [3].
              (sexp
               (unwind-protect
-                  (let ((warning-minimum-log-level :emergency))
+                  (let ((inhibit-message t)                  ;[1]
+                        (macroexp-inhibit-compiler-macros t) ;[2]
+                        (warning-minimum-log-level :emergency))
                     (advice-add 'macroexpand-1 :around macroexpand-advice)
-                    (macroexpand-all sexp elisp--local-macroenv))
+                    (condition-case nil         ;[3]
+                        (macroexpand-all sexp elisp--local-macroenv)
+                      (t sexp)))
                 (advice-remove 'macroexpand-1 macroexpand-advice)))
              (vars (elisp--local-variables-1 nil sexp)))
         (delq nil
-- 
2.30.2