Add byte-compiler warning about useless trailing cond clauses

Warn about clauses after the default clause, as in

    (cond ((= x 0) (say "none"))
          (t (say "some"))
          (say "goodbye"))

because they are very much an indicator of a mistake (such as
misplaced brackets), and since they are deleted by the optimiser, any
other warnings there are lost and the user wouldn't know that
something is wrong otherwise.

* lisp/emacs-lisp/macroexp.el (macroexp--expand-all): Add warning.
* etc/NEWS: Announce.
* test/lisp/emacs-lisp/bytecomp-tests.el
(bytecomp-test--with-suppressed-warnings): Add test case.

etc/NEWS

@@ -1084,6 +1084,21 @@ simplified away.
 This warning can be suppressed using 'with-suppressed-warnings' with
 the warning name 'suspicious'.
 
+---
+*** Warn about useless trailing 'cond' clauses.
+The compiler now warns when a 'cond' form contains clauses following a
+default (unconditional) clause.  Example:
+
+    (cond ((= x 0) (say "none"))
+          (t (say "some"))
+          (say "goodbye"))
+
+Such a clause will never be executed but is likely to be a mistake,
+perhaps due to misplaced brackets.
+
+This warning can be suppressed using 'with-suppressed-warnings' with
+the warning name 'suspicious'.
+
 ---
 *** Warn about mutation of constant values.
 The compiler now warns about code that modifies program constants in

lisp/emacs-lisp/macroexp.el

@@ -330,7 +330,34 @@ Assumes the caller has bound `macroexpand-all-environment'."
         (let ((fn (car-safe form)))
           (pcase form
             (`(cond . ,clauses)
-             (macroexp--cons fn (macroexp--all-clauses clauses) form))
+             ;; Check for rubbish clauses at the end before macro-expansion,
+             ;; to avoid nuisance warnings from clauses that become
+             ;; unconditional through that process.
+             ;; FIXME: this strategy is defeated by forced `macroexpand-all',
+             ;; such as in `cl-flet'.  Haven't seen that in the wild, though.
+             (let ((default-tail nil)
+                   (n 0)
+                   (rest clauses))
+               (while rest
+                 (let ((c (car-safe (car rest))))
+                   (when (cond ((consp c) (and (memq (car c) '(quote function))
+                                               (cadr c)))
+                               ((symbolp c) (or (eq c t) (keywordp c)))
+                               (t t))
+                     ;; This is unquestionably a default clause.
+                     (setq default-tail (cdr rest))
+                     (setq clauses (take (1+ n) clauses))  ; trim the tail
+                     (setq rest nil)))
+                 (setq n (1+ n))
+                 (setq rest (cdr rest)))
+               (let ((expanded-form
+                      (macroexp--cons fn (macroexp--all-clauses clauses) form)))
+                 (if default-tail
+                     (macroexp-warn-and-return
+                      (format-message
+                       "Useless clause following default `cond' clause")
+                      expanded-form '(suspicious cond) t default-tail)
+                   expanded-form))))
             (`(condition-case . ,(or `(,err ,body . ,handlers) pcase--dontcare))
              (let ((exp-body (macroexp--expand-all body)))
                (if handlers

test/lisp/emacs-lisp/bytecomp-tests.el

@@ -1512,6 +1512,15 @@ literals (Bug#20852)."
    '((suspicious unwind-protect))
    "Warning: `unwind-protect' without unwind forms")
 
+  (test-suppression
+   '(defun zot (x)
+      (cond
+       ((zerop x) 'zero)
+       (t 'nonzero)
+       (happy puppy)))
+   '((suspicious cond))
+   "Warning: Useless clause following default `cond' clause")
+
   (test-suppression
    '(defun zot ()
       (let ((_ 1))