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

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

[Spencer Baugh]

It's common in other languages to have a flag such as -Werror to turn
warnings into errors.  This is useful if you're writing some code and
you want to be scrupulous about preventing potential bugs; if code would
warn, you just want to error instead of trying to continue running.

It would be nice to have this feature in Emacs Lisp.

I've personally frequently had trouble with figuring out where exactly a
warning is triggered from.  I eventually settled on (debug-on-warning
'display-warning) and looking at *Backtrace*, but it would be nicer to
just make all warnings into errors which could be handled using whatever
normal error mechanism - which might or might not be the debugger.

I'll implement this feature in a subsequent patch.


In GNU Emacs 29.1.50 (build 4, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2023-09-11 built on

Repository revision: f9bc92d0b36bc631d11c194e4b580f43b7b8dcba
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.8 (Green Obsidian)

Configured using:
 'configure --config-cache --with-x-toolkit=lucid
 --with-gif=ifavailable'

Configured features:
CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM
XINPUT2 XPM LUCID ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Markdown
Memory information:
((conses 16 4175004 208691)
 (symbols 48 59948 1)
 (strings 32 259924 33150)
 (string-bytes 1 10506599)
 (vectors 16 91335)
 (vector-slots 8 2087528 344365)
 (floats 8 615 413)
 (intervals 56 366721 1629)
 (buffers 976 282))

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

[Spencer Baugh]

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


Patch implementing this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-turning-warnings-into-errors.patch --]
[-- Type: text/x-patch, Size: 10631 bytes --]

From 6fad83ea8729569c968ccdfc1ec2807387bc979e Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Tue, 3 Oct 2023 14:36:25 -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.
(display-warning): Check warning-to-error-types.
---
 lisp/emacs-lisp/warnings.el | 209 ++++++++++++++++++++----------------
 1 file changed, 114 insertions(+), 95 deletions(-)

diff --git a/lisp/emacs-lisp/warnings.el b/lisp/emacs-lisp/warnings.el
index 31b840d6c83..9e0a35b87bb 100644
--- a/lisp/emacs-lisp/warnings.el
+++ b/lisp/emacs-lisp/warnings.el
@@ -114,11 +114,20 @@ 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',
+an error is signaled instead of logging a warning.
+See `warning-suppress-types' for the format of elements in this list."
+  :type '(repeat (repeat symbol))
+  :version "30.1")
 \f
 ;; The autoload cookie is so that programs can bind this variable
 ;; safely, testing the existing value, before they call one of the
@@ -230,6 +239,12 @@ warnings-suppress
                               (cons (list type) warning-suppress-types)))
     (_ (message "Exiting"))))
 
+(defun warning-to-error (type message level)
+  (let* ((typename (if (consp type) (car type) type))
+         (level-info (assq level warning-levels)))
+    (error (nth 1 level-info)
+           (format warning-type-format typename))))
+
 ;;;###autoload
 (defun display-warning (type message &optional level buffer-name)
   "Display a warning message, MESSAGE.
@@ -263,105 +278,109 @@ 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."
-  (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)
-    (unless level
-      (setq level :warning))
-    (unless buffer-name
-      (setq buffer-name "*Warnings*"))
+  (unless level
+    (setq level :warning))
+  (unless buffer-name
+    (setq buffer-name "*Warnings*"))
+  (cond
+   ((< (warning-numeric-level level)
+       (warning-numeric-level warning-minimum-log-level)))
+   ((warning-suppress-p type warning-suppress-log-types))
+   ((warning-suppress-p type warning-to-error-types)
+    (warning-to-error type message level))
+   ((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))
+   (t
     (with-suppressed-warnings ((obsolete warning-level-aliases))
       (when-let ((new (cdr (assq level warning-level-aliases))))
         (warn "Warning level `%s' is obsolete; use `%s' instead" level new)
         (setq level new)))
