unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Zero byte-compiler warnings in the test suite
@ 2021-12-21 12:36 Stefan Kangas
  2021-12-21 13:09 ` Lars Ingebrigtsen
  2021-12-21 14:54 ` Zero byte-compiler warnings in the test suite Eli Zaretskii
  0 siblings, 2 replies; 12+ messages in thread
From: Stefan Kangas @ 2021-12-21 12:36 UTC (permalink / raw)
  To: emacs-devel

Dear emacs-devel,

I would like to bring to your attention that we now have a test suite
with no byte-compiler warnings.  This is nice, as there is less noise,
and it will be easier to spot real problems.

In my use, I'm happy that I can now look at a soothing zero in the mode
line of my *Compilation* buffer when saying "make && make check".  ;-)

Thanks to everyone who helped with this.



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

* Re: Zero byte-compiler warnings in the test suite
  2021-12-21 12:36 Zero byte-compiler warnings in the test suite Stefan Kangas
@ 2021-12-21 13:09 ` Lars Ingebrigtsen
  2021-12-21 13:39   ` Stefan Kangas
  2021-12-21 14:54 ` Zero byte-compiler warnings in the test suite Eli Zaretskii
  1 sibling, 1 reply; 12+ messages in thread
From: Lars Ingebrigtsen @ 2021-12-21 13:09 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel

Stefan Kangas <stefan@marxist.se> writes:

> In my use, I'm happy that I can now look at a soothing zero in the mode
> line of my *Compilation* buffer when saying "make && make check".  ;-)

That's great, and it's indeed much more soothing.  😀

But with a native-comp build (at least), I'm getting two
warnings/errors on the current trunk: 

In toplevel form:
lisp/emacs-lisp/eieio-tests/eieio-test-methodinvoke.el:273:1: Warning: ‘defgeneric’ is an obsolete macro (as of 25.1); use ‘cl-defgeneric’ instead.

and

Compiler-macro error for cl-typep: (error "Unknown type notatype")
Compiler-macro error for cl-typep: (error "Unknown type notatype")
Compiler-macro error for cl-typep: (error "Unknown type notatype")
Compiler-macro error for cl-typep: (error "Unknown type notatype")
Compiler-macro error for cl-typep: (error "Unknown type notatype")

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Zero byte-compiler warnings in the test suite
  2021-12-21 13:09 ` Lars Ingebrigtsen
@ 2021-12-21 13:39   ` Stefan Kangas
  2021-12-21 13:49     ` Lars Ingebrigtsen
  2021-12-21 15:41     ` Stefan Monnier
  0 siblings, 2 replies; 12+ messages in thread
From: Stefan Kangas @ 2021-12-21 13:39 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: emacs-devel

Lars Ingebrigtsen <larsi@gnus.org> writes:

> But with a native-comp build (at least), I'm getting two
> warnings/errors on the current trunk:
>
> In toplevel form:
> lisp/emacs-lisp/eieio-tests/eieio-test-methodinvoke.el:273:1: Warning: ‘defgeneric’ is an obsolete macro (as of 25.1); use ‘cl-defgeneric’ instead.

Strange, I don't see that here (with native-comp).  Does this help?

diff --git a/test/lisp/emacs-lisp/eieio-tests/eieio-test-methodinvoke.el
b/test/lisp/emacs-lisp/eieio-tests/eieio-test-methodinvoke.el
index e881e46a2d..bc89f23b58 100644
--- a/test/lisp/emacs-lisp/eieio-tests/eieio-test-methodinvoke.el
+++ b/test/lisp/emacs-lisp/eieio-tests/eieio-test-methodinvoke.el
@@ -271,6 +271,7 @@ E-base2
 (defclass E (E-base1 E-base2) () :method-invocation-order :breadth-first)

 (with-suppressed-warnings ((obsolete defmethod)
+                           (obsolete defgeneric)
                            (obsolete next-method-p)
                            (obsolete call-next-method))
   (defmethod eitest-F ((_p E))

> Compiler-macro error for cl-typep: (error "Unknown type notatype")
> Compiler-macro error for cl-typep: (error "Unknown type notatype")
> Compiler-macro error for cl-typep: (error "Unknown type notatype")
> Compiler-macro error for cl-typep: (error "Unknown type notatype")
> Compiler-macro error for cl-typep: (error "Unknown type notatype")

I've seen these as well, but they weren't counted by compilation-mode so
I just sort of ignored them.  It would be good to fix them as well of
course.

AFAICT, they come from here:

  ELC+ELN  lisp/emacs-lisp/pcase-tests.elc
Compiler-macro error for cl-typep: (error "Unknown type notatype")
Compiler-macro error for cl-typep: (error "Unknown type notatype")
Compiler-macro error for cl-typep: (error "Unknown type notatype")
Compiler-macro error for cl-typep: (error "Unknown type notatype")
Compiler-macro error for cl-typep: (error "Unknown type notatype")
  GEN      lisp/emacs-lisp/pcase-tests.log

Grepping for notatype leads me to:

./lisp/emacs-lisp/pcase-tests.el:111:                  ((cl-type
notatype) 'integer))))

Does anyone know what is going on here?



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

* Re: Zero byte-compiler warnings in the test suite
  2021-12-21 13:39   ` Stefan Kangas
