Re: master 1559cc4: Fix missing file&line info in "Unknown defun property" warnings

[Basil L. Contovounesios]

monnier@iro.umontreal.ca (Stefan Monnier) writes:

> branch: master
> commit 1559cc445a306b61b2a47c710e049ea26fe5265d
> Author: Stefan Monnier <monnier@iro.umontreal.ca>
> Commit: Stefan Monnier <monnier@iro.umontreal.ca>
>
>     Fix missing file&line info in "Unknown defun property" warnings
>     
>     * lisp/emacs-lisp/byte-run.el (defmacro, defun): Use
>     `macroexp--warn-and-return` rather than `message`.
>     
>     * lisp/emacs-lisp/macroexp.el: Fix `macroexp--compiling-p`.
>     (macroexp--warn-and-return): Don't try and detect repetition on forms
>     like `nil`.
>     (macroexp-macroexpand): Don't forget to bind 
> `macroexpand-all-environment`.

This gave rise to the following 'make check' failures:

--8<---------------cut here---------------start------------->8---
Test cl-lib-defstruct-record passed unexpectedly
Ran 47 tests, 46 results as expected, 1 unexpected (2021-01-23 22:05:59+0000, 
0.052134 sec)
1 expected failures
1 unexpected results:
   PASSED  cl-lib-defstruct-record

lisp/emacs-lisp/seq-tests.el:416:41: Error: ‘add-to-list’ can’t use lexical
    var ‘elts’; use ‘push’ or ‘cl-pushnew’

In toplevel form:
lisp/autorevert-tests.el:642:24: Error: ‘add-to-list’ can’t use lexical var
    ‘buffers’; use ‘push’ or ‘cl-pushnew’
lisp/autorevert-tests.el:657:15: Error: ‘add-to-list’ can’t use lexical var
    ‘buffers’; use ‘push’ or ‘cl-pushnew’
--8<---------------cut here---------------end--------------->8---

Are the attached changes TRT, or is there something subtle going on?

Thanks,

-- 
Basil

diff --git a/test/lisp/autorevert-tests.el b/test/lisp/autorevert-tests.el
index 683e3ea30d..19d9f7b13f 100644
--- a/test/lisp/autorevert-tests.el
+++ b/test/lisp/autorevert-tests.el
@@ -609,11 +609,12 @@ auto-revert-test07-auto-revert-several-buffers
              (should auto-revert-mode))
 
            (dotimes (i num-buffers)
-             (add-to-list
-              'buffers
-              (make-indirect-buffer
-               (car buffers) (format "%s-%d" (buffer-file-name (car buffers)) 
i) 'clone)
-              'append))
+             (let ((buf (make-indirect-buffer
+                         (car buffers)
+                         (format "%s-%d" (buffer-file-name (car buffers)) i)
+                         'clone)))
+               (unless (memq buf buffers)
+                 (setq buffers (nconc buffers (list buf))))))
            (dolist (buf buffers)
              (with-current-buffer buf
                (should (string-equal (buffer-string) "any text"))
@@ -640,10 +641,10 @@ auto-revert-test07-auto-revert-several-buffers
            (auto-revert-tests--write-file "any text" tmpfile (pop times))
 
            (dotimes (i num-buffers)
-             (add-to-list
-              'buffers
-              (generate-new-buffer (format "%s-%d" (file-name-nondirectory 
tmpfile) i))
-              'append))
+             (let ((buf (generate-new-buffer
+                         (format "%s-%d" (file-name-nondirectory tmpfile) i))))
+               (unless (memq buf buffers)
+                 (setq buffers (nconc buffers (list buf))))))
            (dolist (buf buffers)
              (with-current-buffer buf
                (insert-file-contents tmpfile 'visit)
diff --git a/test/lisp/emacs-lisp/cl-lib-tests.el 
b/test/lisp/emacs-lisp/cl-lib-tests.el
index 97a44c43ef..888af1b80a 100644
--- a/test/lisp/emacs-lisp/cl-lib-tests.el
+++ b/test/lisp/emacs-lisp/cl-lib-tests.el
@@ -548,10 +548,10 @@ cl-lib-tests--dummy-function
   t)
 
 (ert-deftest cl-lib-defstruct-record ()
-  ;; This test fails when compiled, see Bug#24402/27718.
-  :expected-result (if (byte-code-function-p
-                        (symbol-function 'cl-lib-tests--dummy-function))
-                       :failed :passed)
+  ;; This test used to fail when compiled, see Bug#24402/27718.
+  ;; :expected-result (if (byte-code-function-p
+  ;;                       (symbol-function 'cl-lib-tests--dummy-function))
+  ;;                      :failed :passed)
   (cl-defstruct foo x)
   (let ((x (make-foo :x 42)))
     (should (recordp x))
@@ -566,6 +566,7 @@ old-struct
     (should (eq (type-of x) 'vector))
 
     (cl-old-struct-compat-mode 1)
+    (defvar cl-struct-foo)
     (let ((cl-struct-foo (cl--struct-get-class 'foo)))
       (setf (symbol-function 'cl-struct-foo) :quick-object-witness-check)
       (should (eq (type-of x) 'foo))
diff --git a/test/lisp/emacs-lisp/seq-tests.el 
b/test/lisp/emacs-lisp/seq-tests.el
index 670398354a..f1dfc24b3f 100644
--- a/test/lisp/emacs-lisp/seq-tests.el
+++ b/test/lisp/emacs-lisp/seq-tests.el
@@ -108,16 +108,12 @@ test-seq-map-indexed
                  '((a 0) (b 1) (c 2) (d 3)))))
 
 (ert-deftest test-seq-do-indexed ()
-  (let ((result nil))
-    (seq-do-indexed (lambda (elt i)
-                      (add-to-list 'result (list elt i)))
-                    nil)
-    (should (equal result nil)))
+  (let (result)
+    (seq-do-indexed (lambda (elt i) (push (list elt i) result)) ())
+    (should-not result))
   (with-test-sequences (seq '(4 5 6))
-    (let ((result nil))
-      (seq-do-indexed (lambda (elt i)
-                        (add-to-list 'result (list elt i)))
-                      seq)
+    (let (result)
+      (seq-do-indexed (lambda (elt i) (push (list elt i) result)) seq)
       (should (equal (seq-elt result 0) '(6 2)))
       (should (equal (seq-elt result 1) '(5 1)))
       (should (equal (seq-elt result 2) '(4 0))))))
@@ -410,12 +406,11 @@ test-seq-sort-by
 
 (ert-deftest test-seq-random-elt-take-all ()
   (let ((seq '(a b c d e))
-        (elts '()))
-    (should (= 0 (length elts)))
+        elts)
     (dotimes (_ 1000)
       (let ((random-elt (seq-random-elt seq)))
-        (add-to-list 'elts
-                     random-elt)))
+        (or (memq random-elt elts)
+            (push random-elt elts))))
     (should (= 5 (length elts)))))
 
 (ert-deftest test-seq-random-elt-signal-on-empty ()