-    (or (< (warning-numeric-level level)
-	   (warning-numeric-level warning-minimum-log-level))
-	(warning-suppress-p type warning-suppress-log-types)
-	(let* ((typename (if (consp type) (car type) type))
-	       (old (get-buffer buffer-name))
-	       (buffer (or old (get-buffer-create buffer-name)))
-	       (level-info (assq level warning-levels))
-               ;; `newline' may be unbound during bootstrap.
-               (newline (if (fboundp 'newline) #'newline
-                          (lambda () (insert "\n"))))
-	       start end)
-	  (with-current-buffer buffer
-	    ;; If we created the buffer, disable undo.
-	    (unless old
-	      (when (fboundp 'special-mode) ; Undefined during bootstrap.
-                (special-mode))
-	      (setq buffer-read-only t)
-	      (setq buffer-undo-list t))
-	    (goto-char (point-max))
-	    (when (and warning-series (symbolp warning-series))
-	      (setq warning-series
-		    (prog1 (point-marker)
-		      (unless (eq warning-series t)
-			(funcall warning-series)))))
-	    (let ((inhibit-read-only t))
-	      (unless (bolp)
-		(funcall newline))
-	      (setq start (point))
-              ;; Don't output the button when doing batch compilation
-              ;; and similar.
-              (unless (or noninteractive (eq type 'bytecomp))
-                (insert (buttonize (icon-string 'warnings-suppress)
-                                   #'warnings-suppress type)
-                        " "))
-	      (if warning-prefix-function
-		  (setq level-info (funcall warning-prefix-function
-					    level level-info)))
-	      (insert (format (nth 1 level-info)
-			      (format warning-type-format typename))
-		      message)
-              (funcall newline)
-	      (when (and warning-fill-prefix
-                         (not (string-search "\n" message))
-                         (not noninteractive))
-		(let ((fill-prefix warning-fill-prefix)
-		      (fill-column warning-fill-column))
-		  (fill-region start (point))))
-	      (setq end (point)))
-	    (when (and (markerp warning-series)
-		       (eq (marker-buffer warning-series) buffer))
-	      (goto-char warning-series)))
-	  (if (nth 2 level-info)
-	      (funcall (nth 2 level-info)))
-	  (cond (noninteractive
-		 ;; Noninteractively, take the text we inserted
-		 ;; in the warnings buffer and print it.
-		 ;; Do this unconditionally, since there is no way
-		 ;; to view logged messages unless we output them.
-		 (with-current-buffer buffer
-		   (save-excursion
-		     ;; Don't include the final newline in the arg
-		     ;; to `message', because it adds a newline.
-		     (goto-char end)
-		     (if (bolp)
-			 (forward-char -1))
-		     (message "%s" (buffer-substring start (point))))))
-		((and (daemonp) (null after-init-time))
-		 ;; Warnings assigned during daemon initialization go into
-		 ;; the messages buffer.
-		 (message "%s"
-			  (with-current-buffer buffer
-			    (save-excursion
-			      (goto-char end)
-			      (if (bolp)
-				  (forward-char -1))
-			      (buffer-substring start (point))))))
-		(t
-		 ;; Interactively, decide whether the warning merits
-		 ;; immediate display.
-		 (or (< (warning-numeric-level level)
-			(warning-numeric-level warning-minimum-level))
-		     (warning-suppress-p type warning-suppress-types)
-		     (let ((window (display-buffer buffer)))
-		       (when (and (markerp warning-series)
-				  (eq (marker-buffer warning-series) buffer))
-			 (set-window-start window warning-series))
-		       (sit-for 0)))))))))
+    (let* ((typename (if (consp type) (car type) type))
+	   (old (get-buffer buffer-name))
+	   (buffer (or old (get-buffer-create buffer-name)))
+	   (level-info (assq level warning-levels))
+           ;; `newline' may be unbound during bootstrap.
+           (newline (if (fboundp 'newline) #'newline
+                      (lambda () (insert "\n"))))
+	   start end)
+      (with-current-buffer buffer
+	;; If we created the buffer, disable undo.
+	(unless old
+	  (when (fboundp 'special-mode) ; Undefined during bootstrap.
+            (special-mode))
+	  (setq buffer-read-only t)
+	  (setq buffer-undo-list t))
+	(goto-char (point-max))
+	(when (and warning-series (symbolp warning-series))
+	  (setq warning-series
+		(prog1 (point-marker)
+		  (unless (eq warning-series t)
+		    (funcall warning-series)))))
+	(let ((inhibit-read-only t))
+	  (unless (bolp)
+	    (funcall newline))
+	  (setq start (point))
+          ;; Don't output the button when doing batch compilation
+          ;; and similar.
+          (unless (or noninteractive (eq type 'bytecomp))
+            (insert (buttonize (icon-string 'warnings-suppress)
+                               #'warnings-suppress type)
+                    " "))
+	  (if warning-prefix-function
+	      (setq level-info (funcall warning-prefix-function
+					level level-info)))
+	  (insert (format (nth 1 level-info)
+			  (format warning-type-format typename))
+		  message)
+          (funcall newline)
+	  (when (and warning-fill-prefix
+                     (not (string-search "\n" message))
+                     (not noninteractive))
+	    (let ((fill-prefix warning-fill-prefix)
+		  (fill-column warning-fill-column))
+	      (fill-region start (point))))
+	  (setq end (point)))
+	(when (and (markerp warning-series)
+		   (eq (marker-buffer warning-series) buffer))
+	  (goto-char warning-series)))
+      (if (nth 2 level-info)
+	  (funcall (nth 2 level-info)))
+      (cond (noninteractive
+	     ;; Noninteractively, take the text we inserted
+	     ;; in the warnings buffer and print it.
+	     ;; Do this unconditionally, since there is no way
+	     ;; to view logged messages unless we output them.
+	     (with-current-buffer buffer
+	       (save-excursion
+		 ;; Don't include the final newline in the arg
+		 ;; to `message', because it adds a newline.
+		 (goto-char end)
+		 (if (bolp)
+		     (forward-char -1))
+		 (message "%s" (buffer-substring start (point))))))
+	    ((and (daemonp) (null after-init-time))
+	     ;; Warnings assigned during daemon initialization go into
+	     ;; the messages buffer.
+	     (message "%s"
+		      (with-current-buffer buffer
+			(save-excursion
+			  (goto-char end)
+			  (if (bolp)
+			      (forward-char -1))
+			  (buffer-substring start (point))))))
+	    (t
+	     ;; Interactively, decide whether the warning merits
+	     ;; immediate display.
+	     (or (< (warning-numeric-level level)
+		    (warning-numeric-level warning-minimum-level))
+		 (warning-suppress-p type warning-suppress-types)
+		 (let ((window (display-buffer buffer)))
+		   (when (and (markerp warning-series)
+			      (eq (marker-buffer warning-series) buffer))
+		     (set-window-start window warning-series))
+		   (sit-for 0)))))))))
 \f
 ;; Use \\<special-mode-map> so that help-enable-autoload can do its thing.
 ;; Any keymap that is defined will do.
