[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#5950: defvaralias after defvar should be warned in runtime
From: |
Noam Postavsky |
Subject: |
bug#5950: defvaralias after defvar should be warned in runtime |
Date: |
Sat, 26 May 2018 12:52:48 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
tags 5950 + patch
quit
Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
> Changing defvaralias to try and be more clever would definitely
> be wrong. But changing it to output a warning about the problematic
> situation would be OK and changing the byte-compiler to output a warning
> in cases that make such a situation more likely is also perfectly good.
I see the byte-compiler side of this has been taken care of [1..4], but
I still managed to make this mistake when I made a cross-file alias
(Bug#31603). Here's a patch for the runtime warning. While writing it,
I found that invoking `display-warning' failed during bootstrap, so the
first commit works around that. In the end it's not strictly necessary,
because I added a check to suppress the warning when the two variables
have `eq' values, so no warnings are generated during bootstrap anyway.
The bootstrap warnings were about cl-custom-print-functions,
mail-dont-reply-to-names, and several flymake-proc-* variables all of
which correctly have the alias before the definition. This makes me
think I might be doing something wrong, but it seems to do the right
thing in my tests.
>From e039d38c653059716b68f64fdc7b395bf7593834 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Fri, 25 May 2018 21:37:17 -0400
Subject: [PATCH v1 1/2] Let display-warning work during bootstrap
* lisp/emacs-lisp/warnings.el (display-warning): Only call
`special-mode' and `newline' if they are `fbound'.
---
lisp/emacs-lisp/warnings.el | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/lisp/emacs-lisp/warnings.el b/lisp/emacs-lisp/warnings.el
index 665733181c..c4d97ceab0 100644
--- a/lisp/emacs-lisp/warnings.el
+++ b/lisp/emacs-lisp/warnings.el
@@ -241,11 +241,15 @@ display-warning
(old (get-buffer buffer-name))
(buffer (or old (get-buffer-create buffer-name)))
(level-info (assq level warning-levels))
+ ;; `newline' may be unbound during bootstrap.
+ (newline (if (fboundp 'newline) #'newline
+ (lambda () (insert "\n"))))
start end)
(with-current-buffer buffer
;; If we created the buffer, disable undo.
(unless old
- (special-mode)
+ (when (fboundp 'special-mode) ; Undefined during bootstrap.
+ (special-mode))
(setq buffer-read-only t)
(setq buffer-undo-list t))
(goto-char (point-max))
@@ -256,7 +260,7 @@ display-warning
(funcall warning-series)))))
(let ((inhibit-read-only t))
(unless (bolp)
- (newline))
+ (funcall newline))
(setq start (point))
(if warning-prefix-function
(setq level-info (funcall warning-prefix-function
@@ -264,7 +268,7 @@ display-warning
(insert (format (nth 1 level-info)
(format warning-type-format typename))
message)
- (newline)
+ (funcall newline)
(when (and warning-fill-prefix (not (string-match "\n" message)))
(let ((fill-prefix warning-fill-prefix)
(fill-column 78))
--
2.11.0
>From 6fe5dee5f4ad0a6e18d73658d0392d4444ff1826 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Fri, 25 May 2018 08:40:55 -0400
Subject: [PATCH v1 2/2] Give warning if losing value to defvaralias (Bug#5950)
* src/eval.c (Fdefvaralias): Call `display-warning' if the alias
target has a non-eq value to the variable being aliased.
* test/src/eval-tests.el (defvaralias-overwrite-warning): New test.
---
src/eval.c | 10 ++++++++++
test/src/eval-tests.el | 22 ++++++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/src/eval.c b/src/eval.c
index 90d8c33518..354ab2c2d1 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -623,6 +623,16 @@ DEFUN ("defvaralias", Fdefvaralias, Sdefvaralias, 2, 3, 0,
if (NILP (Fboundp (base_variable)))
set_internal (base_variable, find_symbol_value (new_alias),
Qnil, SET_INTERNAL_BIND);
+ else if (!NILP (Fboundp (new_alias))
+ && !EQ (find_symbol_value (new_alias),
+ find_symbol_value (base_variable)))
+ call2 (intern ("display-warning"),
+ list3 (intern ("defvaralias"), intern ("losing-value"), new_alias),
+ CALLN (Fformat_message,
+ build_string
+ ("Overwriting value of `%s' by aliasing to `%s'"),
+ new_alias, base_variable));
+
{
union specbinding *p;
diff --git a/test/src/eval-tests.el b/test/src/eval-tests.el
index 319dd91c86..281d959b53 100644
--- a/test/src/eval-tests.el
+++ b/test/src/eval-tests.el
@@ -26,6 +26,7 @@
;;; Code:
(require 'ert)
+(eval-when-compile (require 'cl-lib))
(ert-deftest eval-tests--bug24673 ()
"Check that Bug#24673 has been fixed."
@@ -117,4 +118,25 @@ eval-tests--exceed-specbind-limit
"Check that Bug#31072 is fixed."
(should-error (eval '(defvar 1) t) :type 'wrong-type-argument))
+(ert-deftest defvaralias-overwrite-warning ()
+ "Test for Bug#5950."
+ (defvar eval-tests--foo)
+ (setq eval-tests--foo 2)
+ (defvar eval-tests--foo-alias)
+ (setq eval-tests--foo-alias 1)
+ (cl-letf (((symbol-function 'display-warning)
+ (lambda (type &rest _)
+ (throw 'got-warning type))))
+ ;; Warn if we lose a value through aliasing.
+ (should (equal
+ '(defvaralias losing-value eval-tests--foo-alias)
+ (catch 'got-warning
+ (defvaralias 'eval-tests--foo-alias 'eval-tests--foo))))
+ ;; Don't warn if we don't.
+ (makunbound 'eval-tests--foo-alias)
+ (should (eq 'no-warning
+ (catch 'got-warning
+ (defvaralias 'eval-tests--foo-alias 'eval-tests--foo)
+ 'no-warning)))))
+
;;; eval-tests.el ends here
--
2.11.0
[1: 495963cfaf]: 2018-04-20 17:22:47 -0400
* lisp/emacs-lisp/bytecomp.el (byte-compile-file-form-defvar-function):
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=495963cfaf535646350051f47c085b84319572f0
[2: 9c3eeba4db]: 2018-04-20 18:34:39 -0400
The tedious game of whack-a-mole with compiler warnings continues
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9c3eeba4db26ddaeead100beea7a96f9fa640918
[3: 18de2ada24]: 2018-04-20 18:55:04 -0400
More alias-related tedium
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=18de2ada243653ece98b18044233e5d29eee5903
[4: 94e794c8d8]: 2018-04-20 19:02:16 -0400
Tweak recent bytecomp defvaralias change
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=94e794c8d8b93a1d6813742da12135f2746ef80b
- bug#5950: defvaralias after defvar should be warned in runtime,
Noam Postavsky <=