From: Noam Postavsky <npostavs@gmail.com>
To: Stefan Monnier <monnier@IRO.UMontreal.CA>
Cc: 5950@debbugs.gnu.org, IRIE Shinsuke <irieshinsuke@yahoo.co.jp>
Subject: bug#5950: defvaralias after defvar should be warned in runtime
Date: Sat, 26 May 2018 12:52:48 -0400 [thread overview]
Message-ID: <874liufjfj.fsf@gmail.com> (raw)
In-Reply-To: <jwvtylqhwi7.fsf-monnier+emacs@gnu.org> (Stefan Monnier's message of "Thu, 16 Sep 2010 11:27:57 +0200")
[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1942 bytes --]
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch --]
[-- Type: text/x-diff, Size: 2718 bytes --]
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
[-- Attachment #4: Type: text/plain, Size: 782 bytes --]
[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
next prev parent reply other threads:[~2018-05-26 16:52 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-04-15 5:13 bug#5950: defvaralias after defvar should be warned in runtime IRIE Shinsuke
2010-09-15 20:16 ` Glenn Morris
2010-09-16 0:44 ` IRIE Shinsuke
2010-09-16 3:51 ` Glenn Morris
2010-09-16 9:27 ` Stefan Monnier
2018-05-26 16:52 ` Noam Postavsky [this message]
2018-05-26 19:46 ` Stefan Monnier
2018-05-26 23:54 ` Noam Postavsky
2018-06-12 11:48 ` Noam Postavsky
[not found] ` <cb864996-c592-5507-f0c6-be07d17f13ee@gmail.com>
[not found] ` <jwvlg9pkbip.fsf-monnier+emacs@gnu.org>
2018-08-02 13:08 ` Clément Pit-Claudel
2018-08-02 13:28 ` Noam Postavsky
2018-08-02 14:37 ` Clément Pit-Claudel
2018-08-02 17:03 ` Stefan Monnier
2018-08-03 3:33 ` Clément Pit-Claudel
2018-08-03 20:23 ` Noam Postavsky
2018-08-03 21:00 ` Clément Pit-Claudel
2018-08-03 22:09 ` Noam Postavsky
2018-08-03 22:24 ` Clément Pit-Claudel
2018-08-03 21:58 ` Stefan Monnier
2018-08-02 2:14 ` Noam Postavsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874liufjfj.fsf@gmail.com \
--to=npostavs@gmail.com \
--cc=5950@debbugs.gnu.org \
--cc=irieshinsuke@yahoo.co.jp \
--cc=monnier@IRO.UMontreal.CA \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.