-- 
2.39.3

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

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Tue, 03 Oct 2023 14:39:02 -0400
> 
> +(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',
> +an error is signaled instead of logging a warning.
   ^^^^^^^^^^^^^^^^^^^^
Passive voice alert!

>  (defun display-warning (type message &optional level buffer-name)
>    "Display a warning message, MESSAGE.
> @@ -263,105 +278,109 @@ 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."
> -  (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)
> -    (unless level
> -      (setq level :warning))
> -    (unless buffer-name
> -      (setq buffer-name "*Warnings*"))
> +  (unless level
> +    (setq level :warning))
> +  (unless buffer-name
> +    (setq buffer-name "*Warnings*"))
> +  (cond
> +   ((< (warning-numeric-level level)
> +       (warning-numeric-level warning-minimum-log-level)))
> +   ((warning-suppress-p type warning-suppress-log-types))
> +   ((warning-suppress-p type warning-to-error-types)
> +    (warning-to-error type message level))
> +   ((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))
> +   (t

AFAICT, this reorders parts of the evaluation, and thus changes the
semantics/behavior.  Please try to keep the order of the evaluation
the same as it was, to avoid unintended consequences.  (It will also
make the patch review easier.)

Thanks.

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

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Tue, 03 Oct 2023 14:39:02 -0400
>> 
>> +(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',
>> +an error is signaled instead of logging a warning.
>    ^^^^^^^^^^^^^^^^^^^^
> Passive voice alert!
>
>>  (defun display-warning (type message &optional level buffer-name)
>>    "Display a warning message, MESSAGE.
>> @@ -263,105 +278,109 @@ 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."
>> -  (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)
>> -    (unless level
>> -      (setq level :warning))
>> -    (unless buffer-name
>> -      (setq buffer-name "*Warnings*"))
>> +  (unless level
>> +    (setq level :warning))
>> +  (unless buffer-name
>> +    (setq buffer-name "*Warnings*"))
>> +  (cond
>> +   ((< (warning-numeric-level level)
>> +       (warning-numeric-level warning-minimum-log-level)))
>> +   ((warning-suppress-p type warning-suppress-log-types))
>> +   ((warning-suppress-p type warning-to-error-types)
>> +    (warning-to-error type message level))
>> +   ((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))
>> +   (t
>
> AFAICT, this reorders parts of the evaluation, and thus changes the
> semantics/behavior.  Please try to keep the order of the evaluation
> the same as it was, to avoid unintended consequences.  (It will also
> make the patch review easier.)

Unfortuantely it's not possible to avoid either reordering the
evaluation or duplicating some parts of it.  Because, of course we want
a warning to not become an error if it's listed in
warning-suppress-log-types, and we do want it to become an error even if
it occurs early in the startup sequence.  So the
warning-suppress-log-types check has to happen before the to-error
check, and the to-error check has to happen before the early-startup
check.

But currently the warning-suppress-log-types check is after the
early-startup check.  Reordering them doesn't actually change behavior,
because the early-startup check just delays the warning, so it should be
fine.

I can separate out the reordering change into a separate patch if you
like, then you can see that it should not change behavior.

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

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Tue, 03 Oct 2023 19:16:09 +0000 (UTC)
> Cc: Spencer Baugh <sbaugh@janestreet.com>, 66326@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Spencer Baugh <sbaugh@janestreet.com>
> >> Date: Tue, 03 Oct 2023 14:39:02 -0400
> >> 
> >> +(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',
> >> +an error is signaled instead of logging a warning.
> >    ^^^^^^^^^^^^^^^^^^^^
> > Passive voice alert!
> >
> >>  (defun display-warning (type message &optional level buffer-name)
> >>    "Display a warning message, MESSAGE.
> >> @@ -263,105 +278,109 @@ 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."
> >> -  (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)
> >> -    (unless level
> >> -      (setq level :warning))
> >> -    (unless buffer-name
> >> -      (setq buffer-name "*Warnings*"))
> >> +  (unless level
> >> +    (setq level :warning))
> >> +  (unless buffer-name
> >> +    (setq buffer-name "*Warnings*"))
> >> +  (cond
> >> +   ((< (warning-numeric-level level)
> >> +       (warning-numeric-level warning-minimum-log-level)))
> >> +   ((warning-suppress-p type warning-suppress-log-types))
> >> +   ((warning-suppress-p type warning-to-error-types)
> >> +    (warning-to-error type message level))
> >> +   ((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))
> >> +   (t
> >
> > AFAICT, this reorders parts of the evaluation, and thus changes the
> > semantics/behavior.  Please try to keep the order of the evaluation
> > the same as it was, to avoid unintended consequences.  (It will also
> > make the patch review easier.)
> 
> Unfortuantely it's not possible to avoid either reordering the
> evaluation or duplicating some parts of it.  Because, of course we want
> a warning to not become an error if it's listed in
> warning-suppress-log-types, and we do want it to become an error even if
> it occurs early in the startup sequence.  So the
> warning-suppress-log-types check has to happen before the to-error
> check, and the to-error check has to happen before the early-startup
> check.

Even if the above is true, I see no justification to calling
delay-warning with overridden values of level and buffer, and after
the other 'cond' cases, where it originally was called right away.

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.

> Reordering them doesn't actually change behavior, because the
> early-startup check just delays the warning, so it should be fine.

Famous last words.  When will we learn that Emacs is so much complex
that we should humbly realize we never have the full picture to make
such cavalier assumptions?

> I can separate out the reordering change into a separate patch if you
> like, then you can see that it should not change behavior.

No, that won't help me to be sure we are not introducing some
incompatible changes in this long-standing behavior.

Please realize: this is a very minor feature, useful in a small number
of situations, so the risk of it causing us trouble in the much more
important cases of issuing and delaying warnings is unacceptable.  So
unless I'm reasonably sure this risk is very low, I will simply not
agree to installing this feature.

TIA

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

[Spencer Baugh]

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

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?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-turning-warnings-into-errors.patch --]
[-- Type: text/x-patch, Size: 3029 bytes --]

From 11fdd0cdd2d0da28848ad42d8087ebb1a4e05430 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Tue, 3 Oct 2023 14:36:25 -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.
(display-warning): Check warning-to-error-types.
---
 lisp/emacs-lisp/warnings.el | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/lisp/emacs-lisp/warnings.el b/lisp/emacs-lisp/warnings.el
index 31b840d6c83..0e8464c4455 100644
--- a/lisp/emacs-lisp/warnings.el
+++ b/lisp/emacs-lisp/warnings.el
@@ -114,11 +114,20 @@ 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."
+  :type '(repeat (repeat symbol))
+  :version "30.1")
 \f
 ;; The autoload cookie is so that programs can bind this variable
 ;; safely, testing the existing value, before they call one of the
@@ -230,6 +239,14 @@ 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 (nth 1 level-info)
+           (format warning-type-format typename))))
+
 ;;;###autoload
 (defun display-warning (type message &optional level buffer-name)
   "Display a warning message, MESSAGE.
@@ -263,6 +280,11 @@ 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))
+    (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

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

[Eli Zaretskii]

> 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.

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

[sbaugh]

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.  This existing expression is meant to
delay warnings until they can be properly displayed; for warnings, that
user can't tell that the warning was delayed - whether the warning
happens during startup or at the end is indistinguishable for the user.

But if a warning is turned into an error, it would be incorrect to
signal that error later after startup is finished; it would e.g. mean
that --debug-init would just show the error being signaled at the end of
startup, instead of in the actual code that warned.

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

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

[Eli Zaretskii]

> 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.

> (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.  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.

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

[Spencer Baugh]

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

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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-turning-warnings-into-errors.patch --]
[-- Type: text/x-patch, Size: 4675 bytes --]

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")
 \f
 ;; 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

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

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: sbaugh@catern.com,  66326@debbugs.gnu.org
> Date: Mon, 16 Oct 2023 15:26:51 -0400
> 
> > 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?

Because Emacs is not yet able to display stuff reliably, for example.
Or the display environment was not yet set up completely.

> 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?

Did you test that?  If you did, what happens with delayed warnings
when the new option is nil?

> > 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?

By searching for the warning text and analyzing the code which invokes
the warning, of course.  And that's only if the warning message itself
doesn't explain itself (which a good warning should).

> e.g., how would you figure out what function is calling some other
> code which internally warns?  This seems pretty difficult to me.

I normally find this unnecessary.  In very rare and extreme
situations, I run Emacs under GDB.

Like I said: this is a niche feature, which seems to have grown out of
some very specific experience you had at some point.  It is not at all
common enough to consider it important.  So the last thing we should
allow is for it to introduce regressions, and IME messing with startup
code runs a very high risk of introducing regressions.

> --- 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.

Why was this change necessary?  An empty list is the same as nil, and
treating nil as matching everything is unusual and unintuitive, IMO.
If this option is really needed and important, the value of t is much
more natural.

> +(defcustom warning-to-error-types nil
> +  "List of warning types to signal as an error instead.
                                                  ^^^^^^^
"instead of a warning".  Or maybe even

  List of types of warnings 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 logging a warning.

"Logging" is not a good terminology here, as warning are "displayed",
not "logged".

> +(defcustom warning-signal-errors-during-startup nil
> +  "If non-nil, warnings converted into errors are signaled during startup.

 "Whether to signal errors converted from warnings 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."

I think this should explain what does "after startup" mean, and also
mention the special case of daemon.

>  (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))
> +             )

I'd suggest to move the last condition to be the first one, or
thereabouts, as the default values will then make this code a tad
faster

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

[Spencer Baugh]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: sbaugh@catern.com,  66326@debbugs.gnu.org
>> Date: Mon, 16 Oct 2023 15:26:51 -0400
>> 
>> > 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?
>
> Because Emacs is not yet able to display stuff reliably, for example.
> Or the display environment was not yet set up completely.

Right.  But that is not a concern for errors.  We already go to great
lengths to make sure that errors signaled during startup can be
displayed to the user one way or another.  So there's no need to do
extra work to delay errors in warnings.el.

>> 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?
>
> Did you test that?  If you did, what happens with delayed warnings
> when the new option is nil?

They continue to be delayed.  If a warning is turned into an error, the
error is signaled by delayed-warnings-hook, run by after-init-hook.

>> > 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?
>
> By searching for the warning text and analyzing the code which invokes
> the warning, of course.  And that's only if the warning message itself
> doesn't explain itself (which a good warning should).
>
>> e.g., how would you figure out what function is calling some other
>> code which internally warns?  This seems pretty difficult to me.
>
> I normally find this unnecessary.  In very rare and extreme
> situations, I run Emacs under GDB.
>
> Like I said: this is a niche feature, which seems to have grown out of
> some very specific experience you had at some point.  It is not at all
> common enough to consider it important.  So the last thing we should
> allow is for it to introduce regressions, and IME messing with startup
> code runs a very high risk of introducing regressions.

It's an experience I've had several times, but yes.  The very concrete
motivation is: I have some user running code which is emitting warnings.
They find this annoying.  I can't practically tell a user to run Emacs
under GDB.  I'd like to have some easy way to get more information about
the warning.  So, if I have the user do (setq warning-to-error-types t),
or:

./src/emacs --eval '(setq warning-to-error-types t
warning-signal-errors-during-startup t)' --debug-init

then I can just tell them to send me the contents of *Backtrace* and
I can figure out what's warning from that.

That's also useful to me personally when debugging, and I also think
it's useful for running automated tests where warnings should cause
failures.

(All these use cases would be improved if I didn't have to remember to
also set warning-signal-errors-during-startup, BTW.  That variable
doesn't seem to ever help...)

>> --- 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.
>
> Why was this change necessary?  An empty list is the same as nil, and
> treating nil as matching everything is unusual and unintuitive, IMO.
> If this option is really needed and important, the value of t is much
> more natural.

Yes, good point, I'll do that.

The reason I made nil match everything is because currently nil *does*
match any list of warning types; but if the warning type is just a
single symbol, it doesn't match.

But yes, just having t match anything is much more sensible.

>> +(defcustom warning-to-error-types nil
>> +  "List of warning types to signal as an error instead.
>                                                   ^^^^^^^
> "instead of a warning".  Or maybe even
>
>   List of types of warnings that will be converted to errors.

Fixed.

>> +If any element of this list matches the TYPE argument to `display-warning',
>> +`display-warning' signals an error instead of logging a warning.
>
> "Logging" is not a good terminology here, as warning are "displayed",
> not "logged".

Fixed.

>
>> +(defcustom warning-signal-errors-during-startup nil
>> +  "If non-nil, warnings converted into errors are signaled during startup.
>
>  "Whether to signal errors converted from warnings during startup."

Fixed.

>
>> +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."
>
> I think this should explain what does "after startup" mean, and also
> mention the special case of daemon.

Fixed.

>
>>  (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))
>> +             )
>
> I'd suggest to move the last condition to be the first one, or
> thereabouts, as the default values will then make this code a tad
> faster

Fixed.


[-- Attachment #2: 0001-Support-turning-warnings-into-errors.patch --]
[-- Type: text/x-patch, Size: 7403 bytes --]

From 4b342c2a36aad90a34ce21e31514245eb81cf04d 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/emacs-lisp/warnings.el | 108 ++++++++++++++++++++++++++----------
 1 file changed, 79 insertions(+), 29 deletions(-)

diff --git a/lisp/emacs-lisp/warnings.el b/lisp/emacs-lisp/warnings.el
index 31b840d6c83..e7d1a50f72a 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,43 @@ 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 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")
 \f
 ;; The autoload cookie is so that programs can bind this variable
 ;; safely, testing the existing value, before they call one of the
@@ -176,32 +207,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)))
 \f
 (define-icon warnings-suppress button
   `((emoji "⛔")
@@ -230,6 +264,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 +306,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).
-- 
2.39.3

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

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: sbaugh@catern.com,  66326@debbugs.gnu.org
> Date: Thu, 19 Oct 2023 10:50:59 -0400
> 
> >> What is the good reason that we delay warnings?
> >
> > Because Emacs is not yet able to display stuff reliably, for example.
> > Or the display environment was not yet set up completely.
> 
> Right.  But that is not a concern for errors.

It is a concern for displaying anything and everything.

> So there's no need to do extra work to delay errors in warnings.el.

Yes, there is.

> >> 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?
> >
> > Did you test that?  If you did, what happens with delayed warnings
> > when the new option is nil?
> 
> They continue to be delayed.  If a warning is turned into an error, the
> error is signaled by delayed-warnings-hook, run by after-init-hook.

Delayed and shown how?  Can you show a screenshot or post the contents
of the buffer with the error message?

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 10:50:59 -0400
>> 
>> >> What is the good reason that we delay warnings?
>> >
>> > Because Emacs is not yet able to display stuff reliably, for example.
>> > Or the display environment was not yet set up completely.
>> 
>> Right.  But that is not a concern for errors.
>
> 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?

>> 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?

>> >> 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?
>> >
>> > Did you test that?  If you did, what happens with delayed warnings
>> > when the new option is nil?
>> 
>> They continue to be delayed.  If a warning is turned into an error, the
>> error is signaled by delayed-warnings-hook, run by after-init-hook.
>
> 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")

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

[Eli Zaretskii]

> 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.

> >> 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.

> > 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?

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

[Spencer Baugh]

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

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.


[-- Attachment #2: 0001-Support-turning-warnings-into-errors.patch --]
[-- Type: text/x-patch, Size: 8268 bytes --]

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")
 \f
 ;; 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)))
 \f
 (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

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

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: sbaugh@catern.com,  66326@debbugs.gnu.org
> Date: Thu, 19 Oct 2023 12:15:52 -0400
> 
> > 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?

Signaling errors activates a complex mechanism, which unwinds the Lisp
machine stack to top-level and displays various kinds of information.
Being able to do this requires some basic setup of the Lisp machine
and redisplay, and that doesn't happen miraculously, it is the result
of running some bootstrap-like code during startup.  If you signal an
error before this stuff is ready, or try to display something before
Emacs is ready for that, you can cause a segfault or an abort.

That is all I can afford saying; if you want more details, you will
have to study the code, where you will see, for example, that
sometimes some of the display routines return without doing anything
if they detect that the necessary infrastructure is not available.
I'm trying to prevent you from having to study the code, by asking you
not to enable any such dangerous behavior by default, but if you
insist on understanding every single detail, you will have to dive
into the code, because I cannot afford posting detailed lectures about
that -- it will take too much of my scarce free time.

> 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.

I _am_ keeping an open mind.  If I didn't, I would just reject your
proposal from the get-go, because I have reasons to believe that its
risks are way greater than its potential usefulness to Emacs users at
large.

More generally, I'm worried by the tendency of people to submit
patches for Emacs as soon as they find something they think is missing
in Emacs that they need for some one-time job.  Emacs is not supposed
to be a huge heap of random features that someone at some time found
useful for some random job.  Emacs is Free Software: you can easily
implement this stuff in your local copy and use it as much as you
want; no need to add that to the Emacs core, unless there's a real
need for it expressed by enough people, and no better solutions in
hand.  I do this for my local jobs all the time.

So I would be much happier if, instead of dumping a patch on us,
people would first describe the issue and the idea on emacs-devel and
ask if anyone else had those problems and likes the solution.  Posting
patches is a kind of pressure on the maintainers: it is harder for the
maintainers to reject a contribution than to explain why a problem is
not important enough to be solved in core or an idea for its solution
is not the best one.

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

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> More generally, I'm worried by the tendency of people to submit
> patches for Emacs as soon as they find something they think is missing
> in Emacs that they need for some one-time job.  Emacs is not supposed
> to be a huge heap of random features that someone at some time found
> useful for some random job.  Emacs is Free Software: you can easily
> implement this stuff in your local copy and use it as much as you
> want; no need to add that to the Emacs core, unless there's a real
> need for it expressed by enough people, and no better solutions in
> hand.  I do this for my local jobs all the time.

FWIW, I'm generally always in favor of features that make the Emacs Lisp
developers' lives easier, and as much as I appreciate that some users
might want to debug warnings, I don't think I understand the use case(s).

These questions come to mind:

- Are there many warnings during startup?
- Why are they a problem (can't they just be ignored)?
- If some code warns, presumably it's a real issue and should be fixed
  by the user, not in the code.  Otherwise it should be an error.  No?
- Why can't users add an advice to `display-warning' that simply calls
  `error' or somesuch?  Or even just redefine it?

Perhaps this feature would be better as a GNU ELPA package
`debug-warnings' or something like that, so that the community could get
more experience with it.  If it catches on, perhaps we could consider
adding it to core.  Would that way of working make sense?

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

[sbaugh]

Stefan Kangas <stefankangas@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> More generally, I'm worried by the tendency of people to submit
>> patches for Emacs as soon as they find something they think is missing
>> in Emacs that they need for some one-time job.  Emacs is not supposed
>> to be a huge heap of random features that someone at some time found
>> useful for some random job.  Emacs is Free Software: you can easily
>> implement this stuff in your local copy and use it as much as you
>> want; no need to add that to the Emacs core, unless there's a real
>> need for it expressed by enough people, and no better solutions in
>> hand.  I do this for my local jobs all the time.
>
> FWIW, I'm generally always in favor of features that make the Emacs Lisp
> developers' lives easier, and as much as I appreciate that some users
> might want to debug warnings, I don't think I understand the use case(s).

I see, let me try to explain further.  (BTW, before submitting I
discussed the idea of this patch on #emacs on Libera IRC and people
seemed interested and approving, which is why I assumed this would be
uncontroversial.  If I had realized I would have sent it to
emacs-devel.)

I had some warnings and I went to debug them, and I assumed there would
be some kind of debug-on-warning like how there is debug-on-error and
debug-on-message and debug-on-quit.  But there is not.  So I figured it
would be uncontroversial to add something which enables a
debug-on-warning, since right now it is difficult to find out what code
triggered some warning.

The alternative which I've been using is
(debug-on-entry 'display-warning)
but this:
1. always debugs regardless of what kind of warning
2. isn't helpful for the "automated tests where warnings
   should fail the test" use case

> These questions come to mind:
>
> - Are there many warnings during startup?

No.  But also it's not just startup.

> - Why are they a problem (can't they just be ignored)?

Well, sure, but they pop up a *Warnings* buffer which is annoying to
users, so it would be better to get rid of them.

> - If some code warns, presumably it's a real issue and should be fixed
>   by the user, not in the code.  Otherwise it should be an error.  No?

Yes, but sometimes either:

- a warning happens because of bugs in other code, so the proper way to
  fix the warning is to fix those other bugs

- More relevantly, sometimes it's hard to find out what is causing the
  warning.

> - Why can't users add an advice to `display-warning' that simply calls
>   `error' or somesuch?  Or even just redefine it?

That would work.  But it's harder to tell a user to do that than (setq
debug-on-warning t) or some equivalent.

> Perhaps this feature would be better as a GNU ELPA package
> `debug-warnings' or something like that, so that the community could get
> more experience with it.  If it catches on, perhaps we could consider
> adding it to core.  Would that way of working make sense?

Telling one of my users to install some other package to debug an issue
is a bit much.  I would probably just guide them through using
debug-on-entry instead.

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

[Spencer Baugh]

sbaugh@catern.com writes:
> Stefan Kangas <stefankangas@gmail.com> writes:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>> More generally, I'm worried by the tendency of people to submit
>>> patches for Emacs as soon as they find something they think is missing
>>> in Emacs that they need for some one-time job.  Emacs is not supposed
>>> to be a huge heap of random features that someone at some time found
>>> useful for some random job.  Emacs is Free Software: you can easily
>>> implement this stuff in your local copy and use it as much as you
>>> want; no need to add that to the Emacs core, unless there's a real
>>> need for it expressed by enough people, and no better solutions in
>>> hand.  I do this for my local jobs all the time.
>>
>> FWIW, I'm generally always in favor of features that make the Emacs Lisp
>> developers' lives easier, and as much as I appreciate that some users
>> might want to debug warnings, I don't think I understand the use case(s).
>
> I see, let me try to explain further.  (BTW, before submitting I
> discussed the idea of this patch on #emacs on Libera IRC and people
> seemed interested and approving, which is why I assumed this would be
> uncontroversial.  If I had realized I would have sent it to
> emacs-devel.)
>
> I had some warnings and I went to debug them, and I assumed there would
> be some kind of debug-on-warning like how there is debug-on-error and
> debug-on-message and debug-on-quit.  But there is not.  So I figured it
> would be uncontroversial to add something which enables a
> debug-on-warning, since right now it is difficult to find out what code
> triggered some warning.
>
> The alternative which I've been using is
> (debug-on-entry 'display-warning)
> but this:
> 1. always debugs regardless of what kind of warning
> 2. isn't helpful for the "automated tests where warnings
>    should fail the test" use case

I just ran into a situation like this yet again.  Again, there was a
stray warning on startup (something about "Symbol’s value as variable is
void", definitely concerning), which the warning itself didn't provide
sufficient information to track down.  I did my usual (debug-on-entry
'display-warning)...

...but then I ran into the further issue that this Emacs had the
"editorconfig" package installed, which apparently calls display-warning
at :debug level on every buffer switch.  So I spent a while hitting c in
the debugger.

I would have like to be able to turn only :error level warnings
into... errors.  Which is what this feature would provide.

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

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: Stefan Kangas <stefankangas@gmail.com>,  Eli Zaretskii <eliz@gnu.org>,
>    66326@debbugs.gnu.org
> Date: Fri, 10 Nov 2023 16:40:14 -0500
> 
> I just ran into a situation like this yet again.  Again, there was a
> stray warning on startup (something about "Symbol’s value as variable is
> void", definitely concerning)

The text you quote is not a warning, it's an error.  --debug-init
should have taken care of debugging it, AFAIU.

And let me reiterate what I said previously (and you quoted here):

>>> More generally, I'm worried by the tendency of people to submit
>>> patches for Emacs as soon as they find something they think is missing
>>> in Emacs that they need for some one-time job.  Emacs is not supposed
>>> to be a huge heap of random features that someone at some time found
>>> useful for some random job.  Emacs is Free Software: you can easily
>>> implement this stuff in your local copy and use it as much as you
>>> want; no need to add that to the Emacs core, unless there's a real
>>> need for it expressed by enough people, and no better solutions in
>>> hand.  I do this for my local jobs all the time.

I still don't think I see why this particular issue should be
everyone's problem, not just something you solve locally, if you
indeed bump frequently into these issues.

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: Stefan Kangas <stefankangas@gmail.com>,  Eli Zaretskii <eliz@gnu.org>,
>>    66326@debbugs.gnu.org
>> Date: Fri, 10 Nov 2023 16:40:14 -0500
>> 
>> I just ran into a situation like this yet again.  Again, there was a
>> stray warning on startup (something about "Symbol’s value as variable is
>> void", definitely concerning)
>
> The text you quote is not a warning, it's an error.  --debug-init
> should have taken care of debugging it, AFAIU.

I know what the difference between an error and a warning is :) The
reason this text, which usually appears in errors, showed up as a
warning, was because some random code caught the error and turned it
into a warning.

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

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@catern.com>
> Date: Sat, 11 Nov 2023 14:37:03 +0000 (UTC)
> Cc: Spencer Baugh <sbaugh@janestreet.com>, 66326@debbugs.gnu.org,
> 	stefankangas@gmail.com
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >> From: Spencer Baugh <sbaugh@janestreet.com>
> >> Cc: Stefan Kangas <stefankangas@gmail.com>,  Eli Zaretskii <eliz@gnu.org>,
> >>    66326@debbugs.gnu.org
> >> Date: Fri, 10 Nov 2023 16:40:14 -0500
> >> 
> >> I just ran into a situation like this yet again.  Again, there was a
> >> stray warning on startup (something about "Symbol’s value as variable is
> >> void", definitely concerning)
> >
> > The text you quote is not a warning, it's an error.  --debug-init
> > should have taken care of debugging it, AFAIU.
> 
> I know what the difference between an error and a warning is :) The
> reason this text, which usually appears in errors, showed up as a
> warning, was because some random code caught the error and turned it
> into a warning.

Then you need to find that code, and Bob's your uncle.

I see no need to add complications to Emacs based on such use cases.
We cannot be expected to have a ready solution for every possible
Emacs-related calamity.