bug#66326: 29.1.50; There should be a way to promote warnings to errors

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: sbaugh@catern.com,  66326@debbugs.gnu.org
>> Date: Thu, 19 Oct 2023 11:18:53 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > It is a concern for displaying anything and everything.
>> 
>> Could you be more specific?  Is there a way to signal an error during
>> startup, which won't be displayed?
>
> What worries me more is that there's a way to signal an error that
> could crash Emacs during these early stages of startup.

I ask this because I genuinely want to understand: Is it a problem for a
error to be signaled during these early stages of startup?  It will
prevent Emacs from starting, of course, but is that a problem if this
just causes Emacs to exit and print an error?

>> >> So there's no need to do extra work to delay errors in warnings.el.
>> >
>> > Yes, there is.
>> 
>> Can you please elaborate?  What exactly will go wrong if we don't delay
>> errors in warnings.el?
>
> See above.  You seem to be summarily dismissing what I'm saying, so I
> have little motivation to elaborate.

My apologies, I am not intending to dismiss what you're saying, I really
do want to understand the issue.  I'm just keeping an open mind: I don't
yet know whether the issue you're raising can actually cause a problem.
Or, if there is a problem, if maybe there's a way to solve the problem
other than this warning-signal-errors-during-startup variable.  That's
why I'm trying to understand.  I'd appreciate it if you'd keep an open
mind about it too.

>> > Delayed and shown how?  Can you show a screenshot or post the contents
>> > of the buffer with the error message?
>> 
>> With
>> (setq warning-to-error-types t)
>> ;;(setq warning-signal-errors-during-startup nil) ; default
>> (warn "foo")
>> 
>> The following appears in the echo area and in *Messages*:
>> 
>> warning-to-error: Warning (emacs): foo
>> Error in delayed-warnings-hook (display-delayed-warnings): (error "Warning 
>> (emacs): foo")
>
> Sounds like in these cases there's no reason to raise an error, since
> the information is not different from a mere delayed-warning?

Good point, changed to not convert to an error if the warning was
delayed.

>From ed3a4ce62d1084269c5a40d25d33f4635cc8df79 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Thu, 19 Oct 2023 10:49:29 -0400
Subject: [PATCH] Support turning warnings into errors

Support turning warnings into errors in a user-configurable way.  This
is especially useful in combination with (setq debug-on-error t) to
drop to the debugger when a warning happens.

* lisp/emacs-lisp/warnings.el: (warning-suppress-types,
warning-suppress-log-types): Document the effect of t.
(warning-to-error-types, warning-to-error): Add. (bug#66326)
(display-warning): Check warning-to-error-types.
(warning-suppress-p): Allow t to match any type.
* lisp/subr.el (display-delayed-warnings): Bind warning-to-error-types
to nil.
---
 lisp/emacs-lisp/warnings.el | 107 ++++++++++++++++++++++++++----------
 lisp/subr.el                |   7 ++-
 2 files changed, 83 insertions(+), 31 deletions(-)

diff --git a/lisp/emacs-lisp/warnings.el b/lisp/emacs-lisp/warnings.el
index 31b840d6c83..c655814f20a 100644
--- a/lisp/emacs-lisp/warnings.el
+++ b/lisp/emacs-lisp/warnings.el
@@ -102,8 +102,10 @@ warning-suppress-log-types
 Thus, (foo bar) as an element matches (foo bar)
 or (foo bar ANYTHING...) as TYPE.
 If TYPE is a symbol FOO, that is equivalent to the list (FOO),
-so only the element (FOO) will match it."
-  :type '(repeat (repeat symbol))
+so only the element (FOO) will match it.
+If this is t, warnings are never logged."
+  :type '(choice (repeat (repeat symbol))
+                 (const :tag "Completely ignore all warnings" t))
   :version "22.1")
 
 (defcustom warning-suppress-types nil
@@ -111,14 +113,42 @@ warning-suppress-types
 If any element of this list matches the TYPE argument to `display-warning',
 the warning is logged nonetheless, but the warnings buffer is
 not immediately displayed.
+If this is t, the warnings buffer is never displayed.
 The element must match an initial segment of the list TYPE.
 Thus, (foo bar) as an element matches (foo bar)
 or (foo bar ANYTHING...) as TYPE.
 If TYPE is a symbol FOO, that is equivalent to the list (FOO),
 so only the element (FOO) will match it.
 See also `warning-suppress-log-types'."
-  :type '(repeat (repeat symbol))
+  :type '(choice (repeat (repeat symbol))
+                 (const :tag "Never display the warnings buffer" t))
   :version "22.1")