@ 2021-12-21 13:49     ` Lars Ingebrigtsen
  2021-12-21 15:11       ` Stefan Kangas
  2021-12-21 15:41     ` Stefan Monnier
  1 sibling, 1 reply; 12+ messages in thread
From: Lars Ingebrigtsen @ 2021-12-21 13:49 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel

Stefan Kangas <stefan@marxist.se> writes:

> Strange, I don't see that here (with native-comp).  Does this help?

[...]

>  (with-suppressed-warnings ((obsolete defmethod)
> +                           (obsolete defgeneric)

Yup, that fixes it.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Zero byte-compiler warnings in the test suite
  2021-12-21 12:36 Zero byte-compiler warnings in the test suite Stefan Kangas
  2021-12-21 13:09 ` Lars Ingebrigtsen
@ 2021-12-21 14:54 ` Eli Zaretskii
  1 sibling, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2021-12-21 14:54 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 21 Dec 2021 04:36:51 -0800
> 
> Dear emacs-devel,
> 
> I would like to bring to your attention that we now have a test suite
> with no byte-compiler warnings.

Thanks to all those who work relentlessly on making the tests
warning-free.



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

* Re: Zero byte-compiler warnings in the test suite
  2021-12-21 13:49     ` Lars Ingebrigtsen
@ 2021-12-21 15:11       ` Stefan Kangas
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Kangas @ 2021-12-21 15:11 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: emacs-devel

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Yup, that fixes it.

Thanks, pushed to master as commit 1a923e5ac6.



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

* Re: Zero byte-compiler warnings in the test suite
  2021-12-21 13:39   ` Stefan Kangas
  2021-12-21 13:49     ` Lars Ingebrigtsen
@ 2021-12-21 15:41     ` Stefan Monnier
  2021-12-22  5:07       ` Stefan Kangas
  1 sibling, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2021-12-21 15:41 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Lars Ingebrigtsen, emacs-devel

> Compiler-macro error for cl-typep: (error "Unknown type notatype")
>   GEN      lisp/emacs-lisp/pcase-tests.log
>
> Grepping for notatype leads me to:
>
> ./lisp/emacs-lisp/pcase-tests.el:111:                  ((cl-type
> notatype) 'integer))))
>
> Does anyone know what is going on here?

I thought it was pretty obvious: `c-type` takes a type as argument, but
`notatype` is ... not declared as a type, so we get an error.

The test expects the error to be signal'd at run time whereas the
compiler macro prefers to signal the error right away.
As a bias observer, I'd suggest to fix the test rather than the compiler
macro ;-)


        Stefan




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

* Re: Zero byte-compiler warnings in the test suite
  2021-12-21 15:41     ` Stefan Monnier
