From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: master 11860f8: * test/src/comp-tests.el: Eliminate byte-compiler warnings (Bug#52105). Date: Sat, 27 Nov 2021 10:11:36 -0500 Message-ID: References: <2586873.1637987097@tigger3.sg.gildea.net> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="23190"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Stephen Gildea Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sat Nov 27 16:12:43 2021 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mqzNa-0005ry-7n for ged-emacs-devel@m.gmane-mx.org; Sat, 27 Nov 2021 16:12:42 +0100 Original-Received: from localhost ([::1]:58714 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mqzNY-0002LD-Bi for ged-emacs-devel@m.gmane-mx.org; Sat, 27 Nov 2021 10:12:40 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:37024) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mqzMf-0001gl-2j for emacs-devel@gnu.org; Sat, 27 Nov 2021 10:11:45 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:55673) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mqzMc-0003pf-EJ for emacs-devel@gnu.org; Sat, 27 Nov 2021 10:11:44 -0500 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 19A881001B8; Sat, 27 Nov 2021 10:11:40 -0500 (EST) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 67A56100040; Sat, 27 Nov 2021 10:11:37 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1638025897; bh=ztgdbKLgf3zCW2Nz0J4BLp/7oIYXN/nU29K+IuN2zlY=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=R2NdN0b1M76sx2y6Iv3qHb+uIDPC8QlPDgrbvlTbBZfABmWJCNRT75L4R3jeAmuIg lpdiynwJaQfaMKHxhygkpINYe3jYzjf4kuXjvYq+w8P42pcWY2eqOVhd5LFGnsc/PJ 77aru0HRK5VN4bqnmxEpVQUoJvZSlzMjbk+4ULzsxZ8jCK/B3OdiPJW0IQ8Hn26yy+ CHXOfmtJ95En+TkROzoVDsnG9TH5I9WM4pAcNUbO0wqXTDWsBsEVlRbDkhnJCX4b9S jNKPIaTyXYluTqGkZ0Gq/ZgMa5yQbpU3+I9AuzzEnQuu82ndN4eJiUGWsKvCZH6/I3 0ak14nTH128IQ== Original-Received: from ceviche (modemcable034.207-20-96.mc.videotron.ca [96.20.207.34]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 450F41207BE; Sat, 27 Nov 2021 10:11:37 -0500 (EST) In-Reply-To: <2586873.1637987097@tigger3.sg.gildea.net> (Stephen Gildea's message of "Fri, 26 Nov 2021 20:24:57 -0800") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:280345 Archived-At: > 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. And I guess there are too many such functions so using `declare-function` is unwieldy. I see. >> 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 purpose of (eval-when-compile (require 'cl-lib)) is to avoid loading cl-lib during run-time (i.e. loading it only at compile-time). So I think it's wrong, indeed, since we use `cl-every` at run-time. > 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? It's not that big of a difference, but if we move it after the (when (native-comp-available-p) (message "Compiling tests...") (load (native-compile comp-test-src)) (load (native-compile comp-test-dyn-src))) then it will save us from loading the files twice. > 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. I think it's better to keep the `eval-and-compile` and avoid the redundancy. So, all in all, I pushed the patch below, thank you. Stefan commit 8d67a70e97a7002682f641c05b10e1a9d4586e8b Author: Stefan Monnier Date: Sat Nov 27 10:10:26 2021 -0500 * test/src/comp-tests.el: Rework last patch Move `require`s out of `eval-when-compile` if the functions are called at run-time. Don't use #' to quote symbols (i.e. at those places where a lambda expression couldn't be used). Don't pre-load comp-test-45603.el and comp-test-pure.el any more. (comp-deftest pure): Use `declare-function` after loading `comp-test-pure.el` to silence the byte-compiler. diff --git a/test/src/comp-tests.el b/test/src/comp-tests.el index f66a193205..5b20cf38ec 100644 --- a/test/src/comp-tests.el +++ b/test/src/comp-tests.el @@ -27,27 +27,24 @@ (require 'ert) (require 'ert-x) -(eval-when-compile - (require 'cl-lib) - (require 'comp)) +(require 'cl-lib) +(require 'comp) +(require 'comp-cstr) + (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)) + (defconst comp-test-dyn-src (ert-resource-file "comp-test-funcs-dyn.el"))) (when (native-comp-available-p) (message "Compiling tests...") (load (native-compile comp-test-src)) (load (native-compile comp-test-dyn-src))) +;; Load the test code here so the compiler can check the function +;; names used in this file. +(require 'comp-test-funcs comp-test-src) +(require 'comp-test-dyn-funcs comp-test-dyn-src) ;Non-standard feature name! + (defmacro comp-deftest (name args &rest docstring-and-body) "Define a test for the native compiler tagging it as :nativecomp." (declare (indent defun) @@ -75,7 +72,7 @@ comp-deftest (copy-file comp-src comp2-src t) (let ((load-no-native t)) (load (concat comp-src "c") nil nil t t)) - (should-not (subr-native-elisp-p (symbol-function #'native-compile))) + (should-not (subr-native-elisp-p (symbol-function 'native-compile))) (message "Compiling stage1...") (let* ((t0 (current-time)) (comp1-eln (native-compile comp1-src))) @@ -372,7 +369,7 @@ comp-deftest t) (native-compile #'comp-tests-free-fun-f) - (should (subr-native-elisp-p (symbol-function #'comp-tests-free-fun-f))) + (should (subr-native-elisp-p (symbol-function 'comp-tests-free-fun-f))) (should (= (comp-tests-free-fun-f) 3)) (should (string= (documentation #'comp-tests-free-fun-f) "Some doc.")) @@ -386,7 +383,7 @@ comp-deftest "Check we are able to compile a single function." (eval '(defun comp-tests/free\fun-f ()) t) (native-compile #'comp-tests/free\fun-f) - (should (subr-native-elisp-p (symbol-function #'comp-tests/free\fun-f)))) + (should (subr-native-elisp-p (symbol-function 'comp-tests/free\fun-f)))) (comp-deftest bug-40187 () "Check function name shadowing. @@ -397,7 +394,7 @@ comp-deftest (comp-deftest speed--1 () "Check that at speed -1 we do not native compile." (should (= (comp-test-speed--1-f) 3)) - (should-not (subr-native-elisp-p (symbol-function #'comp-test-speed--1-f)))) + (should-not (subr-native-elisp-p (symbol-function 'comp-test-speed--1-f)))) (comp-deftest bug-42360 () "." @@ -446,7 +443,7 @@ comp-test-primitive-redefine-args (comp-deftest primitive-redefine () "Test effectiveness of primitive redefinition." (cl-letf ((comp-test-primitive-redefine-args nil) - ((symbol-function #'-) + ((symbol-function '-) (lambda (&rest args) (setq comp-test-primitive-redefine-args args) 'xxx))) @@ -467,11 +464,11 @@ comp-test-primitive-redefine-args (comp-deftest comp-test-defsubst () ;; Bug#42664, Bug#43280, Bug#44209. - (should-not (subr-native-elisp-p (symbol-function #'comp-test-defsubst-f)))) + (should-not (subr-native-elisp-p (symbol-function 'comp-test-defsubst-f)))) (comp-deftest primitive-redefine-compile-44221 () "Test the compiler still works while primitives are redefined (bug#44221)." - (cl-letf (((symbol-function #'delete-region) + (cl-letf (((symbol-function 'delete-region) (lambda (_ _)))) (should (subr-native-elisp-p (native-compile @@ -506,13 +503,13 @@ comp-test-primitive-redefine-args (comp-deftest 45603-1 () "" - (load (native-compile comp-test-45603-src)) - (should (fboundp #'comp-test-45603--file-local-name))) + (load (native-compile (ert-resource-file "comp-test-45603.el"))) + (should (fboundp 'comp-test-45603--file-local-name))) (comp-deftest 46670-1 () "" (should (string= (comp-test-46670-2-f "foo") "foo")) - (should (equal (subr-type (symbol-function #'comp-test-46670-2-f)) + (should (equal (subr-type (symbol-function 'comp-test-46670-2-f)) '(function (t) t)))) (comp-deftest 46824-1 () @@ -742,7 +739,7 @@ comp-test-primitive-redefine-args (comp-deftest dynamic-help-arglist () "Test `help-function-arglist' works on lisp/d (bug#42572)." (should (equal (help-function-arglist - (symbol-function #'comp-tests-ffuncall-callee-opt-rest-dyn-f) + (symbol-function 'comp-tests-ffuncall-callee-opt-rest-dyn-f) t) '(a b &optional c &rest d)))) @@ -815,7 +812,7 @@ comp-tests-tco-checker (comp-tests-tco-f (+ a b) a (- count 1)))) t) (native-compile #'comp-tests-tco-f) - (should (subr-native-elisp-p (symbol-function #'comp-tests-tco-f))) + (should (subr-native-elisp-p (symbol-function 'comp-tests-tco-f))) (should (= (comp-tests-tco-f 1 0 10) 55)))) (defun comp-tests-fw-prop-checker-1 (_) @@ -842,7 +839,7 @@ comp-tests-fw-prop-checker-1 (length c))) ; <= has to optimize t) (native-compile #'comp-tests-fw-prop-1-f) - (should (subr-native-elisp-p (symbol-function #'comp-tests-fw-prop-1-f))) + (should (subr-native-elisp-p (symbol-function 'comp-tests-fw-prop-1-f))) (should (= (comp-tests-fw-prop-1-f) 6)))) (defun comp-tests-check-ret-type-spec (func-form ret-type) @@ -1421,12 +1418,14 @@ comp-tests-pure-checker-2 (let ((native-comp-speed 3) (comp-post-pass-hooks '((comp-final comp-tests-pure-checker-1 comp-tests-pure-checker-2)))) - (load (native-compile comp-test-pure-src)) + (load (native-compile (ert-resource-file "comp-test-pure.el"))) + (declare-function comp-tests-pure-caller-f nil) + (declare-function comp-tests-pure-fibn-entry-f nil) - (should (subr-native-elisp-p (symbol-function #'comp-tests-pure-caller-f))) + (should (subr-native-elisp-p (symbol-function 'comp-tests-pure-caller-f))) (should (= (comp-tests-pure-caller-f) 4)) - (should (subr-native-elisp-p (symbol-function #'comp-tests-pure-fibn-entry-f))) + (should (subr-native-elisp-p (symbol-function 'comp-tests-pure-fibn-entry-f))) (should (= (comp-tests-pure-fibn-entry-f) 6765)))) (defvar comp-tests-cond-rw-checked-function nil