unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: master 1559cc4: Fix missing file&line info in "Unknown defun property" warnings
       [not found] ` <20210123210447.45FA120AD1@vcs0.savannah.gnu.org>
@ 2021-01-23 22:20   ` Basil L. Contovounesios
  2021-01-23 22:46     ` Stefan Monnier
  0 siblings, 1 reply; 3+ messages in thread
From: Basil L. Contovounesios @ 2021-01-23 22:20 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: check.diff --]
[-- Type: text/x-diff, Size: 4382 bytes --]

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: master 1559cc4: Fix missing file&line info in "Unknown defun property" warnings
  2021-01-23 22:20   ` master 1559cc4: Fix missing file&line info in "Unknown defun property" warnings Basil L. Contovounesios
@ 2021-01-23 22:46     ` Stefan Monnier
  2021-01-23 23:42       ` Basil L. Contovounesios
  0 siblings, 1 reply; 3+ messages in thread
From: Stefan Monnier @ 2021-01-23 22:46 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: emacs-devel

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

This looks like it will do what the old code did, yes.
We can do better, tho:

- the `make-indirect-buffer` will returna  fresh new buffer, so `memq`
  will always return nil.
- the `nconc` gives an over O(n²) complexity which is not justified.

so I'd recommend

    (dotimes (i num-buffers)
      (push (make-indirect-buffer
             (car buffers)
             (format "%s-%d" (buffer-file-name (car buffers)) i)
             'clone)
            buffers))
    (setq buffer (nreverse buffers))

instead.  I think the same applies to the second loop in autorevert-tests.el.

Also in `test-seq-random-elt-take-all` you can use `cl-pushnew` instead
of the or+memq+push.


        Stefan




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: master 1559cc4: Fix missing file&line info in "Unknown defun property" warnings
  2021-01-23 22:46     ` Stefan Monnier
@ 2021-01-23 23:42       ` Basil L. Contovounesios
  0 siblings, 0 replies; 3+ messages in thread
From: Basil L. Contovounesios @ 2021-01-23 23:42 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> This looks like it will do what the old code did, yes.
> We can do better, tho:

Thanks, I was concerned mostly with cl-lib-defstruct-record which now
passes when byte-compiled.  Pushed with your suggestions.

Fix recently uncovered 'make check' failures
e1902ac618 2021-01-23 23:38:19 +0000
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e1902ac6182b156efaaf93013a707abb4b627765

-- 
Basil



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-23 23:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210123210445.12536.65283@vcs0.savannah.gnu.org>
     [not found] ` <20210123210447.45FA120AD1@vcs0.savannah.gnu.org>
2021-01-23 22:20   ` master 1559cc4: Fix missing file&line info in "Unknown defun property" warnings Basil L. Contovounesios
2021-01-23 22:46     ` Stefan Monnier
2021-01-23 23:42       ` Basil L. Contovounesios

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).