[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
master 9c31ee46861 1/2: Warn about unwind-protect without unwind forms
From: |
Mattias Engdegård |
Subject: |
master 9c31ee46861 1/2: Warn about unwind-protect without unwind forms |
Date: |
Wed, 29 Mar 2023 13:48:23 -0400 (EDT) |
branch: master
commit 9c31ee468618c95959454736d939eb46bc52b19b
Author: Mattias Engdegård <mattiase@acm.org>
Commit: Mattias Engdegård <mattiase@acm.org>
Warn about unwind-protect without unwind forms
`unwind-protect` without unwind forms is not just pointless but often
indicates a mistake where the intended unwind part is misplaced, as in
(unwind-protect (progn PROT-FORMS UNWIND-FORMS)) ; oops
or
(unwind-protect PROT-FORM) UNWIND-FORMS ; also oops
or entirely forgotten for that matter. Warning about this makes
sense, and the warning can always be silenced by removing the
`unwind-protect` altogether if it shouldn't be there in the first
place.
* lisp/emacs-lisp/macroexp.el (macroexp--expand-all): Implement
warning.
* etc/NEWS: Announce.
* test/lisp/emacs-lisp/bytecomp-tests.el
(bytecomp-test--with-suppressed-warnings): Add test case.
---
etc/NEWS | 15 +++++++++++++++
lisp/emacs-lisp/macroexp.el | 5 +++++
test/lisp/emacs-lisp/bytecomp-tests.el | 6 ++++++
3 files changed, 26 insertions(+)
diff --git a/etc/NEWS b/etc/NEWS
index 9e45b1d80b2..f27cafb3d41 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -418,6 +418,21 @@ was to catch all errors, add an explicit handler for
'error', or use
This warning can be suppressed using 'with-suppressed-warnings' with
the warning name 'suspicious'.
+---
+*** Warn about 'unwind-protect' without unwind forms.
+The compiler now warns when the 'unwind-protect' form is used without
+any unwind forms, as in
+
+ (unwind-protect (read buffer))
+
+because the behaviour is identical to that of the argument; there is
+no protection of any kind. Perhaps the intended unwind forms have
+been misplaced or forgotten, or the use of 'unwind-protect' could be
+simplified away.
+
+This warning can be suppressed using 'with-suppressed-warnings' with
+the warning name 'suspicious'.
+
+++
** New function 'file-user-uid'.
This function is like 'user-uid', but is aware of file name handlers,
diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el
index 8cb67c3b8b5..b05aba3e1a7 100644
--- a/lisp/emacs-lisp/macroexp.el
+++ b/lisp/emacs-lisp/macroexp.el
@@ -383,6 +383,11 @@ Assumes the caller has bound
`macroexpand-all-environment'."
(format-message "missing `while' condition")
`(signal 'wrong-number-of-arguments '(while 0))
nil 'compile-only form))
+ (`(unwind-protect ,expr)
+ (macroexp-warn-and-return
+ (format-message "`unwind-protect' without unwind forms")
+ (macroexp--expand-all expr)
+ (list 'suspicious 'unwind-protect) t form))
(`(setq ,(and var (pred symbolp)
(pred (not booleanp)) (pred (not keywordp)))
,expr)
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el
b/test/lisp/emacs-lisp/bytecomp-tests.el
index 2cd4dd75742..5bad1ce41a8 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -1461,6 +1461,12 @@ literals (Bug#20852)."
'((suspicious condition-case))
"Warning: `condition-case' without handlers")
+ (test-suppression
+ '(defun zot (x)
+ (unwind-protect (print x)))
+ '((suspicious unwind-protect))
+ "Warning: `unwind-protect' without unwind forms")
+
(test-suppression
'(defun zot ()
(let ((_ 1))