From a126c3684f8854f8c0d7ab5dcf55f31bac77dcf9 Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Mon, 30 Nov 2020 22:42:08 +0100 Subject: Test byte-compiler free variable warning * test/lisp/emacs-lisp/bytecomp-tests.el (ert-x): Require. (bytecomp--define-warning-file-test): New macro. (bytecomp-warn/warn-free-setq\.el) (bytecomp-warn/warn-free-variable-reference\.el): New tests. * test/lisp/emacs-lisp/bytecomp-resources/warn-free-setq.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-free-variable-reference.el: New files. --- test/lisp/emacs-lisp/bytecomp-resources/warn-free-setq.el | 2 ++ .../emacs-lisp/bytecomp-resources/warn-free-variable-reference.el | 4 ++++ 2 files changed, 6 insertions(+) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-free-setq.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-free-variable-reference.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-free-setq.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-free-setq.el new file mode 100644 index 00000000000..6e187129c9b --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-free-setq.el @@ -0,0 +1,2 @@ +;;; -*- lexical-binding: t -*- +(setq foo 'bar) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-free-variable-reference.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-free-variable-reference.el new file mode 100644 index 00000000000..50a95272874 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-free-variable-reference.el @@ -0,0 +1,4 @@ +;;; -*- lexical-binding: t -*- +(defvar xxx-test) +(defun foo () + (setq xxx-test bar)) -- cgit v1.2.3 From 4457b9590c83f0245604cf6a706383d9aa2c659c Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Tue, 1 Dec 2020 04:46:33 +0100 Subject: Add tests for some byte-compiler warnings * test/lisp/emacs-lisp/bytecomp-tests.el (bytecomp-warn/warn-interactive-only\.el) (bytecomp-warn/warn-obsolete-defun\.el) (bytecomp-warn/warn-obsolete-hook\.el) (bytecomp-warn/warn-obsolete-variable-same-file\.el) (bytecomp-warn/warn-obsolete-variable\.el): New tests. * test/lisp/emacs-lisp/bytecomp-resources/warn-interactive-only.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-defun.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-hook.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-same-file.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable.el: New files. --- .../bytecomp-resources/warn-interactive-only.el | 3 +++ .../bytecomp-resources/warn-obsolete-defun.el | 8 ++++++++ .../bytecomp-resources/warn-obsolete-hook.el | 3 +++ .../warn-obsolete-variable-same-file.el | 13 +++++++++++++ .../bytecomp-resources/warn-obsolete-variable.el | 4 ++++ test/lisp/emacs-lisp/bytecomp-tests.el | 18 ++++++++++++++++++ 6 files changed, 49 insertions(+) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-interactive-only.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-defun.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-hook.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-same-file.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-interactive-only.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-interactive-only.el new file mode 100644 index 00000000000..9e0c99bd30b --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-interactive-only.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo () + (next-line)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-defun.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-defun.el new file mode 100644 index 00000000000..2a7af617ac9 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-defun.el @@ -0,0 +1,8 @@ +;;; -*- lexical-binding: t -*- + +(defun foo-obsolete () + (declare (obsolete nil "99.99")) + nil) + +(defun foo () + (foo-obsolete)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-hook.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-hook.el new file mode 100644 index 00000000000..078e6e4a3a9 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-hook.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo () + (add-hook 'bytecomp--tests-obsolete-var #'next-line)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-same-file.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-same-file.el new file mode 100644 index 00000000000..31deb6155ba --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-same-file.el @@ -0,0 +1,13 @@ +;;; -*- lexical-binding: t -*- + +(defvar foo-obsolete nil) +(make-obsolete-variable 'foo-obsolete nil "99.99") + +;; From bytecomp.el: +;; If foo.el declares `toto' as obsolete, it is likely that foo.el will +;; actually use `toto' in order for this obsolete variable to still work +;; correctly, so paradoxically, while byte-compiling foo.el, the presence +;; of a make-obsolete-variable call for `toto' is an indication that `toto' +;; should not trigger obsolete-warnings in foo.el. +(defun foo () + foo-obsolete) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable.el new file mode 100644 index 00000000000..9a517cc6767 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable.el @@ -0,0 +1,4 @@ +;;; -*- lexical-binding: t -*- + +(defun foo () + bytecomp--tests-obsolete-var) diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index c9070c03b3f..bea9663d241 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -560,6 +560,24 @@ Subtests signal errors if something goes wrong." (bytecomp--define-warning-file-test "warn-free-variable-reference.el" "free.*bar") +(bytecomp--define-warning-file-test "warn-obsolete-defun.el" + "foo-obsolete.*obsolete function.*99.99") + +(defvar bytecomp--tests-obsolete-var nil) +(make-obsolete-variable 'bytecomp--tests-obsolete-var nil "99.99") + +(bytecomp--define-warning-file-test "warn-obsolete-hook.el" + "bytecomp--tests-obs.*obsolete.*99.99") + +(bytecomp--define-warning-file-test "warn-obsolete-variable-same-file.el" + "foo-obs.*obsolete.*99.99" t) + +(bytecomp--define-warning-file-test "warn-obsolete-variable.el" + "bytecomp--tests-obs.*obsolete.*99.99") + +(bytecomp--define-warning-file-test "warn-interactive-only.el" + "next-line.*interactive use only.*forward-line") + (ert-deftest test-eager-load-macro-expansion-eval-when-compile () ;; Make sure we interpret eval-when-compile forms properly. CLISP ;; and SBCL interpreter eval-when-compile (well, the CL equivalent) -- cgit v1.2.3 From ace6eba036e64ff9eee6965951c48d0634b9c696 Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Tue, 1 Dec 2020 13:34:17 +0100 Subject: Fix byte-compiler warning for failed uses of lexical vars * lisp/emacs-lisp/bytecomp.el (byte-compile-form): Fix byte-compiler warning for failed uses of lexical vars. (Bug#44980) * test/lisp/emacs-lisp/bytecomp-tests.el (bytecomp--define-warning-file-test): Don't prefix tests with 'warn'. (bytecomp/error-lexical-var-with-add-hook\.el) (bytecomp/error-lexical-var-with-remove-hook\.el) (bytecomp/error-lexical-var-with-run-hook-with-args-until-failure\.el) (bytecomp/error-lexical-var-with-run-hook-with-args-until-success\.el) (bytecomp/error-lexical-var-with-run-hook-with-args\.el) (bytecomp/error-lexical-var-with-symbol-value\.el): New tests. * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-symbol-value.el: * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args.el: * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-success.el: * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-failure.el: * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-remove-hook.el: * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-add-hook.el: New files. --- lisp/emacs-lisp/bytecomp.el | 2 +- .../error-lexical-var-with-add-hook.el | 4 ++++ .../error-lexical-var-with-remove-hook.el | 4 ++++ ...al-var-with-run-hook-with-args-until-failure.el | 3 +++ ...al-var-with-run-hook-with-args-until-success.el | 3 +++ .../error-lexical-var-with-run-hook-with-args.el | 3 +++ .../error-lexical-var-with-symbol-value.el | 4 ++++ test/lisp/emacs-lisp/bytecomp-tests.el | 26 +++++++++++++++++++--- 8 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-add-hook.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-remove-hook.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-failure.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-success.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-symbol-value.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index a20f3634560..879f08a09f6 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -3203,7 +3203,7 @@ for symbols generated by the byte compiler itself." run-hook-with-args-until-failure)) (pcase (cdr form) (`(',var . ,_) - (when (assq var byte-compile-lexical-variables) + (when (memq var byte-compile-lexical-variables) (byte-compile-report-error (format-message "%s cannot use lexical var `%s'" fn var)))))) ;; Warn about using obsolete hooks. diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-add-hook.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-add-hook.el new file mode 100644 index 00000000000..5f390898e6a --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-add-hook.el @@ -0,0 +1,4 @@ +;;; -*- lexical-binding: t; -*- +(let ((foo nil)) + (add-hook 'foo #'next-line) + foo) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-remove-hook.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-remove-hook.el new file mode 100644 index 00000000000..eaa625eba1c --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-remove-hook.el @@ -0,0 +1,4 @@ +;;; -*- lexical-binding: t; -*- +(let ((foo nil)) + (remove-hook 'foo #'next-line) + foo) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-failure.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-failure.el new file mode 100644 index 00000000000..7a116ad464b --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-failure.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t; -*- +(let ((foo nil)) + (run-hook-with-args-until-failure 'foo)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-success.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-success.el new file mode 100644 index 00000000000..96d10a343df --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-success.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t; -*- +(let ((foo nil)) + (run-hook-with-args-until-success 'foo #'next-line)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args.el new file mode 100644 index 00000000000..bb9101bd070 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t; -*- +(let ((foo nil)) + (run-hook-with-args 'foo)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-symbol-value.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-symbol-value.el new file mode 100644 index 00000000000..5f390898e6a --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-symbol-value.el @@ -0,0 +1,4 @@ +;;; -*- lexical-binding: t; -*- +(let ((foo nil)) + (add-hook 'foo #'next-line) + foo) diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index bea9663d241..d9052da5436 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -548,7 +548,7 @@ Subtests signal errors if something goes wrong." (should (equal (funcall 'def) -1))) (defmacro bytecomp--define-warning-file-test (file re-warning &optional reverse) - `(ert-deftest ,(intern (format "bytecomp-warn/%s" file)) () + `(ert-deftest ,(intern (format "bytecomp/%s" file)) () :expected-result ,(if reverse :failed :passed) (with-current-buffer (get-buffer-create "*Compile-Log*") (let ((inhibit-read-only t)) (erase-buffer)) @@ -556,9 +556,29 @@ Subtests signal errors if something goes wrong." (ert-info ((buffer-string) :prefix "buffer: ") (should (re-search-forward ,re-warning)))))) -(bytecomp--define-warning-file-test "warn-free-setq.el" "free.*foo") +(bytecomp--define-warning-file-test "error-lexical-var-with-add-hook.el" + "add-hook.*lexical var") -(bytecomp--define-warning-file-test "warn-free-variable-reference.el" "free.*bar") +(bytecomp--define-warning-file-test "error-lexical-var-with-remove-hook.el" + "remove-hook.*lexical var") + +(bytecomp--define-warning-file-test "error-lexical-var-with-run-hook-with-args-until-failure.el" + "args-until-failure.*lexical var") + +(bytecomp--define-warning-file-test "error-lexical-var-with-run-hook-with-args-until-success.el" + "args-until-success.*lexical var") + +(bytecomp--define-warning-file-test "error-lexical-var-with-run-hook-with-args.el" + "args.*lexical var") + +(bytecomp--define-warning-file-test "error-lexical-var-with-symbol-value.el" + "symbol-value.*lexical var") + +(bytecomp--define-warning-file-test "warn-free-setq.el" + "free.*foo") + +(bytecomp--define-warning-file-test "warn-free-variable-reference.el" + "free.*bar") (bytecomp--define-warning-file-test "warn-obsolete-defun.el" "foo-obsolete.*obsolete function.*99.99") -- cgit v1.2.3 From 55300e6cdc9b33b52cf17fe64a8ffbb6dce7ae8f Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Thu, 3 Dec 2020 17:08:28 +0100 Subject: Add tests for several byte-compiler warnings * test/lisp/emacs-lisp/bytecomp-tests.el (bytecomp/warn-autoload-not-on-top-level\.el) (bytecomp/warn-callargs\.el) (bytecomp/warn-defcustom-nogroup\.el) (bytecomp/warn-defcustom-notype\.el) (bytecomp/warn-defvar-lacks-prefix\.el) (bytecomp/warn-format\.el) (bytecomp/warn-lambda-malformed-interactive-spec\.el) (bytecomp/warn-make-variable-buffer-local\.el) (bytecomp/warn-redefine-defun-as-macro\.el) (bytecomp/warn-redefine-defun\.el) (bytecomp/warn-redefine-macro-as-defun\.el) (bytecomp/warn-save-excursion\.el) (bytecomp/warn-variable-let-bind-constant\.el) (bytecomp/warn-variable-let-bind-nonvariable\.el) (bytecomp/warn-variable-set-constant\.el) (bytecomp/warn-variable-set-nonvariable\.el): New tests. * test/lisp/emacs-lisp/bytecomp-resources/warn-autoload-not-on-top-level.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-callargs.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-defcustom-nogroup.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-defcustom-notype.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-defvar-lacks-prefix.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-format.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-lambda-malformed-interactive-spec.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-make-variable-buffer-local.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-defun-as-macro.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-defun.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-macro-as-defun.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-save-excursion.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-variable-let-bind-constant.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-variable-let-bind-nonvariable.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-constant.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-nonvariable.el: * test/lisp/emacs-lisp/bytecomp-tests.el: New files. --- .../warn-autoload-not-on-top-level.el | 3 ++ .../emacs-lisp/bytecomp-resources/warn-callargs.el | 5 +++ .../bytecomp-resources/warn-defcustom-nogroup.el | 3 ++ .../bytecomp-resources/warn-defcustom-notype.el | 3 ++ .../bytecomp-resources/warn-defvar-lacks-prefix.el | 2 + .../emacs-lisp/bytecomp-resources/warn-format.el | 2 + .../warn-lambda-malformed-interactive-spec.el | 4 ++ .../warn-make-variable-buffer-local.el | 4 ++ .../warn-redefine-defun-as-macro.el | 3 ++ .../bytecomp-resources/warn-redefine-defun.el | 3 ++ .../warn-redefine-macro-as-defun.el | 3 ++ .../bytecomp-resources/warn-save-excursion.el | 5 +++ .../warn-variable-let-bind-constant.el | 3 ++ .../warn-variable-let-bind-nonvariable.el | 3 ++ .../warn-variable-set-constant.el | 3 ++ .../warn-variable-set-nonvariable.el | 3 ++ test/lisp/emacs-lisp/bytecomp-tests.el | 52 +++++++++++++++++++++- 17 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-autoload-not-on-top-level.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-callargs.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-defcustom-nogroup.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-defcustom-notype.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-defvar-lacks-prefix.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-format.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-lambda-malformed-interactive-spec.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-make-variable-buffer-local.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-defun-as-macro.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-defun.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-macro-as-defun.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-save-excursion.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-variable-let-bind-constant.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-variable-let-bind-nonvariable.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-constant.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-nonvariable.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-autoload-not-on-top-level.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-autoload-not-on-top-level.el new file mode 100644 index 00000000000..f193130c6ca --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-autoload-not-on-top-level.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo () + (autoload 'bar "baz" nil nil 'macro)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-callargs.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-callargs.el new file mode 100644 index 00000000000..687add380b9 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-callargs.el @@ -0,0 +1,5 @@ +;;; -*- lexical-binding: t -*- +(defun foo (_x) + nil) +(defun bar () + (foo 1 2)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-defcustom-nogroup.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-defcustom-nogroup.el new file mode 100644 index 00000000000..a67d4f041f3 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-defcustom-nogroup.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defcustom foo nil + :type 'boolean) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-defcustom-notype.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-defcustom-notype.el new file mode 100644 index 00000000000..c15ab9b192a --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-defcustom-notype.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defcustom foo nil + :group 'emacs) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-defvar-lacks-prefix.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-defvar-lacks-prefix.el new file mode 100644 index 00000000000..9f3cbb98900 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-defvar-lacks-prefix.el @@ -0,0 +1,2 @@ +;;; -*- lexical-binding: t -*- +(defvar foo nil) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-format.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-format.el new file mode 100644 index 00000000000..a1902bc03b0 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-format.el @@ -0,0 +1,2 @@ +;;; -*- lexical-binding: t -*- +(message "%s" 1 2) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-lambda-malformed-interactive-spec.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-lambda-malformed-interactive-spec.el new file mode 100644 index 00000000000..6bd902705ed --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-lambda-malformed-interactive-spec.el @@ -0,0 +1,4 @@ +;;; -*- lexical-binding: t -*- +(defun foo () + (interactive "foo" "bar") + nil) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-make-variable-buffer-local.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-variable-buffer-local.el new file mode 100644 index 00000000000..aa1e6c0463b --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-variable-buffer-local.el @@ -0,0 +1,4 @@ +;;; -*- lexical-binding: t -*- +(defvar foobar) +(defun foo () + (make-variable-buffer-local 'foobar)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-defun-as-macro.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-defun-as-macro.el new file mode 100644 index 00000000000..6bd239b6598 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-defun-as-macro.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo () nil) +(defmacro foo () t) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-defun.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-defun.el new file mode 100644 index 00000000000..53e4c0ac8de --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-defun.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo () nil) +(defun foo () t) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-macro-as-defun.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-macro-as-defun.el new file mode 100644 index 00000000000..f71ae445615 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-redefine-macro-as-defun.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defmacro foo () t) +(defun foo () nil) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-save-excursion.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-save-excursion.el new file mode 100644 index 00000000000..38185457192 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-save-excursion.el @@ -0,0 +1,5 @@ +;;; -*- lexical-binding: t -*- +(defun foo () + (save-excursion + (set-buffer (current-buffer)) + nil)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-let-bind-constant.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-let-bind-constant.el new file mode 100644 index 00000000000..cc1fb572577 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-let-bind-constant.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo () + (let ((t 1)) t)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-let-bind-nonvariable.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-let-bind-nonvariable.el new file mode 100644 index 00000000000..dde2dcee6e7 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-let-bind-nonvariable.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo () + (let (('t 1)) t)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-constant.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-constant.el new file mode 100644 index 00000000000..2fc0680cfab --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-constant.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo () + (setq t nil)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-nonvariable.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-nonvariable.el new file mode 100644 index 00000000000..0c76c4d388b --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-nonvariable.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo () + (set '(a) nil)) diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index 7ed90217365..8fa4d278f11 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -567,12 +567,39 @@ Subtests signal errors if something goes wrong." (bytecomp--define-warning-file-test "error-lexical-var-with-symbol-value.el" "symbol-value.*lexical var") +(bytecomp--define-warning-file-test "warn-autoload-not-on-top-level.el" + "compiler ignores.*autoload.*") + +(bytecomp--define-warning-file-test "warn-callargs.el" + "with 2 arguments, but accepts only 1") + +(bytecomp--define-warning-file-test "warn-defcustom-nogroup.el" + "fails to specify containing group") + +(bytecomp--define-warning-file-test "warn-defcustom-notype.el" + "fails to specify type") + +(bytecomp--define-warning-file-test "warn-defvar-lacks-prefix.el" + "var.*foo.*lacks a prefix") + +(bytecomp--define-warning-file-test "warn-format.el" + "called with 2 args to fill 1 format field") + (bytecomp--define-warning-file-test "warn-free-setq.el" "free.*foo") (bytecomp--define-warning-file-test "warn-free-variable-reference.el" "free.*bar") +(bytecomp--define-warning-file-test "warn-make-variable-buffer-local.el" + "make-variable-buffer-local.*not called at toplevel") + +(bytecomp--define-warning-file-test "warn-interactive-only.el" + "next-line.*interactive use only.*forward-line") + +(bytecomp--define-warning-file-test "warn-lambda-malformed-interactive-spec.el" + "malformed interactive spec") + (bytecomp--define-warning-file-test "warn-obsolete-defun.el" "foo-obsolete.*obsolete function.*99.99") @@ -588,8 +615,29 @@ Subtests signal errors if something goes wrong." (bytecomp--define-warning-file-test "warn-obsolete-variable.el" "bytecomp--tests-obs.*obsolete.*99.99") -(bytecomp--define-warning-file-test "warn-interactive-only.el" - "next-line.*interactive use only.*forward-line") +(bytecomp--define-warning-file-test "warn-redefine-defun-as-macro.el" + "as both function and macro") + +(bytecomp--define-warning-file-test "warn-redefine-macro-as-defun.el" + "as both function and macro") + +(bytecomp--define-warning-file-test "warn-redefine-defun.el" + "defined multiple") + +(bytecomp--define-warning-file-test "warn-save-excursion.el" + "with-current.*rather than save-excursion") + +(bytecomp--define-warning-file-test "warn-variable-let-bind-constant.el" + "let-bind constant") + +(bytecomp--define-warning-file-test "warn-variable-let-bind-nonvariable.el" + "let-bind nonvariable") + +(bytecomp--define-warning-file-test "warn-variable-set-constant.el" + "variable reference to constant") + +(bytecomp--define-warning-file-test "warn-variable-set-nonvariable.el" + "variable reference to nonvariable") ;;;; Macro expansion. -- cgit v1.2.3 From 0ebea8ffbfb7b9b1bd92f30011df0875b54eb663 Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Sun, 6 Dec 2020 12:44:19 +0100 Subject: Make byte-compiler warn about wide docstrings * lisp/emacs-lisp/bytecomp.el (byte-compile--wide-docstring-p): (byte-compile-docstring-length-warn): New defuns. (byte-compile-docstring-max-column): New defcustom. (byte-compile--wide-docstring-substitution-len): New variable. (byte-compile-warning-types, byte-compile-warnings): New value 'docstrings'. (byte-compile-file-form-autoload, byte-compile-file-form-defvar): (byte-compile-file-form-defvar-function, byte-compile-lambda): (byte-compile-defvar, byte-compile-file-form-defalias): Warn about too wide docstrings. (Bug#44858) * test/lisp/emacs-lisp/bytecomp-tests.el (bytecomp-warn-wide-docstring/defconst) (bytecomp-warn-wide-docstring/defvar): New tests. (bytecomp--define-warning-file-test): New macro. (bytecomp/warn-wide-docstring-autoload\.el) (bytecomp/warn-wide-docstring-custom-declare-variable\.el) (bytecomp/warn-wide-docstring-defalias\.el) (bytecomp/warn-wide-docstring-defconst\.el) (bytecomp/warn-wide-docstring-define-abbrev-table\.el) (bytecomp/warn-wide-docstring-define-obsolete-function-alias\.el) (bytecomp/warn-wide-docstring-define-obsolete-variable-alias\.el) (bytecomp/warn-wide-docstring-defun\.el) (bytecomp/warn-wide-docstring-defvar\.el) (bytecomp/warn-wide-docstring-defvaralias\.el) (bytecomp/warn-wide-docstring-ignore-fill-column\.el) (bytecomp/warn-wide-docstring-ignore-override\.el) (bytecomp/warn-wide-docstring-ignore\.el) (bytecomp/warn-wide-docstring-multiline-first\.el) (bytecomp/warn-wide-docstring-multiline\.el): New tests. * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-autoload.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-custom-declare-variable.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defalias.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defconst.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-abbrev-table.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-function-alias.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-variable-alias.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defun.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvar.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvaralias.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-fill-column.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-override.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline-first.el: * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline.el: New files. --- etc/NEWS | 11 ++- etc/TODO | 2 - lisp/emacs-lisp/bytecomp.el | 88 +++++++++++++++++++++- .../warn-wide-docstring-autoload.el | 3 + .../warn-wide-docstring-custom-declare-variable.el | 4 + .../warn-wide-docstring-defalias.el | 3 + .../warn-wide-docstring-defconst.el | 3 + .../warn-wide-docstring-define-abbrev-table.el | 3 + ...ide-docstring-define-obsolete-function-alias.el | 3 + ...ide-docstring-define-obsolete-variable-alias.el | 3 + .../warn-wide-docstring-defun.el | 3 + .../warn-wide-docstring-defvar.el | 6 ++ .../warn-wide-docstring-defvaralias.el | 3 + .../warn-wide-docstring-ignore-fill-column.el | 7 ++ .../warn-wide-docstring-ignore-override.el | 8 ++ .../warn-wide-docstring-ignore.el | 7 ++ .../warn-wide-docstring-multiline-first.el | 5 ++ .../warn-wide-docstring-multiline.el | 6 ++ test/lisp/emacs-lisp/bytecomp-tests.el | 71 +++++++++++++++++ 19 files changed, 233 insertions(+), 6 deletions(-) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-autoload.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-custom-declare-variable.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defalias.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defconst.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-abbrev-table.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-function-alias.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-variable-alias.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defun.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvar.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvaralias.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-fill-column.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-override.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline-first.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/etc/NEWS b/etc/NEWS index 131931052a5..f8282696e46 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -2200,17 +2200,24 @@ menu handling. +++ ** 'inhibit-nul-byte-detection' is renamed to 'inhibit-null-byte-detection'. +** Byte compiler + +++ -** New byte-compiler check for missing dynamic variable declarations. +*** New byte-compiler check for missing dynamic variable declarations. It is meant as an (experimental) aid for converting Emacs Lisp code to lexical binding, where dynamic (special) variables bound in one file can affect code in another. For details, see the manual section "(Elisp) Converting to Lexical Binding". +++ -** 'byte-recompile-directory' can now compile symlinked ".el" files. +*** 'byte-recompile-directory' can now compile symlinked ".el" files. This is achieved by giving a non-nil FOLLOW-SYMLINKS parameter. +*** The byte-compiler now warns about too wide documentation strings. +By default, it will warn if a documentation string is wider than the +largest of 80 characters or 'fill-column'. This is controlled by the +new user option 'byte-compile-docstring-max-column'. + --- ** 'unload-feature' now also tries to undo additions to buffer-local hooks. diff --git a/etc/TODO b/etc/TODO index 2a9fd8869e0..d7bcfd4d97c 100644 --- a/etc/TODO +++ b/etc/TODO @@ -638,8 +638,6 @@ Do this for some or all errors associated with using subprocesses. ** Maybe reinterpret 'parse-error' as a category of errors Put some other errors under it. -** Make byte-compiler warn when a doc string is too wide - ** Make byte-optimization warnings issue accurate line numbers ** Record the sxhash of the default value for customized variables diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 7e1a3304cc8..f14ad93d2e0 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -299,7 +299,8 @@ The information is logged to `byte-compile-log-buffer'." (defconst byte-compile-warning-types '(redefine callargs free-vars unresolved obsolete noruntime interactive-only - make-local mapcar constants suspicious lexical lexical-dynamic) + make-local mapcar constants suspicious lexical lexical-dynamic + docstrings) "The list of warning types used when `byte-compile-warnings' is t.") (defcustom byte-compile-warnings t "List of warnings that the byte-compiler should issue (t for all). @@ -322,6 +323,8 @@ Elements of the list may be: make-local calls to make-variable-buffer-local that may be incorrect. mapcar mapcar called for effect. constants let-binding of, or assignment to, constants/nonvariables. + docstrings docstrings that are too wide (longer than 80 characters, + or `fill-column', whichever is bigger) suspicious constructs that usually don't do what the coder wanted. If the list begins with `not', then the remaining elements specify warnings to @@ -1563,6 +1566,81 @@ extra args." (if (equal sig1 '(1 . 1)) "argument" "arguments") (byte-compile-arglist-signature-string sig2))))))) +(defvar byte-compile--wide-docstring-substitution-len 3 + "Substitution width used in `byte-compile--wide-docstring-p'. +This is a heuristic for guessing the width of a documentation +string: `byte-compile--wide-docstring-p' assumes that any +`substitute-command-keys' command substitutions are this long.") + +(defun byte-compile--wide-docstring-p (docstring col) + "Return t if string DOCSTRING is wider than COL. +Ignore all `substitute-command-keys' substitutions, except for +the `\\\\=[command]' ones that are assumed to be of length +`byte-compile--wide-docstring-substitution-len'. Also ignore +URLs." + (string-match + (format "^.\\{%s,\\}$" (int-to-string (1+ col))) + (replace-regexp-in-string + (rx (or + ;; Ignore some URLs. + (seq "http" (? "s") "://" (* anychar)) + ;; Ignore these `substitute-command-keys' substitutions. + (seq "\\" (or "=" + (seq "<" (* (not ">")) ">") + (seq "{" (* (not "}")) "}"))))) + "" + ;; Heuristic: assume these substitutions are of some length N. + (replace-regexp-in-string + (rx "\\" (or (seq "[" (* (not "]")) "]"))) + (make-string byte-compile--wide-docstring-substitution-len ?x) + docstring)))) + +(defcustom byte-compile-docstring-max-column 80 + "Recommended maximum width of doc string lines. +The byte-compiler will emit a warning for documentation strings +containing lines wider than this. If `fill-column' has a larger +value, it will override this variable." + :group 'bytecomp + :type 'integer + :safe #'integerp + :version "28.1") + +(defun byte-compile-docstring-length-warn (form) + "Warn if documentation string of FORM is too wide. +It is too wide if it has any lines longer than the largest of +`fill-column' and `byte-compile-docstring-max-column'." + ;; This has some limitations that it would be nice to fix: + ;; 1. We don't try to handle defuns. It is somewhat tricky to get + ;; it right since `defun' is a macro. Also, some macros + ;; themselves produce defuns (e.g. `define-derived-mode'). + ;; 2. We assume that any `subsititute-command-keys' command replacement has a + ;; given length. We can't reliably do these replacements, since the value + ;; of the keymaps in general can't be known at compile time. + (when (byte-compile-warning-enabled-p 'docstrings) + (let ((col (max byte-compile-docstring-max-column fill-column)) + kind name docs) + (pcase (car form) + ((or 'autoload 'custom-declare-variable 'defalias + 'defconst 'define-abbrev-table + 'defvar 'defvaralias) + (setq kind (nth 0 form)) + (setq name (nth 1 form)) + (setq docs (nth 3 form))) + ;; Here is how one could add lambda's here: + ;; ('lambda + ;; (setq kind "") ; can't be "function", unfortunately + ;; (setq docs (and (stringp (nth 2 form)) + ;; (nth 2 form)))) + ) + (when (and (consp name) (eq (car name) 'quote)) + (setq name (cadr name))) + (setq name (if name (format " `%s'" name) "")) + (when (and kind docs (stringp docs) + (byte-compile--wide-docstring-p docs col)) + (byte-compile-warn "%s%s docstring wider than %s characters" + kind name col)))) + form) + (defun byte-compile-print-syms (str1 strn syms) (when syms (byte-compile-set-symbol-position (car syms) t)) @@ -2410,7 +2488,8 @@ list that represents a doc string reference. (delq (assq funsym byte-compile-unresolved-functions) byte-compile-unresolved-functions))))) (if (stringp (nth 3 form)) - form + (prog1 form + (byte-compile-docstring-length-warn form)) ;; No doc string, so we can compile this as a normal form. (byte-compile-keep-pending form 'byte-compile-normal-call))) @@ -2438,6 +2517,7 @@ list that represents a doc string reference. (if (and (null (cddr form)) ;No `value' provided. (eq (car form) 'defvar)) ;Just a declaration. nil + (byte-compile-docstring-length-warn form) (cond ((consp (nth 2 form)) (setq form (copy-sequence form)) (setcar (cdr (cdr form)) @@ -2461,6 +2541,7 @@ list that represents a doc string reference. (if (byte-compile-warning-enabled-p 'suspicious) (byte-compile-warn "Alias for `%S' should be declared before its referent" newname))))) + (byte-compile-docstring-length-warn form) (byte-compile-keep-pending form)) (put 'custom-declare-variable 'byte-hunk-handler @@ -2844,6 +2925,7 @@ for symbols generated by the byte compiler itself." (unless (eq 'lambda (car-safe fun)) (error "Not a lambda list: %S" fun)) (byte-compile-set-symbol-position 'lambda)) + (byte-compile-docstring-length-warn fun) (byte-compile-check-lambda-list (nth 1 fun)) (let* ((arglist (nth 1 fun)) (arglistvars (byte-compile-arglist-vars arglist)) @@ -4624,6 +4706,7 @@ binding slots have been popped." (byte-compile-warning-enabled-p 'lexical (nth 1 form))) (byte-compile-warn "global/dynamic var `%s' lacks a prefix" (nth 1 form))) + (byte-compile-docstring-length-warn form) (let ((fun (nth 0 form)) (var (nth 1 form)) (value (nth 2 form)) @@ -4698,6 +4781,7 @@ binding slots have been popped." ;; - `arg' is the expression to which it is defined. ;; - `rest' is the rest of the arguments. (`(,_ ',name ,arg . ,rest) + (byte-compile-docstring-length-warn form) (pcase-let* ;; `macro' is non-nil if it defines a macro. ;; `fun' is the function part of `arg' (defaults to `arg'). diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-autoload.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-autoload.el new file mode 100644 index 00000000000..96deb1bbb0a --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-autoload.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(autoload 'foox "foo" + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-custom-declare-variable.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-custom-declare-variable.el new file mode 100644 index 00000000000..2a4700bfda5 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-custom-declare-variable.el @@ -0,0 +1,4 @@ +;;; -*- lexical-binding: t -*- +(custom-declare-variable + 'foo t + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defalias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defalias.el new file mode 100644 index 00000000000..a4235d22bd3 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defalias.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defalias 'foo #'ignore + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defconst.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defconst.el new file mode 100644 index 00000000000..946f01989a0 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defconst.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defconst foo-bar nil + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-abbrev-table.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-abbrev-table.el new file mode 100644 index 00000000000..3da9ccd48c6 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-abbrev-table.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(define-abbrev-table 'foo () + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-function-alias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-function-alias.el new file mode 100644 index 00000000000..fea841b12ec --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-function-alias.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(define-obsolete-function-alias 'foo #'ignore "99.1" + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-variable-alias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-variable-alias.el new file mode 100644 index 00000000000..2d5f201cb65 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-variable-alias.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(define-obsolete-variable-alias 'foo 'ignore "99.1" + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defun.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defun.el new file mode 100644 index 00000000000..94b0e80c979 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defun.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo () + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvar.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvar.el new file mode 100644 index 00000000000..99aacd09cbd --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvar.el @@ -0,0 +1,6 @@ +;;; -*- lexical-binding: t -*- +(defvar foo-bar nil + "multiline +foo +xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +bar") diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvaralias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvaralias.el new file mode 100644 index 00000000000..52fdc17f5bf --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvaralias.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defvaralias 'foo-bar #'ignore + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-fill-column.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-fill-column.el new file mode 100644 index 00000000000..1ff554f3704 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-fill-column.el @@ -0,0 +1,7 @@ +;;; -*- lexical-binding: t -*- +(defvar foo-bar nil + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + +;; Local Variables: +;; fill-column: 100 +;; End: diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-override.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-override.el new file mode 100644 index 00000000000..0bcf7b1d63b --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-override.el @@ -0,0 +1,8 @@ +;;; -*- lexical-binding: t -*- +(defvar foo-bar nil + "123456789012345") + +;; Local Variables: +;; byte-compile-docstring-max-column: 10 +;; fill-column: 20 +;; End: diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore.el new file mode 100644 index 00000000000..c80ddd180d9 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore.el @@ -0,0 +1,7 @@ +;;; -*- lexical-binding: t -*- +(defvar foo-bar nil + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + +;; Local Variables: +;; byte-compile-docstring-max-column: 100 +;; End: diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline-first.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline-first.el new file mode 100644 index 00000000000..2563dbbb3b9 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline-first.el @@ -0,0 +1,5 @@ +;;; -*- lexical-binding: t -*- +(defvar foo-bar nil + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +This is a multiline docstring where the first line is long. +foobar") diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline.el new file mode 100644 index 00000000000..9ae7bc9b9f0 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline.el @@ -0,0 +1,6 @@ +;;; -*- lexical-binding: t -*- +(defvar foo-bar nil + "This is a multiline docstring. +But it's not the first line that is long. +xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +foobar") diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index 4a6e28f7c7c..47aab563f6f 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -540,6 +540,16 @@ Subtests signal errors if something goes wrong." (bytecomp--with-warning-test "foo.*lacks a prefix" '(defvar foo nil))) +(defvar bytecomp-tests--docstring (make-string 100 ?x)) + +(ert-deftest bytecomp-warn-wide-docstring/defconst () + (bytecomp--with-warning-test "defconst.*foo.*wider than.*characters" + `(defconst foo t ,bytecomp-tests--docstring))) + +(ert-deftest bytecomp-warn-wide-docstring/defvar () + (bytecomp--with-warning-test "defvar.*foo.*wider than.*characters" + `(defvar foo t ,bytecomp-tests--docstring))) + (defmacro bytecomp--define-warning-file-test (file re-warning &optional reverse) `(ert-deftest ,(intern (format "bytecomp/%s" file)) () :expected-result ,(if reverse :failed :passed) @@ -639,6 +649,67 @@ Subtests signal errors if something goes wrong." (bytecomp--define-warning-file-test "warn-variable-set-nonvariable.el" "variable reference to nonvariable") +(bytecomp--define-warning-file-test + "warn-wide-docstring-autoload.el" + "autoload.*foox.*wider than.*characters") + +(bytecomp--define-warning-file-test + "warn-wide-docstring-custom-declare-variable.el" + "custom-declare-variable.*foo.*wider than.*characters") + +(bytecomp--define-warning-file-test + "warn-wide-docstring-defalias.el" + "defalias.*foo.*wider than.*characters") + +(bytecomp--define-warning-file-test + "warn-wide-docstring-defconst.el" + "defconst.*foo.*wider than.*characters") + +(bytecomp--define-warning-file-test + "warn-wide-docstring-define-abbrev-table.el" + "define-abbrev.*foo.*wider than.*characters") + +(bytecomp--define-warning-file-test + "warn-wide-docstring-define-obsolete-function-alias.el" + "defalias.*foo.*wider than.*characters") + +(bytecomp--define-warning-file-test + "warn-wide-docstring-define-obsolete-variable-alias.el" + "defvaralias.*foo.*wider than.*characters") + +;; TODO: We don't yet issue warnings for defuns. +(bytecomp--define-warning-file-test + "warn-wide-docstring-defun.el" + "wider than.*characters" 'reverse) + +(bytecomp--define-warning-file-test + "warn-wide-docstring-defvar.el" + "defvar.*foo.*wider than.*characters") + +(bytecomp--define-warning-file-test + "warn-wide-docstring-defvaralias.el" + "defvaralias.*foo.*wider than.*characters") + +(bytecomp--define-warning-file-test + "warn-wide-docstring-ignore-fill-column.el" + "defvar.*foo.*wider than.*characters" 'reverse) + +(bytecomp--define-warning-file-test + "warn-wide-docstring-ignore-override.el" + "defvar.*foo.*wider than.*characters" 'reverse) + +(bytecomp--define-warning-file-test + "warn-wide-docstring-ignore.el" + "defvar.*foo.*wider than.*characters" 'reverse) + +(bytecomp--define-warning-file-test + "warn-wide-docstring-multiline-first.el" + "defvar.*foo.*wider than.*characters") + +(bytecomp--define-warning-file-test + "warn-wide-docstring-multiline.el" + "defvar.*foo.*wider than.*characters") + ;;;; Macro expansion. -- cgit v1.2.3 From 96bbbaec5c1b2612946ff08abf9d43e7478e8c43 Mon Sep 17 00:00:00 2001 From: Michael Heerdegen Date: Tue, 22 Dec 2020 05:44:47 +0100 Subject: Fix obsolete variable warnings about class names * lisp/emacs-lisp/eieio-core.el (eieio-defclass-autoload): Try to make the wording of the warning about the obsoleted variable less confusing. * lisp/emacs-lisp/bytecomp.el (byte-compile-check-variable): Don't warn for lexical variables (Bug#39169). Fix spurious `or'. * test/lisp/emacs-lisp/bytecomp-tests.el (bytecomp/warn-obsolete-variable-bound\.el): New test. * test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-bound.el: New file. --- lisp/emacs-lisp/bytecomp.el | 9 +++++---- lisp/emacs-lisp/eieio-core.el | 3 ++- .../bytecomp-resources/warn-obsolete-variable-bound.el | 7 +++++++ test/lisp/emacs-lisp/bytecomp-tests.el | 3 +++ 4 files changed, 17 insertions(+), 5 deletions(-) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-bound.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 76457814acd..360da6b6ba6 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -3441,10 +3441,11 @@ for symbols generated by the byte compiler itself." (and od (not (memq var byte-compile-not-obsolete-vars)) (not (memq var byte-compile-global-not-obsolete-vars)) - (or (pcase (nth 1 od) - ('set (not (eq access-type 'reference))) - ('get (eq access-type 'reference)) - (_ t))))) + (not (memq var byte-compile-lexical-variables)) + (pcase (nth 1 od) + ('set (not (eq access-type 'reference))) + ('get (eq access-type 'reference)) + (_ t)))) (byte-compile-warn-obsolete var)))) (defsubst byte-compile-dynamic-variable-op (base-op var) diff --git a/lisp/emacs-lisp/eieio-core.el b/lisp/emacs-lisp/eieio-core.el index 3e5e9b95235..a8361c0d4b4 100644 --- a/lisp/emacs-lisp/eieio-core.el +++ b/lisp/emacs-lisp/eieio-core.el @@ -215,7 +215,8 @@ It creates an autoload function for CNAME's constructor." ;; turn this into a usable self-pointing symbol (when eieio-backward-compatibility (set cname cname) - (make-obsolete-variable cname (format "use \\='%s instead" cname) + (make-obsolete-variable cname (format "\ +use \\='%s or turn off `eieio-backward-compatibility' instead" cname) "25.1")) (setf (cl--find-class cname) newc) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-bound.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-bound.el new file mode 100644 index 00000000000..e65a541e6e3 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-bound.el @@ -0,0 +1,7 @@ +;;; -*- lexical-binding: t -*- + +(make-obsolete-variable 'bytecomp--tests-obsolete-var-2 nil "99.99") + +(defun foo () + (let ((bytecomp--tests-obsolete-var-2 2)) + bytecomp--tests-obsolete-var-2)) diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index 5e5f99dbdab..a07af188fac 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -625,6 +625,9 @@ Subtests signal errors if something goes wrong." (bytecomp--define-warning-file-test "warn-obsolete-variable.el" "bytecomp--tests-obs.*obsolete.*99.99") +(bytecomp--define-warning-file-test "warn-obsolete-variable-bound.el" + "bytecomp--tests-obs.*obsolete.*99.99" t) + (bytecomp--define-warning-file-test "warn-redefine-defun-as-macro.el" "as both function and macro") -- cgit v1.2.3 From b41b4add7bc2485fadc6ff3a890efbd1307b2351 Mon Sep 17 00:00:00 2001 From: Stefan Monnier Date: Thu, 21 Jan 2021 13:15:05 -0500 Subject: Fix spurious "Lexical argument shadows the dynamic variable" due to inlining Before this patch doing: rm lisp/calendar/calendar.elc make lisp/calendar/cal-hebrew.elc would spew out lots of spurious such warnings about a `date` argument, pointing to code which has no `date` argument in sight. This was because that code had calls to inlinable functions (taking a `date` argument) defined in `calendar.el`, and while `date` is a normal lexical var at the site of those functions' definitions, it was declared as dynbound at the call site. * lisp/emacs-lisp/byte-opt.el (byte-compile-inline-expand): Don't impose our local context onto the inlined function. * test/lisp/emacs-lisp/bytecomp-tests.el: Add matching test. --- lisp/emacs-lisp/byte-opt.el | 6 ++++-- .../lisp/emacs-lisp/bytecomp-resources/foo-inlinable.el | 6 ++++++ .../bytecomp-resources/nowarn-inline-after-defvar.el | 17 +++++++++++++++++ test/lisp/emacs-lisp/bytecomp-tests.el | 4 ++++ 4 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/foo-inlinable.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/nowarn-inline-after-defvar.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el index cfa407019a7..66a117fccc8 100644 --- a/lisp/emacs-lisp/byte-opt.el +++ b/lisp/emacs-lisp/byte-opt.el @@ -284,8 +284,10 @@ ;; If `fn' is from the same file, it has already ;; been preprocessed! `(function ,fn) - (byte-compile-preprocess - (byte-compile--reify-function fn))))) + ;; Try and process it "in its original environment". + (let ((byte-compile-bound-variables nil)) + (byte-compile-preprocess + (byte-compile--reify-function fn)))))) (if (eq (car-safe newfn) 'function) (byte-compile-unfold-lambda `(,(cadr newfn) ,@(cdr form))) ;; This can happen because of macroexp-warn-and-return &co. diff --git a/test/lisp/emacs-lisp/bytecomp-resources/foo-inlinable.el b/test/lisp/emacs-lisp/bytecomp-resources/foo-inlinable.el new file mode 100644 index 00000000000..47481574ea8 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/foo-inlinable.el @@ -0,0 +1,6 @@ +;; -*- lexical-binding: t; -*- + +(defsubst foo-inlineable (foo-var) + (+ foo-var 2)) + +(provide 'foo-inlinable) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/nowarn-inline-after-defvar.el b/test/lisp/emacs-lisp/bytecomp-resources/nowarn-inline-after-defvar.el new file mode 100644 index 00000000000..5582b2ab0ea --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/nowarn-inline-after-defvar.el @@ -0,0 +1,17 @@ +;; -*- lexical-binding: t; -*- + +;; In this test, we try and make sure that inlined functions's code isn't +;; mistakenly re-interpreted in the caller's context: we import an +;; inlinable function from another file where `foo-var' is a normal +;; lexical variable, and then call(inline) it in a function where +;; `foo-var' is a dynamically-scoped variable. + +(require 'foo-inlinable + (expand-file-name "foo-inlinable.el" + (file-name-directory + (or byte-compile-current-file load-file-name)))) + +(defvar foo-var) + +(defun foo-fun () + (+ (foo-inlineable 5) 1)) diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index 263736af4ed..980b402ca2d 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -713,6 +713,10 @@ Subtests signal errors if something goes wrong." "warn-wide-docstring-multiline.el" "defvar.*foo.*wider than.*characters") +(bytecomp--define-warning-file-test + "nowarn-inline-after-defvar.el" + "Lexical argument shadows" 'reverse) + ;;;; Macro expansion. -- cgit v1.2.3 From 40d2970f4360bd942ffc3f86db9ff1499a5a5393 Mon Sep 17 00:00:00 2001 From: Mattias Engdegård Date: Thu, 27 May 2021 14:03:14 +0200 Subject: Don't propagate lexical variables into inlined functions Functions compiled when inlined (thus from inside the optimiser) mustn't retain the lexical environment of the caller or there will be tears. See discussion at https://lists.gnu.org/archive/html/emacs-devel/2021-05/msg01227.html . Bug found by Stefan Monnier. * lisp/emacs-lisp/byte-opt.el (byte-compile-inline-expand): Bind byte-optimize--lexvars to nil when re-entering the compiler recursively. * test/lisp/emacs-lisp/bytecomp-resources/bc-test-alpha.el: * test/lisp/emacs-lisp/bytecomp-resources/bc-test-beta.el: New files. * test/lisp/emacs-lisp/bytecomp-tests.el (bytecomp-defsubst): New test. --- lisp/emacs-lisp/byte-opt.el | 5 ++++- .../emacs-lisp/bytecomp-resources/bc-test-alpha.el | 9 +++++++++ .../lisp/emacs-lisp/bytecomp-resources/bc-test-beta.el | 6 ++++++ test/lisp/emacs-lisp/bytecomp-tests.el | 18 ++++++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/bc-test-alpha.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/bc-test-beta.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el index 99b5319ab3d..842697c7245 100644 --- a/lisp/emacs-lisp/byte-opt.el +++ b/lisp/emacs-lisp/byte-opt.el @@ -278,7 +278,10 @@ ;; first, and then inline its byte-code. This also has the advantage ;; that the final code does not depend on the order of compilation ;; of ELisp files, making the build more reproducible. - (byte-compile name) + ;; Since we are called from inside the optimiser, we need to make + ;; sure not to propagate lexvar values. + (dlet ((byte-optimize--lexvars nil)) + (byte-compile name)) `(,(symbol-function name) ,@(cdr form)))) (_ ;; Give up on inlining. diff --git a/test/lisp/emacs-lisp/bytecomp-resources/bc-test-alpha.el b/test/lisp/emacs-lisp/bytecomp-resources/bc-test-alpha.el new file mode 100644 index 00000000000..6997d91b26a --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/bc-test-alpha.el @@ -0,0 +1,9 @@ +;;; -*- lexical-binding: t -*- + +(require 'bc-test-beta) + +(defun bc-test-alpha-f (x) + (let ((y nil)) + (list y (bc-test-beta-f x)))) + +(provide 'bc-test-alpha) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/bc-test-beta.el b/test/lisp/emacs-lisp/bytecomp-resources/bc-test-beta.el new file mode 100644 index 00000000000..9205a13d7d5 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/bc-test-beta.el @@ -0,0 +1,6 @@ +;;; -*- lexical-binding: t -*- + +(defsubst bc-test-beta-f (y) + y) + +(provide 'bc-test-beta) diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index c9ab3ec1f1b..33413f5a002 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -1312,6 +1312,24 @@ compiled correctly." (funcall f 3)) 4))) +(declare-function bc-test-alpha-f (ert-resource-file "bc-test-alpha.el")) + +(ert-deftest bytecomp-defsubst () + ;; Check that lexical variables don't leak into inlined code. See + ;; https://lists.gnu.org/archive/html/emacs-devel/2021-05/msg01227.html + + ;; First, remove any trace of the functions and package defined: + (fmakunbound 'bc-test-alpha-f) + (fmakunbound 'bc-test-beta-f) + (setq features (delq 'bc-test-beta features)) + ;; Byte-compile one file that uses a function from another file that isn't + ;; compiled. + (let ((file (ert-resource-file "bc-test-alpha.el")) + (load-path (cons (ert-resource-directory) load-path))) + (byte-compile-file file) + (load-file (concat file "c")) + (should (equal (bc-test-alpha-f 'a) '(nil a))))) + ;; Local Variables: ;; no-byte-compile: t ;; End: -- cgit v1.2.3 From 109ca1bd00b56ba66b123b505d8c2187fded0ef7 Mon Sep 17 00:00:00 2001 From: Mattias Engdegård Date: Thu, 22 Jul 2021 15:00:17 +0200 Subject: Warn about arity errors in inlining calls (bug#12299) Wrong number of arguments in inlining function calls (to `defsubst` or explicitly using `inline`) did not result in warnings, or in very cryptic ones. * lisp/emacs-lisp/byte-opt.el (byte-compile-inline-expand): Add calls to `byte-compile--check-arity-bytecode`. * lisp/emacs-lisp/bytecomp.el (byte-compile-emit-callargs-warn) (byte-compile--check-arity-bytecode): New functions. (byte-compile-callargs-warn): Use factored-out function. * test/lisp/emacs-lisp/bytecomp-resources/warn-callargs-defsubst.el: * test/lisp/emacs-lisp/bytecomp-tests.el ("warn-callargs-defsubst.el"): New test case. --- lisp/emacs-lisp/byte-opt.el | 5 ++- lisp/emacs-lisp/bytecomp.el | 37 ++++++++++++++++------ .../bytecomp-resources/warn-callargs-defsubst.el | 5 +++ test/lisp/emacs-lisp/bytecomp-tests.el | 3 ++ 4 files changed, 39 insertions(+), 11 deletions(-) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-callargs-defsubst.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el index 341643c7d16..ad9f827171a 100644 --- a/lisp/emacs-lisp/byte-opt.el +++ b/lisp/emacs-lisp/byte-opt.el @@ -274,6 +274,7 @@ Earlier variables shadow later ones with the same name.") ((pred byte-code-function-p) ;; (message "Inlining byte-code for %S!" name) ;; The byte-code will be really inlined in byte-compile-unfold-bcf. + (byte-compile--check-arity-bytecode form fn) `(,fn ,@(cdr form))) ((or `(lambda . ,_) `(closure . ,_)) ;; While byte-compile-unfold-bcf can inline dynbind byte-code into @@ -300,7 +301,9 @@ Earlier variables shadow later ones with the same name.") ;; surrounded the `defsubst'. (byte-compile-warnings nil)) (byte-compile name)) - `(,(symbol-function name) ,@(cdr form)))) + (let ((bc (symbol-function name))) + (byte-compile--check-arity-bytecode form bc) + `(,bc ,@(cdr form))))) (_ ;; Give up on inlining. form)))) diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 2968f1af5df..f6150069e81 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -1477,6 +1477,30 @@ when printing the error message." (push (list f byte-compile-last-position nargs) byte-compile-unresolved-functions))))) +(defun byte-compile-emit-callargs-warn (name actual-args min-args max-args) + (byte-compile-set-symbol-position name) + (byte-compile-warn + "%s called with %d argument%s, but %s %s" + name actual-args + (if (= 1 actual-args) "" "s") + (if (< actual-args min-args) + "requires" + "accepts only") + (byte-compile-arglist-signature-string (cons min-args max-args)))) + +(defun byte-compile--check-arity-bytecode (form bytecode) + "Check that the call in FORM matches that allowed by BYTECODE." + (when (and (byte-code-function-p bytecode) + (byte-compile-warning-enabled-p 'callargs)) + (let* ((actual-args (length (cdr form))) + (arity (func-arity bytecode)) + (min-args (car arity)) + (max-args (and (numberp (cdr arity)) (cdr arity)))) + (when (or (< actual-args min-args) + (and max-args (> actual-args max-args))) + (byte-compile-emit-callargs-warn + (car form) actual-args min-args max-args))))) + ;; Warn if the form is calling a function with the wrong number of arguments. (defun byte-compile-callargs-warn (form) (let* ((def (or (byte-compile-fdefinition (car form) nil) @@ -1491,16 +1515,9 @@ when printing the error message." (setcdr sig nil)) (if sig (when (or (< ncall (car sig)) - (and (cdr sig) (> ncall (cdr sig)))) - (byte-compile-set-symbol-position (car form)) - (byte-compile-warn - "%s called with %d argument%s, but %s %s" - (car form) ncall - (if (= 1 ncall) "" "s") - (if (< ncall (car sig)) - "requires" - "accepts only") - (byte-compile-arglist-signature-string sig)))) + (and (cdr sig) (> ncall (cdr sig)))) + (byte-compile-emit-callargs-warn + (car form) ncall (car sig) (cdr sig)))) (byte-compile-format-warn form) (byte-compile-function-warn (car form) (length (cdr form)) def))) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-callargs-defsubst.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-callargs-defsubst.el new file mode 100644 index 00000000000..3a29128cf3a --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-callargs-defsubst.el @@ -0,0 +1,5 @@ +;;; -*- lexical-binding: t -*- +(defsubst warn-callargs-defsubst-f1 (_x) + nil) +(defun warn-callargs-defsubst-f2 () + (warn-callargs-defsubst-f1 1 2)) diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index 33413f5a002..7c40f7ebca3 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -700,6 +700,9 @@ byte-compiled. Run with dynamic binding." (bytecomp--define-warning-file-test "warn-callargs.el" "with 2 arguments, but accepts only 1") +(bytecomp--define-warning-file-test "warn-callargs-defsubst.el" + "with 2 arguments, but accepts only 1") + (bytecomp--define-warning-file-test "warn-defcustom-nogroup.el" "fails to specify containing group") -- cgit v1.2.3 From e91f71676c19127dd90efabfc0da36483aa53a82 Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Mon, 22 Nov 2021 08:08:11 +0100 Subject: Avoid false positives about wide docstrings for key sequences * lisp/emacs-lisp/bytecomp.el (byte-compile--wide-docstring-p): Ignore literal key sequence substitutions. * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-substitutions.el: New file. * test/lisp/emacs-lisp/bytecomp-tests.el ("warn-wide-docstring-ignore-substitutions.el"): New test. --- lisp/emacs-lisp/bytecomp.el | 7 ++++++- .../warn-wide-docstring-ignore-substitutions.el | 17 +++++++++++++++++ test/lisp/emacs-lisp/bytecomp-tests.el | 4 ++++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-substitutions.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 3338c383171..bd74c79d717 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -1674,7 +1674,12 @@ URLs." (replace-regexp-in-string (rx "\\" (or (seq "[" (* (not "]")) "]"))) (make-string byte-compile--wide-docstring-substitution-len ?x) - docstring)))) + ;; For literal key sequence substitutions (e.g. "\\`C-h'"), just + ;; remove the markup as `substitute-command-keys' would. + (replace-regexp-in-string + (rx "\\" (seq "`" (group (* (not "]"))) "'")) + "\\1" + docstring))))) (defcustom byte-compile-docstring-max-column 80 "Recommended maximum width of doc string lines. diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-substitutions.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-substitutions.el new file mode 100644 index 00000000000..37cfe463bfe --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-substitutions.el @@ -0,0 +1,17 @@ +;;; -*- lexical-binding: t -*- +(defalias 'foo #'ignore + "None of this should be considered too wide. + +; this should be treated as 60 characters - no warning +\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window]\\[quit-window] + +; 64 * 'x' does not warn +\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x'\\`x' + +; keymaps are just ignored +\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\ + +\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map}\\{foo-bar-map} + +bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar +") diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index dbc0aa3db42..816f14a18d5 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -955,6 +955,10 @@ byte-compiled. Run with dynamic binding." "warn-wide-docstring-ignore-override.el" "defvar .foo-bar. docstring wider than .* characters" 'reverse) +(bytecomp--define-warning-file-test + "warn-wide-docstring-ignore-substitutions.el" + "defvar .foo-bar. docstring wider than .* characters" 'reverse) + (bytecomp--define-warning-file-test "warn-wide-docstring-ignore.el" "defvar .foo-bar. docstring wider than .* characters" 'reverse) -- cgit v1.2.3 From 6825e5686a4bf21f5d5a0ae1af889097cfa2f597 Mon Sep 17 00:00:00 2001 From: Mattias Engdegård Date: Fri, 3 Jun 2022 20:31:10 +0200 Subject: Normalise setq during macro-expansion Early normalisation of setq during macroexpand-all allows later stages, cconv, byte-opt and codegen, to be simplified and duplicated checks to be eliminated. * lisp/emacs-lisp/macroexp.el (macroexp--expand-all): Normalise all setq forms to a sequence of (setq VAR EXPR). Emit warnings if necessary. * lisp/emacs-lisp/cconv.el (cconv-convert, cconv-analyze-form): * lisp/emacs-lisp/byte-opt.el (byte-optimize-form-code-walker): * lisp/emacs-lisp/bytecomp.el (byte-compile-setq): Simplify. * test/lisp/emacs-lisp/bytecomp-tests.el: Adapt and add tests. * test/lisp/emacs-lisp/bytecomp-resources/warn-variable-setq-nonvariable.el; * test/lisp/emacs-lisp/bytecomp-resources/warn-variable-setq-odd.el: New files. --- lisp/emacs-lisp/byte-opt.el | 41 +++++++----------- lisp/emacs-lisp/bytecomp.el | 26 ++++-------- lisp/emacs-lisp/cconv.el | 47 ++++++++------------- lisp/emacs-lisp/macroexp.el | 48 ++++++++++++++++++++++ .../warn-variable-setq-nonvariable.el | 3 ++ .../bytecomp-resources/warn-variable-setq-odd.el | 3 ++ test/lisp/emacs-lisp/bytecomp-tests.el | 8 +++- 7 files changed, 101 insertions(+), 75 deletions(-) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-variable-setq-nonvariable.el create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-variable-setq-odd.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el index 69795f9c112..0e10e332b29 100644 --- a/lisp/emacs-lisp/byte-opt.el +++ b/lisp/emacs-lisp/byte-opt.el @@ -463,32 +463,21 @@ for speeding up processing.") ;; is a *value* and shouldn't appear in the car. (`((closure . ,_) . ,_) form) - (`(setq . ,args) - (let ((var-expr-list nil)) - (while args - (unless (and (consp args) - (symbolp (car args)) (consp (cdr args))) - (byte-compile-warn-x form "malformed setq form: %S" form)) - (let* ((var (car args)) - (expr (cadr args)) - (lexvar (assq var byte-optimize--lexvars)) - (value (byte-optimize-form expr nil))) - (when lexvar - (setcar (cdr lexvar) t) ; Mark variable to be kept. - (setcdr (cdr lexvar) nil) ; Inhibit further substitution. - - (when (memq var byte-optimize--aliased-vars) - ;; Cancel aliasing of variables aliased to this one. - (dolist (v byte-optimize--lexvars) - (when (eq (nth 2 v) var) - ;; V is bound to VAR but VAR is now mutated: - ;; cancel aliasing. - (setcdr (cdr v) nil))))) - - (push var var-expr-list) - (push value var-expr-list)) - (setq args (cddr args))) - (cons fn (nreverse var-expr-list)))) + (`(setq ,var ,expr) + (let ((lexvar (assq var byte-optimize--lexvars)) + (value (byte-optimize-form expr nil))) + (when lexvar + (setcar (cdr lexvar) t) ; Mark variable to be kept. + (setcdr (cdr lexvar) nil) ; Inhibit further substitution. + + (when (memq var byte-optimize--aliased-vars) + ;; Cancel aliasing of variables aliased to this one. + (dolist (v byte-optimize--lexvars) + (when (eq (nth 2 v) var) + ;; V is bound to VAR but VAR is now mutated: + ;; cancel aliasing. + (setcdr (cdr v) nil))))) + `(,fn ,var ,value))) (`(defvar ,(and (pred symbolp) name) . ,rest) (let ((optimized-rest (and rest diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index ab21fba8a27..1f868d2217c 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -4225,25 +4225,13 @@ This function is never called when `lexical-binding' is nil." (byte-defop-compiler-1 quote) (defun byte-compile-setq (form) - (let* ((args (cdr form)) - (len (length args))) - (if (= (logand len 1) 1) - (progn - (byte-compile-report-error - (format-message - "missing value for `%S' at end of setq" (car (last args)))) - (byte-compile-form - `(signal 'wrong-number-of-arguments '(setq ,len)) - byte-compile--for-effect)) - (if args - (while args - (byte-compile-form (car (cdr args))) - (or byte-compile--for-effect (cdr (cdr args)) - (byte-compile-out 'byte-dup 0)) - (byte-compile-variable-set (car args)) - (setq args (cdr (cdr args)))) - ;; (setq), with no arguments. - (byte-compile-form nil byte-compile--for-effect))) + (cl-assert (= (length form) 3)) ; normalised in macroexp + (let ((var (nth 1 form)) + (expr (nth 2 form))) + (byte-compile-form expr) + (unless byte-compile--for-effect + (byte-compile-out 'byte-dup 0)) + (byte-compile-variable-set var) (setq byte-compile--for-effect nil))) (byte-defop-compiler-1 set-default) diff --git a/lisp/emacs-lisp/cconv.el b/lisp/emacs-lisp/cconv.el index 1a501f50bfc..b12f1db677e 100644 --- a/lisp/emacs-lisp/cconv.el +++ b/lisp/emacs-lisp/cconv.el @@ -555,29 +555,19 @@ places where they originally did not directly appear." `(,(car form) ,(cconv-convert form1 env extend) :fun-body ,(cconv--convert-function () body env form1))) - (`(setq . ,forms) ; setq special form - (if (= (logand (length forms) 1) 1) - ;; With an odd number of args, let bytecomp.el handle the error. - form - (let ((prognlist ())) - (while forms - (let* ((sym (pop forms)) - (sym-new (or (cdr (assq sym env)) sym)) - (value (cconv-convert (pop forms) env extend))) - (push (pcase sym-new - ((pred symbolp) `(,(car form) ,sym-new ,value)) - (`(car-safe ,iexp) `(setcar ,iexp ,value)) - ;; This "should never happen", but for variables which are - ;; mutated+captured+unused, we may end up trying to `setq' - ;; on a closed-over variable, so just drop the setq. - (_ ;; (byte-compile-report-error - ;; (format "Internal error in cconv of (setq %s ..)" - ;; sym-new)) - value)) - prognlist))) - (if (cdr prognlist) - `(progn . ,(nreverse prognlist)) - (car prognlist))))) + (`(setq ,var ,expr) + (let ((var-new (or (cdr (assq var env)) var)) + (value (cconv-convert expr env extend))) + (pcase var-new + ((pred symbolp) `(,(car form) ,var-new ,value)) + (`(car-safe ,iexp) `(setcar ,iexp ,value)) + ;; This "should never happen", but for variables which are + ;; mutated+captured+unused, we may end up trying to `setq' + ;; on a closed-over variable, so just drop the setq. + (_ ;; (byte-compile-report-error + ;; (format "Internal error in cconv of (setq %s ..)" + ;; sym-new)) + value)))) (`(,(and (or 'funcall 'apply) callsym) ,fun . ,args) ;; These are not special forms but we treat them separately for the needs @@ -751,14 +741,13 @@ This function does not return anything but instead fills the (cconv-analyze-form (cadr (pop body-forms)) env)) (cconv--analyze-function vrs body-forms env form)) - (`(setq . ,forms) + (`(setq ,var ,expr) ;; If a local variable (member of env) is modified by setq then ;; it is a mutated variable. - (while forms - (let ((v (assq (car forms) env))) ; v = non nil if visible - (when v (setf (nth 2 v) t))) - (cconv-analyze-form (cadr forms) env) - (setq forms (cddr forms)))) + (let ((v (assq var env))) ; v = non nil if visible + (when v + (setf (nth 2 v) t))) + (cconv-analyze-form expr env)) (`((lambda . ,_) . ,_) ; First element is lambda expression. (byte-compile-warn-x diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el index 51c6e8e0ca2..bae303c213c 100644 --- a/lisp/emacs-lisp/macroexp.el +++ b/lisp/emacs-lisp/macroexp.el @@ -369,6 +369,54 @@ Assumes the caller has bound `macroexpand-all-environment'." (macroexp--all-forms body)) (cdr form)) form))) + (`(setq ,(and var (pred symbolp) + (pred (not booleanp)) (pred (not keywordp))) + ,expr) + ;; Fast path for the setq common case. + (let ((new-expr (macroexp--expand-all expr))) + (if (eq new-expr expr) + form + `(,fn ,var ,new-expr)))) + (`(setq . ,args) + ;; Normalise to a sequence of (setq SYM EXPR). + ;; Malformed code is translated to code that signals an error + ;; at run time. + (let ((nargs (length args))) + (if (/= (logand nargs 1) 0) + (macroexp-warn-and-return + "odd number of arguments in `setq' form" + `(signal 'wrong-number-of-arguments '(setq ,nargs)) + nil 'compile-only fn) + (let ((assignments nil)) + (while (consp (cdr-safe args)) + (let* ((var (car args)) + (expr (cadr args)) + (new-expr (macroexp--expand-all expr)) + (assignment + (if (and (symbolp var) + (not (booleanp var)) (not (keywordp var))) + `(,fn ,var ,new-expr) + (macroexp-warn-and-return + (format-message "attempt to set %s `%s'" + (if (symbolp var) + "constant" + "non-variable") + var) + (cond + ((keywordp var) + ;; Accept `(setq :a :a)' for compatibility. + `(if (eq ,var ,new-expr) + ,var + (signal 'setting-constant (list ',var)))) + ((symbolp var) + `(signal 'setting-constant (list ',var))) + (t + `(signal 'wrong-type-argument + (list 'symbolp ',var)))) + nil 'compile-only var)))) + (push assignment assignments)) + (setq args (cddr args))) + (cons 'progn (nreverse assignments)))))) (`(,(and fun `(lambda . ,_)) . ,args) ;; Embedded lambda in function position. ;; If the byte-optimizer is loaded, try to unfold this, diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-setq-nonvariable.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-setq-nonvariable.el new file mode 100644 index 00000000000..5a56913cd9b --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-setq-nonvariable.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo () + (setq (a) nil)) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-setq-odd.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-setq-odd.el new file mode 100644 index 00000000000..9ce80de08cd --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-setq-odd.el @@ -0,0 +1,3 @@ +;;; -*- lexical-binding: t -*- +(defun foo (a b) + (setq a 1 b)) diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index 27098d0bb1c..9abc17a1c41 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -951,11 +951,17 @@ byte-compiled. Run with dynamic binding." "let-bind nonvariable") (bytecomp--define-warning-file-test "warn-variable-set-constant.el" - "variable reference to constant") + "attempt to set constant") (bytecomp--define-warning-file-test "warn-variable-set-nonvariable.el" "variable reference to nonvariable") +(bytecomp--define-warning-file-test "warn-variable-setq-nonvariable.el" + "attempt to set non-variable") + +(bytecomp--define-warning-file-test "warn-variable-setq-odd.el" + "odd number of arguments") + (bytecomp--define-warning-file-test "warn-wide-docstring-autoload.el" "autoload .foox. docstring wider than .* characters") -- cgit v1.2.3 From 73e75e18d170826e1838324d39ac0698948071f8 Mon Sep 17 00:00:00 2001 From: Mattias Engdegård Date: Fri, 17 Jun 2022 17:06:05 +0200 Subject: Warn about misplaced or duplicated function/macro declarations Doc strings, `declare` and `interactive` forms must appear in that order and at most once each. Complain if they don't, instead of silently ignoring the problem (bug#55905). * lisp/emacs-lisp/byte-run.el (byte-run--parse-body) (byte-run--parse-declarations): New. (defmacro, defun): Check for declaration well-formedness as described above. Clarify doc strings. Refactor some common code. * test/lisp/emacs-lisp/bytecomp-resources/fun-attr-warn.el: * test/lisp/emacs-lisp/bytecomp-tests.el (bytecomp-fun-attr-warn): New test. --- lisp/emacs-lisp/byte-run.el | 208 +++++++++------- .../emacs-lisp/bytecomp-resources/fun-attr-warn.el | 266 +++++++++++++++++++++ test/lisp/emacs-lisp/bytecomp-tests.el | 63 +++++ 3 files changed, 446 insertions(+), 91 deletions(-) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/fun-attr-warn.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/lisp/emacs-lisp/byte-run.el b/lisp/emacs-lisp/byte-run.el index 92c2699c6e3..17c15549666 100644 --- a/lisp/emacs-lisp/byte-run.el +++ b/lisp/emacs-lisp/byte-run.el @@ -272,6 +272,75 @@ This is used by `declare'.") (list 'function-put (list 'quote name) ''no-font-lock-keyword (list 'quote val)))) +(defalias 'byte-run--parse-body + #'(lambda (body allow-interactive) + "Decompose BODY into (DOCSTRING DECLARE INTERACTIVE BODY-REST WARNINGS)." + (let* ((top body) + (docstring nil) + (declare-form nil) + (interactive-form nil) + (warnings nil) + (warn #'(lambda (msg form) + (push (macroexp-warn-and-return msg nil nil t form) + warnings)))) + (while + (and body + (let* ((form (car body)) + (head (car-safe form))) + (cond + ((or (and (stringp form) (cdr body)) + (eq head :documentation)) + (cond + (docstring (funcall warn "More than one doc string" top)) + (declare-form + (funcall warn "Doc string after `declare'" declare-form)) + (interactive-form + (funcall warn "Doc string after `interactive'" + interactive-form)) + (t (setq docstring form))) + t) + ((eq head 'declare) + (cond + (declare-form + (funcall warn "More than one `declare' form" form)) + (interactive-form + (funcall warn "`declare' after `interactive'" form)) + (t (setq declare-form form))) + t) + ((eq head 'interactive) + (cond + ((not allow-interactive) + (funcall warn "No `interactive' form allowed here" form)) + (interactive-form + (funcall warn "More than one `interactive' form" form)) + (t (setq interactive-form form))) + t)))) + (setq body (cdr body))) + (list docstring declare-form interactive-form body warnings)))) + +(defalias 'byte-run--parse-declarations + #'(lambda (name arglist clauses construct declarations-alist) + (let* ((cl-decls nil) + (actions + (mapcar + #'(lambda (x) + (let ((f (cdr (assq (car x) declarations-alist)))) + (cond + (f (apply (car f) name arglist (cdr x))) + ;; Yuck!! + ((and (featurep 'cl) + (memq (car x) ;C.f. cl--do-proclaim. + '(special inline notinline optimize warn))) + (push (list 'declare x) cl-decls) + nil) + (t + (macroexp-warn-and-return + (format-message "Unknown %s property `%S'" + construct (car x)) + nil nil nil (car x)))))) + clauses))) + (cons actions cl-decls)))) + (defvar macro-declarations-alist (cons (list 'debug #'byte-run--set-debug) @@ -289,7 +358,7 @@ This is used by `declare'.") (defalias 'defmacro (cons 'macro - #'(lambda (name arglist &optional docstring &rest body) + #'(lambda (name arglist &rest body) "Define NAME as a macro. When the macro is called, as in (NAME ARGS...), the function (lambda ARGLIST BODY...) is applied to @@ -300,116 +369,73 @@ DECLS is a list of elements of the form (PROP . VALUES). These are interpreted according to `macro-declarations-alist'. The return value is undefined. -\(fn NAME ARGLIST &optional DOCSTRING DECL &rest BODY)" - ;; We can't just have `decl' as an &optional argument, because we need - ;; to distinguish - ;; (defmacro foo (arg) (bar) nil) - ;; from - ;; (defmacro foo (arg) (bar)). - (let ((decls (cond - ((eq (car-safe docstring) 'declare) - (prog1 (cdr docstring) (setq docstring nil))) - ((and (stringp docstring) - (eq (car-safe (car body)) 'declare)) - (prog1 (cdr (car body)) (setq body (cdr body))))))) - (if docstring (setq body (cons docstring body)) - (if (null body) (setq body '(nil)))) - ;; Can't use backquote because it's not defined yet! - (let* ((fun (list 'function (cons 'lambda (cons arglist body)))) - (def (list 'defalias - (list 'quote name) - (list 'cons ''macro fun))) - (declarations - (mapcar - #'(lambda (x) - (let ((f (cdr (assq (car x) macro-declarations-alist)))) - (if f (apply (car f) name arglist (cdr x)) - (macroexp-warn-and-return - (format-message - "Unknown macro property %S in %S" - (car x) name) - nil nil nil (car x))))) - decls))) - ;; Refresh font-lock if this is a new macro, or it is an - ;; existing macro whose 'no-font-lock-keyword declaration - ;; has changed. - (if (and - ;; If lisp-mode hasn't been loaded, there's no reason - ;; to flush. - (fboundp 'lisp--el-font-lock-flush-elisp-buffers) - (or (not (fboundp name)) ;; new macro - (and (fboundp name) ;; existing macro - (member `(function-put ',name 'no-font-lock-keyword - ',(get name 'no-font-lock-keyword)) - declarations)))) - (lisp--el-font-lock-flush-elisp-buffers)) - (if declarations - (cons 'prog1 (cons def declarations)) +\(fn NAME ARGLIST [DOCSTRING] [DECL] BODY...)" + (let* ((parse (byte-run--parse-body body nil)) + (docstring (nth 0 parse)) + (declare-form (nth 1 parse)) + (body (nth 3 parse)) + (warnings (nth 4 parse)) + (declarations + (and declare-form (byte-run--parse-declarations + name arglist (cdr declare-form) 'macro + macro-declarations-alist)))) + (setq body (nconc warnings body)) + (setq body (nconc (cdr declarations) body)) + (if docstring + (setq body (cons docstring body))) + (if (null body) + (setq body '(nil))) + (let* ((fun (list 'function (cons 'lambda (cons arglist body)))) + (def (list 'defalias + (list 'quote name) + (list 'cons ''macro fun)))) + (if declarations + (cons 'prog1 (cons def (car declarations))) def)))))) ;; Now that we defined defmacro we can use it! -(defmacro defun (name arglist &optional docstring &rest body) +(defmacro defun (name arglist &rest body) "Define NAME as a function. -The definition is (lambda ARGLIST [DOCSTRING] BODY...). -See also the function `interactive'. +The definition is (lambda ARGLIST [DOCSTRING] [INTERACTIVE] BODY...). DECL is a declaration, optional, of the form (declare DECLS...) where DECLS is a list of elements of the form (PROP . VALUES). These are interpreted according to `defun-declarations-alist'. +INTERACTIVE is an optional `interactive' specification. The return value is undefined. -\(fn NAME ARGLIST &optional DOCSTRING DECL &rest BODY)" - ;; We can't just have `decl' as an &optional argument, because we need - ;; to distinguish - ;; (defun foo (arg) (toto) nil) - ;; from - ;; (defun foo (arg) (toto)). +\(fn NAME ARGLIST [DOCSTRING] [DECL] [INTERACTIVE] BODY...)" (declare (doc-string 3) (indent 2)) (or name (error "Cannot define '%s' as a function" name)) (if (null (and (listp arglist) (null (delq t (mapcar #'symbolp arglist))))) (error "Malformed arglist: %s" arglist)) - (let ((decls (cond - ((eq (car-safe docstring) 'declare) - (prog1 (cdr docstring) (setq docstring nil))) - ((and (stringp docstring) - (eq (car-safe (car body)) 'declare)) - (prog1 (cdr (car body)) (setq body (cdr body))))))) - (if docstring (setq body (cons docstring body)) - (if (null body) (setq body '(nil)))) - (let ((declarations - (mapcar - #'(lambda (x) - (let ((f (cdr (assq (car x) defun-declarations-alist)))) - (cond - (f (apply (car f) name arglist (cdr x))) - ;; Yuck!! - ((and (featurep 'cl) - (memq (car x) ;C.f. cl-do-proclaim. - '(special inline notinline optimize warn))) - (push (list 'declare x) - (if (stringp docstring) - (if (eq (car-safe (cadr body)) 'interactive) - (cddr body) - (cdr body)) - (if (eq (car-safe (car body)) 'interactive) - (cdr body) - body))) - nil) - (t - (macroexp-warn-and-return - (format-message "Unknown defun property `%S' in %S" - (car x) name) - nil nil nil (car x)))))) - decls)) - (def (list 'defalias + (let* ((parse (byte-run--parse-body body t)) + (docstring (nth 0 parse)) + (declare-form (nth 1 parse)) + (interactive-form (nth 2 parse)) + (body (nth 3 parse)) + (warnings (nth 4 parse)) + (declarations + (and declare-form (byte-run--parse-declarations + name arglist (cdr declare-form) 'defun + defun-declarations-alist)))) + (setq body (nconc warnings body)) + (setq body (nconc (cdr declarations) body)) + (if interactive-form + (setq body (cons interactive-form body))) + (if docstring + (setq body (cons docstring body))) + (if (null body) + (setq body '(nil))) + (let ((def (list 'defalias (list 'quote name) (list 'function (cons 'lambda (cons arglist body)))))) (if declarations - (cons 'prog1 (cons def declarations)) - def)))) + (cons 'prog1 (cons def (car declarations))) + def)))) ;; Redefined in byte-opt.el. diff --git a/test/lisp/emacs-lisp/bytecomp-resources/fun-attr-warn.el b/test/lisp/emacs-lisp/bytecomp-resources/fun-attr-warn.el new file mode 100644 index 00000000000..be907b32f47 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/fun-attr-warn.el @@ -0,0 +1,266 @@ +;;; -*- lexical-binding: t -*- + +;; Correct + +(defun faw-str-decl-code (x) + "something" + (declare (pure t)) + (print x)) + +(defun faw-doc-decl-code (x) + (:documentation "something") + (declare (pure t)) + (print x)) + +(defun faw-str-int-code (x) + "something" + (interactive "P") + (print x)) + +(defun faw-doc-int-code (x) + (:documentation "something") + (interactive "P") + (print x)) + +(defun faw-decl-int-code (x) + (declare (pure t)) + (interactive "P") + (print x)) + +(defun faw-str-decl-int-code (x) + "something" + (declare (pure t)) + (interactive "P") + (print x)) + +(defun faw-doc-decl-int-code (x) + (:documentation "something") + (declare (pure t)) + (interactive "P") + (print x)) + + +;; Correct (last string is return value) + +(defun faw-str () + "something") + +(defun faw-decl-str () + (declare (pure t)) + "something") + +(defun faw-decl-int-str () + (declare (pure t)) + (interactive) + "something") + +(defun faw-str-str () + "something" + "something else") + +(defun faw-doc-str () + (:documentation "something") + "something else") + + +;; Incorrect (bad order) + +(defun faw-int-decl-code (x) + (interactive "P") + (declare (pure t)) + (print x)) + +(defun faw-int-str-code (x) + (interactive "P") + "something" + (print x)) + +(defun faw-int-doc-code (x) + (interactive "P") + (:documentation "something") + (print x)) + +(defun faw-decl-str-code (x) + (declare (pure t)) + "something" + (print x)) + +(defun faw-decl-doc-code (x) + (declare (pure t)) + (:documentation "something") + (print x)) + +(defun faw-str-int-decl-code (x) + "something" + (interactive "P") + (declare (pure t)) + (print x)) + +(defun faw-doc-int-decl-code (x) + (:documentation "something") + (interactive "P") + (declare (pure t)) + (print x)) + +(defun faw-int-str-decl-code (x) + (interactive "P") + "something" + (declare (pure t)) + (print x)) + +(defun faw-int-doc-decl-code (x) + (interactive "P") + (:documentation "something") + (declare (pure t)) + (print x)) + +(defun faw-int-decl-str-code (x) + (interactive "P") + (declare (pure t)) + "something" + (print x)) + +(defun faw-int-decl-doc-code (x) + (interactive "P") + (declare (pure t)) + (:documentation "something") + (print x)) + +(defun faw-decl-int-str-code (x) + (declare (pure t)) + (interactive "P") + "something" + (print x)) + +(defun faw-decl-int-doc-code (x) + (declare (pure t)) + (interactive "P") + (:documentation "something") + (print x)) + +(defun faw-decl-str-int-code (x) + (declare (pure t)) + "something" + (interactive "P") + (print x)) + +(defun faw-decl-doc-int-code (x) + (declare (pure t)) + (:documentation "something") + (interactive "P") + (print x)) + + +;; Incorrect (duplication) + +(defun faw-str-str-decl-int-code (x) + "something" + "something else" + (declare (pure t)) + (interactive "P") + (print x)) + +(defun faw-str-doc-decl-int-code (x) + "something" + (:documentation "something else") + (declare (pure t)) + (interactive "P") + (print x)) + +(defun faw-doc-str-decl-int-code (x) + (:documentation "something") + "something else" + (declare (pure t)) + (interactive "P") + (print x)) + +(defun faw-doc-doc-decl-int-code (x) + (:documentation "something") + (:documentation "something else") + (declare (pure t)) + (interactive "P") + (print x)) + +(defun faw-str-decl-str-int-code (x) + "something" + (declare (pure t)) + "something else" + (interactive "P") + (print x)) + +(defun faw-doc-decl-str-int-code (x) + (:documentation "something") + (declare (pure t)) + "something else" + (interactive "P") + (print x)) + +(defun faw-str-decl-doc-int-code (x) + "something" + (declare (pure t)) + (:documentation "something else") + (interactive "P") + (print x)) + +(defun faw-doc-decl-doc-int-code (x) + (:documentation "something") + (declare (pure t)) + (:documentation "something else") + (interactive "P") + (print x)) + +(defun faw-str-decl-decl-int-code (x) + "something" + (declare (pure t)) + (declare (indent 1)) + (interactive "P") + (print x)) + +(defun faw-doc-decl-decl-int-code (x) + (:documentation "something") + (declare (pure t)) + (declare (indent 1)) + (interactive "P") + (print x)) + +(defun faw-str-decl-int-decl-code (x) + "something" + (declare (pure t)) + (interactive "P") + (declare (indent 1)) + (print x)) + +(defun faw-doc-decl-int-decl-code (x) + (:documentation "something") + (declare (pure t)) + (interactive "P") + (declare (indent 1)) + (print x)) + +(defun faw-str-decl-int-int-code (x) + "something" + (declare (pure t)) + (interactive "P") + (interactive "p") + (print x)) + +(defun faw-doc-decl-int-int-code (x) + (:documentation "something") + (declare (pure t)) + (interactive "P") + (interactive "p") + (print x)) + +(defun faw-str-int-decl-int-code (x) + "something" + (interactive "P") + (declare (pure t)) + (interactive "p") + (print x)) + +(defun faw-doc-int-decl-int-code (x) + (:documentation "something") + (interactive "P") + (declare (pure t)) + (interactive "p") + (print x)) diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index 9abc17a1c41..fbc00b30c54 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -1580,6 +1580,69 @@ EXPECTED-POINT BINDINGS (MODES \\='\\='(ruby-mode js-mode python-mode)) \ (should (equal (get fname 'lisp-indent-function) 1)) (should (equal (aref bc 4) "tata\n\n(fn X)"))))) +(ert-deftest bytecomp-fun-attr-warn () + ;; Check that warnings are emitted when doc strings, `declare' and + ;; `interactive' forms don't come in the proper order, or more than once. + (let* ((filename "fun-attr-warn.el") + (el (ert-resource-file filename)) + (elc (concat el "c")) + (text-quoting-style 'grave)) + (with-current-buffer (get-buffer-create "*Compile-Log*") + (let ((inhibit-read-only t)) + (erase-buffer)) + (byte-compile-file el) + (let ((expected + '("70:4: Warning: `declare' after `interactive'" + "74:4: Warning: Doc string after `interactive'" + "79:4: Warning: Doc string after `interactive'" + "84:4: Warning: Doc string after `declare'" + "89:4: Warning: Doc string after `declare'" + "96:4: Warning: `declare' after `interactive'" + "102:4: Warning: `declare' after `interactive'" + "108:4: Warning: `declare' after `interactive'" + "106:4: Warning: Doc string after `interactive'" + "114:4: Warning: `declare' after `interactive'" + "112:4: Warning: Doc string after `interactive'" + "118:4: Warning: Doc string after `interactive'" + "119:4: Warning: `declare' after `interactive'" + "124:4: Warning: Doc string after `interactive'" + "125:4: Warning: `declare' after `interactive'" + "130:4: Warning: Doc string after `declare'" + "136:4: Warning: Doc string after `declare'" + "142:4: Warning: Doc string after `declare'" + "148:4: Warning: Doc string after `declare'" + "159:4: Warning: More than one doc string" + "165:4: Warning: More than one doc string" + "171:4: Warning: More than one doc string" + "178:4: Warning: More than one doc string" + "186:4: Warning: More than one doc string" + "192:4: Warning: More than one doc string" + "200:4: Warning: More than one doc string" + "206:4: Warning: More than one doc string" + "215:4: Warning: More than one `declare' form" + "222:4: Warning: More than one `declare' form" + "230:4: Warning: More than one `declare' form" + "237:4: Warning: More than one `declare' form" + "244:4: Warning: More than one `interactive' form" + "251:4: Warning: More than one `interactive' form" + "258:4: Warning: More than one `interactive' form" + "257:4: Warning: `declare' after `interactive'" + "265:4: Warning: More than one `interactive' form" + "264:4: Warning: `declare' after `interactive'"))) + (goto-char (point-min)) + (let ((actual nil)) + (while (re-search-forward + (rx bol (* (not ":")) ":" + (group (+ digit) ":" (+ digit) ": Warning: " + (or "More than one " (+ nonl) " form" + (: (+ nonl) " after " (+ nonl)))) + eol) + nil t) + (push (match-string 1) actual)) + (setq actual (nreverse actual)) + (should (equal actual expected))))))) + + ;; Local Variables: ;; no-byte-compile: t ;; End: -- cgit v1.2.3 From dbbf38d43f1f49a38efd260bda655e0b3cd2b6d5 Mon Sep 17 00:00:00 2001 From: Philipp Stephani Date: Tue, 21 Jun 2022 19:10:14 +0200 Subject: Document and test 'no-byte-compile' behavior. * lisp/emacs-lisp/bytecomp.el (byte-compile-file): Document behavior if 'no-byte-compile' is set. * test/lisp/emacs-lisp/bytecomp-tests.el (byte-compile-file/no-byte-compile): New unit test. * test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el: New test file. --- lisp/emacs-lisp/bytecomp.el | 3 +++ test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el | 1 + test/lisp/emacs-lisp/bytecomp-tests.el | 7 +++++++ 3 files changed, 11 insertions(+) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 04c107a7cfa..4fd65bb5d53 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -2089,6 +2089,9 @@ If compilation is needed, this functions returns the result of The output file's name is generated by passing FILENAME to the function `byte-compile-dest-file' (which see). The value is non-nil if there were no errors, nil if errors. +If the file sets the file variable `no-byte-compile', it is not +compiled, any existing output file is removed, and the return +value is `no-byte-compile'. See also `emacs-lisp-byte-compile-and-load'." (declare (advertised-calling-convention (filename) "28.1")) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el b/test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el new file mode 100644 index 00000000000..00ad1947507 --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el @@ -0,0 +1 @@ +;; -*- no-byte-compile: t; -*- diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index fbc00b30c54..9c5bef09a34 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -1642,6 +1642,13 @@ EXPECTED-POINT BINDINGS (MODES \\='\\='(ruby-mode js-mode python-mode)) \ (setq actual (nreverse actual)) (should (equal actual expected))))))) +(ert-deftest byte-compile-file/no-byte-compile () + (let* ((src-file (ert-resource-file "no-byte-compile.el")) + (dest-file (make-temp-file "bytecomp-tests-" nil ".elc")) + (byte-compile-dest-file-function (lambda (_) dest-file))) + (should (eq (byte-compile-file src-file) 'no-byte-compile)) + (should-not (file-exists-p dest-file)))) + ;; Local Variables: ;; no-byte-compile: t -- cgit v1.2.3 From 6938a2ddd2d9861a0f04e79d05ba976bdf91cc8c Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Fri, 16 Sep 2022 22:24:20 +0200 Subject: Accept more wide function signatures in docstrings * test/lisp/emacs-lisp/bytecomp-tests.el ("warn-wide-docstring-ignore-function-signature.el"): New test. * lisp/emacs-lisp/bytecomp.el (byte-compile--wide-docstring-p): Make regexp more allowing to silence warning. * test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-function-signature.el: New file. --- lisp/emacs-lisp/bytecomp.el | 2 +- .../warn-wide-docstring-ignore-function-signature.el | 4 ++++ test/lisp/emacs-lisp/bytecomp-tests.el | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-function-signature.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 48929e62bdf..3b3f7bb6190 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -1705,7 +1705,7 @@ URLs." (+ " " (or ;; Arguments. (+ (or (syntax symbol) - (any word "-/:[]&=().?^\\#'"))) + (any word "-/:[]&=()<>.,?^\\#*'\""))) ;; Argument that is a list. (seq "(" (* (not ")")) ")"))) ")"))) diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-function-signature.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-function-signature.el new file mode 100644 index 00000000000..e83f516e58c --- /dev/null +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-function-signature.el @@ -0,0 +1,4 @@ +;;; -*- lexical-binding: t -*- +(defun foo-bar () + "This should not warn: +(fn COMMAND &rest ARGS &key (MARGIN (rx bol (+ \" \"))) (ARGUMENT (rx \"-\" (+ (any \"-\" alnum)) (32 \"=\"))) (METAVAR (rx (32 \" \") (or (+ (any alnum \"_-\")) (seq \"[\" (+? nonl) \"]\") (seq \"<\" (+? nonl) \">\") (seq \"{\" (+? nonl) \"}\")))) (SEPARATOR (rx \", \" symbol-start)) (DESCRIPTION (rx (* nonl) (* \"\\=\\n\" (>= 9 \" \") (* nonl)))) NARROW-START NARROW-END)") diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index bc9f8d802a6..1ca44dc7a48 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -1006,6 +1006,10 @@ byte-compiled. Run with dynamic binding." "warn-wide-docstring-ignore-fill-column.el" "defvar .foo-bar. docstring wider than .* characters" 'reverse) +(bytecomp--define-warning-file-test + "warn-wide-docstring-ignore-function-signature.el" + "defvar .foo-bar. docstring wider than .* characters" 'reverse) + (bytecomp--define-warning-file-test "warn-wide-docstring-ignore-override.el" "defvar .foo-bar. docstring wider than .* characters" 'reverse) -- cgit v1.2.3 From e4964de952a8246307faaf9875d2c278f42c53fc Mon Sep 17 00:00:00 2001 From: Mattias Engdegård Date: Thu, 22 Sep 2022 14:15:56 +0200 Subject: Don't rewrite `set` to `setq` of lexical variables Only perform the rewrite (set 'VAR X) -> (setq VAR X) for dynamic variables, as `set` isn't supposed to affect lexical vars (and never does so when interpreted). * lisp/emacs-lisp/byte-opt.el (byte-optimize-set): * test/lisp/emacs-lisp/bytecomp-tests.el (bytecomp-tests--xx): New. (bytecomp-tests--test-cases): Add test cases. * test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-nonvariable.el: Remove obsolete test. --- lisp/emacs-lisp/byte-opt.el | 19 ++++++++++--------- .../warn-variable-set-nonvariable.el | 3 --- test/lisp/emacs-lisp/bytecomp-tests.el | 15 ++++++++++++--- 3 files changed, 22 insertions(+), 15 deletions(-) delete mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-nonvariable.el (limited to 'test/lisp/emacs-lisp/bytecomp-resources') diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el index 0d5f8c26eb2..4ef9cb0a1e4 100644 --- a/lisp/emacs-lisp/byte-opt.el +++ b/lisp/emacs-lisp/byte-opt.el @@ -1531,15 +1531,16 @@ See Info node `(elisp) Integer Basics'." (put 'set 'byte-optimizer #'byte-optimize-set) (defun byte-optimize-set (form) - (let ((var (car-safe (cdr-safe form)))) - (cond - ((and (eq (car-safe var) 'quote) (consp (cdr var))) - `(setq ,(cadr var) ,@(cddr form))) - ((and (eq (car-safe var) 'make-local-variable) - (eq (car-safe (setq var (car-safe (cdr var)))) 'quote) - (consp (cdr var))) - `(progn ,(cadr form) (setq ,(cadr var) ,@(cddr form)))) - (t form)))) + (pcase (cdr form) + ;; Make sure we only turn `set' into `setq' for dynamic variables. + (`((quote ,(and var (guard (and (symbolp var) + (not (macroexp--const-symbol-p var)) + (not (assq var byte-optimize--lexvars)))))) + ,newval) + `(setq ,var ,newval)) + (`(,(and ml `(make-local-variable ,(and v `(quote ,_)))) ,newval) + `(progn ,ml (,(car form) ,v ,newval))) + (_ form))) ;; enumerating those functions which need not be called if the returned ;; value is not used. That is, something like diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-nonvariable.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-nonvariable.el deleted file mode 100644 index 0c76c4d388b..00000000000 --- a/test/lisp/emacs-lisp/bytecomp-resources/warn-variable-set-nonvariable.el +++ /dev/null @@ -1,3 +0,0 @@ -;;; -*- lexical-binding: t -*- -(defun foo () - (set '(a) nil)) diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el index 1ca44dc7a48..e7c308213e4 100644 --- a/test/lisp/emacs-lisp/bytecomp-tests.el +++ b/test/lisp/emacs-lisp/bytecomp-tests.el @@ -59,6 +59,8 @@ inner loops respectively." (setq i (1- i))) res)) +(defvar bytecomp-tests--xx nil) + (defconst bytecomp-tests--test-cases '( ;; some functional tests @@ -692,6 +694,16 @@ inner loops respectively." (f (lambda () (let ((y x)) (list y 3 y))))) (funcall f)) + + ;; Test rewriting of `set' to `setq' (only done on dynamic variables). + (let ((xx 1)) (set 'xx 2) xx) + (let ((bytecomp-tests--xx 1)) + (set 'bytecomp-tests--xx 2) + bytecomp-tests--xx) + (let ((aaa 1)) (set (make-local-variable 'aaa) 2) aaa) + (let ((bytecomp-tests--xx 1)) + (set (make-local-variable 'bytecomp-tests--xx) 2) + bytecomp-tests--xx) ) "List of expressions for cross-testing interpreted and compiled code.") @@ -953,9 +965,6 @@ byte-compiled. Run with dynamic binding." (bytecomp--define-warning-file-test "warn-variable-set-constant.el" "attempt to set constant") -(bytecomp--define-warning-file-test "warn-variable-set-nonvariable.el" - "variable reference to nonvariable") - (bytecomp--define-warning-file-test "warn-variable-setq-nonvariable.el" "attempt to set non-variable") -- cgit v1.2.3