+
+(defcustom warning-to-error-types nil
+  "List of warning types that will be converted to errors.
+If any element of this list matches the TYPE argument to `display-warning',
+`display-warning' signals an error instead of displaying a warning.
+See `warning-suppress-types' for the format of elements in this list.
+If this is t, all warnings are converted to errors.
+See also `warning-signal-errors-during-startup'."
+  :type '(choice (repeat (repeat symbol))
+                 (const :tag "Convert all warnings into errors"))
+  :version "30.1")
+
+(defcustom warning-signal-errors-during-startup nil
+  "Whether to signal errors converted from warnings during startup.
+
+Normally, if Emacs is interactive and not a daemon, then warnings
+generated before init finishes loading are delayed until
+`after-init-hook' runs.  This includes warnings converted into
+errors by `warning-to-error-types': they will be displayed after
+startup completes and not converted into an error.  If you'd
+prefer that these errors be signaled immediately so that the
+context is present during debugging, set this variable to nil."
+  :type '(choice
+          (const :tag "Warnings converted into errors raise after startup" nil)
+          (const :tag "Warnings converted into errors raise immediately" t))
+  :version "30.1")

 ;; The autoload cookie is so that programs can bind this variable
 ;; safely, testing the existing value, before they call one of the
@@ -176,32 +206,35 @@ warning-numeric-level
 
 (defun warning-suppress-p (type suppress-list)
   "Non-nil if a warning with type TYPE should be suppressed.
-SUPPRESS-LIST is the list of kinds of warnings to suppress."
-  (let (some-match)
-    (dolist (elt suppress-list)
-      (if (symbolp type)
-         ;; If TYPE is a symbol, the ELT must be (TYPE).
-         (if (and (consp elt)
-                  (eq (car elt) type)
-                  (null (cdr elt)))
-             (setq some-match t))
-       ;; If TYPE is a list, ELT must match it or some initial segment of it.
-       (let ((tem1 type)
-             (tem2 elt)
-             (match t))
-         ;; Check elements of ELT until we run out of them.
-         (while tem2
-           (if (not (equal (car tem1) (car tem2)))
-               (setq match nil))
-           (setq tem1 (cdr tem1)
-                 tem2 (cdr tem2)))
-         ;; If ELT is an initial segment of TYPE, MATCH is t now.
-         ;; So set SOME-MATCH.
-         (if match
-             (setq some-match t)))))
-    ;; If some element of SUPPRESS-LIST matched,
-    ;; we return t.
-    some-match))
+SUPPRESS-LIST is the list of kinds of warnings to suppress,
+or t if all warnings should be suppressed."
+  (or (eq suppress-list t)
+      (let (some-match)
+        (dolist (elt suppress-list)
+          (if (symbolp type)
+             ;; If TYPE is a symbol, the ELT must be (TYPE) or ().
+             (if (or (null elt)
+                      (and (consp elt)
+                          (eq (car elt) type)
+                          (null (cdr elt))))
+                 (setq some-match t))
+           ;; If TYPE is a list, ELT must match it or some initial segment of 
it.
+           (let ((tem1 type)
+                 (tem2 elt)
+                 (match t))
+             ;; Check elements of ELT until we run out of them.
+             (while tem2
+               (if (not (equal (car tem1) (car tem2)))
+                   (setq match nil))
+               (setq tem1 (cdr tem1)
+                     tem2 (cdr tem2)))
+             ;; If ELT is an initial segment of TYPE, MATCH is t now.
+             ;; So set SOME-MATCH.
+             (if match
+                 (setq some-match t)))))
+        ;; If some element of SUPPRESS-LIST matched,
+        ;; we return t.
+        some-match)))

 (define-icon warnings-suppress button
   `((emoji "⛔")
@@ -230,6 +263,15 @@ warnings-suppress
                               (cons (list type) warning-suppress-types)))
     (_ (message "Exiting"))))
 
+(defun warning-to-error (type message level)
+  (unless level
+    (setq level :warning))
+  (let* ((typename (if (consp type) (car type) type))
+         (level-info (assq level warning-levels)))
+    (error (concat (nth 1 level-info) "%s")
+           (format warning-type-format typename)
+           message)))
+
 ;;;###autoload
 (defun display-warning (type message &optional level buffer-name)
   "Display a warning message, MESSAGE.
@@ -263,6 +305,13 @@ display-warning
 disable automatic display of the warning or disable the warning
 entirely by setting `warning-suppress-types' or
 `warning-suppress-log-types' on their behalf."
+  (when (and (or warning-signal-errors-during-startup
+                 after-init-time noninteractive (daemonp))
+             (>= (warning-numeric-level (or level :warning))
+                (warning-numeric-level warning-minimum-log-level))
+            (not (warning-suppress-p type warning-suppress-log-types))
+             (warning-suppress-p type warning-to-error-types))
+    (warning-to-error type message level))
   (if (not (or after-init-time noninteractive (daemonp)))
       ;; Ensure warnings that happen early in the startup sequence
       ;; are visible when startup completes (bug#20792).
diff --git a/lisp/subr.el b/lisp/subr.el
index 58274987d71..5ab6305f856 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -5898,8 +5898,11 @@ do-after-load-evaluation
 (defun display-delayed-warnings ()
   "Display delayed warnings from `delayed-warnings-list'.
 Used from `delayed-warnings-hook' (which see)."
-  (dolist (warning (nreverse delayed-warnings-list))
-    (apply #'display-warning warning))
+  ;; There's no point in converting to errors now, since the warnings
+  ;; have already been delayed out of their original context.
+  (let ((warning-to-error-types nil))
+    (dolist (warning (nreverse delayed-warnings-list))
+      (apply #'display-warning warning)))
   (setq delayed-warnings-list nil))
 
 (defun collapse-delayed-warnings ()
-- 
2.39.3