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

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: sbaugh@catern.com
>> Date: Sat, 14 Oct 2023 22:25:37 +0000 (UTC)
>> Cc: Spencer Baugh <sbaugh@janestreet.com>, 66326@debbugs.gnu.org
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Spencer Baugh <sbaugh@janestreet.com>
>> >> Cc: sbaugh@catern.com,  66326@debbugs.gnu.org
>> >> Date: Wed, 04 Oct 2023 08:20:49 -0400
>> >> 
>> >> Eli Zaretskii <eliz@gnu.org> writes:
>> >> > And in this case, duplication is a lesser evil than reordering of
>> >> > logic, since the chances of unintended consequences would be lower in
>> >> > the former case.
>> >> 
>> >> OK, how about this version then?
>> >
>> > This is much better, thanks.  But it still fails to execute this part
>> > right away:
>> >
>> >   (if (not (or after-init-time noninteractive (daemonp)))
>> >       ;; Ensure warnings that happen early in the startup sequence
>> >       ;; are visible when startup completes (bug#20792).
>> >       (delay-warning type message level buffer-name)
>> >
>> > We must preserve this functionality, unaffected by these changes.  The
>> > patch you propose doesn't seem to guarantee that, at least not
>> > clearly enough for my palate.
>> 
>> Ah, actually that's deliberate.
>
> If it's deliberate, it will have to come with an additional option to
> enable it.  I don't want users to have their startup aborted just
> because they want some warning later on converted to an error.
> Startup is a delicate process where signaling errors is not always a
> good idea, and we delay warnings there for a good reason.  Changing
> this unconditionally is not acceptable.

What is the good reason that we delay warnings?  AFAIK the reason is
solely that the warnings would not be visible.  Which is indeed a good
reason, but does not apply if the warnings are turned into errors.

But OK, attached is a new patch which adds an option to immediately
raise the warnings converted into errors during startup, instead of
delaying them.  So the default is to delay the error until after
startup.  Does this work?

>> (And that's one of the motivations of this change: to make it easier to
>> debug a warning that happens during startup, by turning it into an error
>> that can be debug-init'd)
>
> There's no difficulty in debugging a warning whatsoever, IME.  It is a
> serious exaggeration to claim that there's a significant problem here
> that needs a solution.

I'm curious: if you have some piece of code which warns during startup,
how would you find out why that code is running?  e.g., how would you
figure out what function is calling some other code which internally
warns?  This seems pretty difficult to me.

> Nevertheless, I'm okay with people opting in to shooting themselves in
> the foot, I just don't agree to doing that without an explicit user's
> consent.

>From 25370397fb45d16f434e1c80bc5b04e8567c9f69 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Mon, 16 Oct 2023 15:22:51 -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): Improve
docstring.
(warning-to-error-types, warning-to-error): Add. (bug#66326)
(display-warning): Check warning-to-error-types.
(warning-suppress-p): Allow () to match any type.
---
 lisp/emacs-lisp/warnings.el | 52 ++++++++++++++++++++++++++++++++++---
 1 file changed, 48 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/warnings.el b/lisp/emacs-lisp/warnings.el
index 31b840d6c83..f6d10be8141 100644
--- a/lisp/emacs-lisp/warnings.el
+++ b/lisp/emacs-lisp/warnings.el
@@ -114,11 +114,37 @@ warning-suppress-types
 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.
+An empty list as an element matches any 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))
   :version "22.1")
+
+(defcustom warning-to-error-types nil
+  "List of warning types to signal as an error instead.
+If any element of this list matches the TYPE argument to `display-warning',
+`display-warning' signals an error instead of logging a warning.
+See `warning-suppress-types' for the format of elements in this list.
+A useful value is (()) to convert all warnings into errors.
+See also `warning-signal-errors-during-startup'."
+  :type '(repeat (repeat symbol))
+  :version "30.1")
+
+(defcustom warning-signal-errors-during-startup nil
+  "If non-nil, warnings converted into errors are signaled during startup.
+
+Normally, warnings generated during startup are delayed until
+after startup.  This includes warnings converted into errors by
+`warning-to-error-types': they will be signaled after startup
+completes, outside the context of the code which caused the
+warning.  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
@@ -180,10 +206,11 @@ warning-suppress-p
   (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)))
+         ;; 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)
@@ -230,6 +257,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 +299,14 @@ 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 (>= (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)
+             (or warning-signal-errors-during-startup
+                 after-init-time noninteractive (daemonp))
+             )
+    (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).
-- 
2.39.3