@ 2021-12-22  5:07       ` Stefan Kangas
  2021-12-22 17:40         ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Kangas @ 2021-12-22  5:07 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Lars Ingebrigtsen, emacs-devel

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

> The test expects the error to be signal'd at run time whereas the
> compiler macro prefers to signal the error right away.
> As a bias observer, I'd suggest to fix the test rather than the compiler
> macro ;-)

How do we usually test compiler macros?  I guess we should create a
separate file and try to byte-compile it?

Or should we just disable the test in non-interactive sessions?



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

* Re: Zero byte-compiler warnings in the test suite
  2021-12-22  5:07       ` Stefan Kangas
@ 2021-12-22 17:40         ` Stefan Monnier
  2021-12-22 22:11           ` Stefan Kangas
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2021-12-22 17:40 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Lars Ingebrigtsen, emacs-devel

>> The test expects the error to be signal'd at run time whereas the
>> compiler macro prefers to signal the error right away.
>> As a bias observer, I'd suggest to fix the test rather than the compiler
>> macro ;-)
>
> How do we usually test compiler macros?  I guess we should create a
> separate file and try to byte-compile it?

I'd do something like

    (should-error
     (macroexpand-all '(pcase ...)))


-- Stefan




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

* Re: Zero byte-compiler warnings in the test suite
  2021-12-22 17:40         ` Stefan Monnier
@ 2021-12-22 22:11           ` Stefan Kangas
  2021-12-23  0:07             ` Weird non-caught error in ERT (was: Zero byte-compiler warnings in the test suite) Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Kangas @ 2021-12-22 22:11 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Lars Ingebrigtsen, emacs-devel

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

>> How do we usually test compiler macros?  I guess we should create a
>> separate file and try to byte-compile it?
>
> I'd do something like
>
>     (should-error
>      (macroexpand-all '(pcase ...)))

macroexpand-all didn't work, but I installed this, which seems to have
fixed it:

diff --git a/test/lisp/emacs-lisp/pcase-tests.el
b/test/lisp/emacs-lisp/pcase-tests.el
index 7ad01e7aef..40ae9809b5 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -107,8 +107,11 @@ pcase-tests-cl-type
   (should (equal (pcase 1
                    ((cl-type (integer 0 2)) 'integer-0<=n<=2))
                  'integer-0<=n<=2))
-  (should-error (pcase 1
-                  ((cl-type notatype) 'integer))))
+  (should-error
+   ;; Avoid error at compile time due to compiler macro.
+   (eval '(pcase 1
+            ((cl-type notatype) 'integer))
+         t)))

 (ert-deftest pcase-tests-setq ()
   (should (equal (let (a b)



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

* Weird non-caught error in ERT (was: Zero byte-compiler warnings in the test suite)
  2021-12-22 22:11           ` Stefan Kangas
@ 2021-12-23  0:07             ` Stefan Monnier
  2021-12-23 10:04               ` Philipp Stephani
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2021-12-23  0:07 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Lars Ingebrigtsen, emacs-devel, Christian Ohler

>> I'd do something like
>>
>>     (should-error
>>      (macroexpand-all '(pcase ...)))
>
> macroexpand-all didn't work, but I installed this, which seems to have
> fixed it:

Indeed, how odd.  I tried:

    (should
     (condition-case err
         (progn
           (macroexpand-all
            '(pcase 1
               ((cl-type notatype) 'integer)))
           nil)
       (error (string-match "Unknown type notabug" (cadr err))))))

and that gives me:

    Test pcase-tests-cl-type backtrace:
      error("Unknown type %S" notatype)
      cl-typep--inliner((cl-typep 1 'notatype) 1 'notatype)
      apply(cl-typep--inliner (cl-typep 1 'notatype) (1 'notatype))
      macroexp--compiler-macro(cl-typep--inliner (cl-typep 1 'notatype))
      macroexp--expand-all((pcase--flip cl-typep 'notatype 1))
      macroexp--all-forms((if (pcase--flip cl-typep 'notatype 1) (let nil 
      macroexp--expand-all((pcase 1 ((cl-type notatype) 'integer)))
      macroexpand-all((pcase 1 ((cl-type notatype) 'integer)))
      (progn (macroexpand-all '(pcase 1 ((cl-type notatype) 'integer))) ni
      (condition-case err (progn (macroexpand-all '(pcase 1 ((cl-type nota
    [...]

Notice the place where I truncated the stack: we're inside the
`condition-case`!  So why do we get a backtrace instead of letting
`condition-case` catch the error?


        Stefan




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

* Re: Weird non-caught error in ERT (was: Zero byte-compiler warnings in the test suite)
  2021-12-23  0:07             ` Weird non-caught error in ERT (was: Zero byte-compiler warnings in the test suite) Stefan Monnier
@ 2021-12-23 10:04               ` Philipp Stephani
  0 siblings, 0 replies; 12+ messages in thread
From: Philipp Stephani @ 2021-12-23 10:04 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Lars Ingebrigtsen, Christian Ohler, Stefan Kangas,
	Emacs developers

Am Do., 23. Dez. 2021 um 01:07 Uhr schrieb Stefan Monnier
<monnier@iro.umontreal.ca>:
>
> >> I'd do something like
> >>
> >>     (should-error
> >>      (macroexpand-all '(pcase ...)))
> >
> > macroexpand-all didn't work, but I installed this, which seems to have
> > fixed it:
>
> Indeed, how odd.  I tried:
>
>     (should
>      (condition-case err
>          (progn
>            (macroexpand-all
>             '(pcase 1
>                ((cl-type notatype) 'integer)))
>            nil)
>        (error (string-match "Unknown type notabug" (cadr err))))))
>
> and that gives me:
>
>     Test pcase-tests-cl-type backtrace:
>       error("Unknown type %S" notatype)
>       cl-typep--inliner((cl-typep 1 'notatype) 1 'notatype)
>       apply(cl-typep--inliner (cl-typep 1 'notatype) (1 'notatype))
>       macroexp--compiler-macro(cl-typep--inliner (cl-typep 1 'notatype))
>       macroexp--expand-all((pcase--flip cl-typep 'notatype 1))
>       macroexp--all-forms((if (pcase--flip cl-typep 'notatype 1) (let nil
>       macroexp--expand-all((pcase 1 ((cl-type notatype) 'integer)))
>       macroexpand-all((pcase 1 ((cl-type notatype) 'integer)))
>       (progn (macroexpand-all '(pcase 1 ((cl-type notatype) 'integer))) ni
>       (condition-case err (progn (macroexpand-all '(pcase 1 ((cl-type nota
>     [...]
>
> Notice the place where I truncated the stack: we're inside the
> `condition-case`!  So why do we get a backtrace instead of letting
> `condition-case` catch the error?
>

Maybe because `should' installs a signal hook function?



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

end of thread, other threads:[~2021-12-23 10:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-21 12:36 Zero byte-compiler warnings in the test suite Stefan Kangas
2021-12-21 13:09 ` Lars Ingebrigtsen
2021-12-21 13:39   ` Stefan Kangas
2021-12-21 13:49     ` Lars Ingebrigtsen
2021-12-21 15:11       ` Stefan Kangas
2021-12-21 15:41     ` Stefan Monnier
2021-12-22  5:07       ` Stefan Kangas
2021-12-22 17:40         ` Stefan Monnier
2021-12-22 22:11           ` Stefan Kangas
2021-12-23  0:07             ` Weird non-caught error in ERT (was: Zero byte-compiler warnings in the test suite) Stefan Monnier
2021-12-23 10:04               ` Philipp Stephani
2021-12-21 14:54 ` Zero byte-compiler warnings in the test suite Eli Zaretskii

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