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.
This commit is contained in:
Mattias Engdegård 2023-03-29 13:21:26 +02:00
parent 7177393826
commit 9c31ee4686
3 changed files with 26 additions and 0 deletions

View file

@ -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,

View file

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

View file

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