emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: master 11860f8: * test/src/comp-tests.el: Eliminate byte-compiler wa


From: Stephen Gildea
Subject: Re: master 11860f8: * test/src/comp-tests.el: Eliminate byte-compiler warnings (Bug#52105).
Date: Fri, 26 Nov 2021 20:24:57 -0800

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

>   > +(eval-when-compile
>   > +  (require 'cl-lib)
>   > +  (require 'comp))
>   > +(eval-and-compile
>   > +  (require 'comp-cstr)          ;in eval-and-compile for its defstruct
>   > +  (defconst comp-test-src (ert-resource-file "comp-test-funcs.el"))
>   > +  (defconst comp-test-dyn-src (ert-resource-file 
> "comp-test-funcs-dyn.el"))
>   > +  (defconst comp-test-pure-src (ert-resource-file "comp-test-pure.el"))
>   > +  (defconst comp-test-45603-src (ert-resource-file "comp-test-45603.el"))
>   > +  ;; Load the test code here so the compiler can check the function
>   > +  ;; names used in this file.
>   > +  (load comp-test-src nil t)
>   > +  (load comp-test-dyn-src nil t)
>   > +  (load comp-test-pure-src nil t)
>   > +  (load comp-test-45603-src nil t))
>   
>   This looks pretty ugly, in my book.
>   
>   Were the warnings false positives or were they diagnosing real problems?

False positives.  comp-tests.el compiles some test functions, then
examines the result, referring to the test functions by name.  The
test functions are in resource files, so the compiler thinks they are
undefined (and emits warnings) unless the functions are loaded when
comp-tests.el is compiled.

>   The cleanup could start by moving the (require 'comp-cstr) outside of
>   the `eval-and-compile` since toplevel `require`s are processed both at
>   compile and run time anyway.

I didn't know that.  So the "(eval-when-compile (require 'cl-lib))" is
wrong, too?  Here and in hundreds of other Emacs Lisp files?

The test functions here could be loaded with "require" instead of
"load", and then they could be moved outside of the "eval-and-compile"
form.  Would that be more elegant?

We could go even further and eliminate the "defconst" forms, calling
"ert-resource-file" twice for each file (once here for the "require",
and once below for the "native-compile").  That rewrite would
eliminate the ugly "eval-and-compile" entirely.

 < Stephen



reply via email to

[Prev in Thread] Current Thread [Next in Thread]