unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#5950: defvaralias after defvar should be warned in runtime
@ 2010-04-15  5:13 IRIE Shinsuke
  2010-09-15 20:16 ` Glenn Morris
  2018-08-02  2:14 ` Noam Postavsky
  0 siblings, 2 replies; 20+ messages in thread
From: IRIE Shinsuke @ 2010-04-15  5:13 UTC (permalink / raw)
  To: 5950

Please consider below:

We assume an user wrote .emacs file as:

 (setq old-foo 123)
 (load "bar")

Here, bar.el includes the following:

 (defvar foo 456)
 (define-obsolete-variable-alias 'old-foo 'foo)

By this .emacs setting, the user probably hopes `old-foo' remains 123
after loading bar.el, but actually it will be changed to 456. This
behavior is inconsistent with the principle that `defvar' must not
override existing value, so bar.el should be modified as:

 (define-obsolete-variable-alias 'old-foo 'foo)
 (defvar foo 456)

I think the programs like bar.el should be warned when running (and
byte-compiling, if possible).

-- 
IRIE Shinsuke, Ph.D.







^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  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
  2018-08-02  2:14 ` Noam Postavsky
  1 sibling, 1 reply; 20+ messages in thread
From: Glenn Morris @ 2010-09-15 20:16 UTC (permalink / raw)
  To: IRIE Shinsuke; +Cc: 5950

IRIE Shinsuke wrote:

> We assume an user wrote .emacs file as:
>
>  (setq old-foo 123)
>  (load "bar")
>
> Here, bar.el includes the following:
>
>  (defvar foo 456)
>  (define-obsolete-variable-alias 'old-foo 'foo)
>
> By this .emacs setting, the user probably hopes `old-foo' remains 123
> after loading bar.el, but actually it will be changed to 456.
[...]
> I think the programs like bar.el should be warned when running (and
> byte-compiling, if possible).


You can never have too many compilation warnings...

Here is an attempt at this. Comments:

1) It gives a spurious warning in cases where the
define-obsolete-variable-alias statement is autoloaded. One could
simply reposition such statements anyway.

2) I never know when to use normal-call or keep-pending.

It does reveal a couple of bugs in the current trunk.


*** lisp/emacs-lisp/bytecomp.el	2010-09-15 15:30:43 +0000
--- lisp/emacs-lisp/bytecomp.el	2010-09-15 18:59:50 +0000
***************
*** 3874,3879 ****
--- 3874,3892 ----
    (byte-compile-set-symbol-position 'lambda)
    (error "`lambda' used as function name is invalid"))
  
+ (put 'define-obsolete-variable-alias 'byte-hunk-handler
+      'byte-compile-define-obsolete-variable-alias)
+ ;; Gives a spurious warning if the d-o-v-a is autoloaded.
+ (defun byte-compile-define-obsolete-variable-alias (form)
+   (and (byte-compile-warning-enabled-p 'suspicious)
+        (eq (car-safe (nth 2 form)) 'quote)
+        (memq (car-safe (cdr (nth 2 form))) byte-compile-bound-variables)
+        (not (memq (cadr (nth 2 form)) byte-compile-const-variables))
+        (byte-compile-warn "variable `%s' aliased after definition"
+ 			  (cadr (nth 2 form))))
+   (byte-compile-keep-pending form)
+   nil)
+ 
  ;; Compile normally, but deal with warnings for the function being defined.
  (put 'defalias 'byte-hunk-handler 'byte-compile-file-form-defalias)
  (defun byte-compile-file-form-defalias (form)






^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2010-09-15 20:16 ` Glenn Morris
@ 2010-09-16  0:44   ` IRIE Shinsuke
  2010-09-16  3:51     ` Glenn Morris
  0 siblings, 1 reply; 20+ messages in thread
From: IRIE Shinsuke @ 2010-09-16  0:44 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 5950

I think the byte-hunk-handler should be put to `defvaralias' rather than
`define-obsolete-variable-alias'. `defvaralias' is a primitive function
which causes the problem in `define-obsolete-variable-alias' macro.

Thanks.

IRIE Shinsuke





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2010-09-16  0:44   ` IRIE Shinsuke
@ 2010-09-16  3:51     ` Glenn Morris
  2010-09-16  9:27       ` Stefan Monnier
  0 siblings, 1 reply; 20+ messages in thread
From: Glenn Morris @ 2010-09-16  3:51 UTC (permalink / raw)
  To: IRIE Shinsuke; +Cc: 5950

IRIE Shinsuke wrote:

> I think the byte-hunk-handler should be put to `defvaralias' rather than
> `define-obsolete-variable-alias'. `defvaralias' is a primitive function
> which causes the problem in `define-obsolete-variable-alias' macro.

The issue _is_ with defvaralias, but it's only a problem for user
options, things that might be set in .emacs before the associated
alias definition is evaluated. It's hard to see why there should be
non-obsolete aliases to user options, it just causes confusion. I
think there would be false positives if the warning was associated
with defvaralias, which is mostly used with non-user-options, I would
think.

The last time a similar issue came up, Stefan preferred that only
define-obsolete-variable-alias be changed; see

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=4706#21





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2010-09-16  3:51     ` Glenn Morris
@ 2010-09-16  9:27       ` Stefan Monnier
  2018-05-26 16:52         ` Noam Postavsky
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Monnier @ 2010-09-16  9:27 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 5950

>> I think the byte-hunk-handler should be put to `defvaralias' rather than
>> `define-obsolete-variable-alias'. `defvaralias' is a primitive function
>> which causes the problem in `define-obsolete-variable-alias' macro.
> The issue _is_ with defvaralias, but it's only a problem for user
> options, things that might be set in .emacs before the associated
> alias definition is evaluated.  It's hard to see why there should be
> non-obsolete aliases to user options, it just causes confusion.  I
> think there would be false positives if the warning was associated
> with defvaralias, which is mostly used with non-user-options, I would
> think.

> The last time a similar issue came up, Stefan preferred that only
> define-obsolete-variable-alias be changed; see

Well, in this case I'd rather check defvaralias, since the problem is
there: if (defvaralias 'foo 'bar) is executed after both `foo' and `bar'
have been given values, then it will necessarily have to drop one of the
two, which is the source of the problem.

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.


        Stefan





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2010-09-16  9:27       ` Stefan Monnier
@ 2018-05-26 16:52         ` Noam Postavsky
  2018-05-26 19:46           ` Stefan Monnier
  2018-05-26 23:54           ` Noam Postavsky
  0 siblings, 2 replies; 20+ messages in thread
From: Noam Postavsky @ 2018-05-26 16:52 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 5950, IRIE Shinsuke

[-- 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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2018-05-26 16:52         ` Noam Postavsky
@ 2018-05-26 19:46           ` Stefan Monnier
  2018-05-26 23:54           ` Noam Postavsky
  1 sibling, 0 replies; 20+ messages in thread
From: Stefan Monnier @ 2018-05-26 19:46 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 5950, IRIE Shinsuke

> +  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));

I'd make it just a `message` rather than a `display-warning`.


        Stefan





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2018-05-26 16:52         ` Noam Postavsky
  2018-05-26 19:46           ` Stefan Monnier
@ 2018-05-26 23:54           ` Noam Postavsky
  2018-06-12 11:48             ` Noam Postavsky
  1 sibling, 1 reply; 20+ messages in thread
From: Noam Postavsky @ 2018-05-26 23:54 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: IRIE Shinsuke, 5950

>> The bootstrap warnings were about cl-custom-print-functions,
>> mail-dont-reply-to-names,

Oh, it's because the definition gets into loaddefs.el, but the alias
doesn't.

>> and several flymake-proc-* variables all of
>> which correctly have the alias before the definition.

Not 100% sure about the flymake ones, but I guess it's some double
loading during bootstrap.


Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> I'd make it just a `message` rather than a `display-warning`.

Hmm, I think it's too easy to overlook that, I prefer the in-your-face
nature of display-warning.






^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2018-05-26 23:54           ` Noam Postavsky
@ 2018-06-12 11:48             ` Noam Postavsky
       [not found]               ` <cb864996-c592-5507-f0c6-be07d17f13ee@gmail.com>
  0 siblings, 1 reply; 20+ messages in thread
From: Noam Postavsky @ 2018-06-12 11:48 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 5950, IRIE Shinsuke

tags 5950 fixed
close 5950 27.1
quit

Noam Postavsky <npostavs@gmail.com> writes:

> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>
>> I'd make it just a `message` rather than a `display-warning`.
>
> Hmm, I think it's too easy to overlook that, I prefer the in-your-face
> nature of display-warning.

Pushed to master.

[1: c912db0836]: 2018-06-12 07:40:33 -0400
  Give warning if losing value to defvaralias (Bug#5950)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=c912db0836f6975519dea57fb0a4adc2988da1b1





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2010-04-15  5:13 bug#5950: defvaralias after defvar should be warned in runtime IRIE Shinsuke
  2010-09-15 20:16 ` Glenn Morris
@ 2018-08-02  2:14 ` Noam Postavsky
  1 sibling, 0 replies; 20+ messages in thread
From: Noam Postavsky @ 2018-08-02  2:14 UTC (permalink / raw)
  To: 5950; +Cc: Clément Pit-Claudel

[-- Attachment #1: Type: text/plain, Size: 72 bytes --]

[forwarding to list, original didn't make it because bug was archived]


[-- Attachment #2: Type: message/rfc822, Size: 1764 bytes --]

From: "Clément Pit-Claudel" <cpitclaudel@gmail.com>
To: Noam Postavsky <npostavs@gmail.com>, Stefan Monnier <monnier@iro.umontreal.ca>
Subject: Re: bug#5950: defvaralias after defvar should be warned in runtime
Date: Wed, 1 Aug 2018 14:55:56 -0400
Message-ID: <cb864996-c592-5507-f0c6-be07d17f13ee@gmail.com>

Hi Noam,

I just updated my Emacs and ran into this warning, but I have no idea
what's wrong with the following code :)

  (defvaralias 'flycheck-python-pylint-executable 'python-shell-interpreter)
  (defvaralias 'flycheck-python-flake8-executable 'python-shell-interpreter)

These two variables `flycheck-&-executable' are configuration knobs
created by Flycheck, and I never want to set them individually; instead,
I want them to have the same value as `python-shell-interpreter'.  While
the code above works, it also pops a "warning" buffer now:

Warning (defvaralias): Overwriting value of ‘flycheck-python-pylint-executable’ by aliasing to ‘python-shell-interpreter’
Warning (defvaralias): Overwriting value of ‘flycheck-python-flake8-executable’ by aliasing to ‘python-shell-interpreter’

I've set warning-suppress-log-types to '(defvaralias losing-value) to
work around this, but was the warning actually intended in this case?

Clément.

[-- Attachment #3: Type: text/plain, Size: 810 bytes --]



It's warning because you overwrite the original value of
flycheck-python-pylint-executable when you call defvaralias.  In this
case you intended that, but usually it's a mistake.

You could make the warning suppression more selective by adding
(defvaralias losing-value flycheck-python-pylint-executable) and
(defvaralias losing-value flycheck-python-flake8-executable) to
warning-suppress-log-types.

Alternatively, you could do

  (setq flycheck-python-pylint-executable python-shell-interpreter)
  (defvaralias 'flycheck-python-pylint-executable 'python-shell-interpreter)
  (setq flycheck-python-flake8-executable python-shell-interpreter)
  (defvaralias 'flycheck-python-flake8-executable 'python-shell-interpreter)

(the warning doesn't trigger if the value being overwritten is `eq' to
the new one).


^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
       [not found]                 ` <jwvlg9pkbip.fsf-monnier+emacs@gnu.org>
@ 2018-08-02 13:08                   ` Clément Pit-Claudel
  2018-08-02 13:28                     ` Noam Postavsky
  0 siblings, 1 reply; 20+ messages in thread
From: Clément Pit-Claudel @ 2018-08-02 13:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: IRIE Shinsuke, 5950, Noam Postavsky

On 2018-08-01 17:46, Stefan Monnier wrote:
>> Warning (defvaralias): Overwriting value of
>> ‘flycheck-python-pylint-executable’ by aliasing to
>> ‘python-shell-interpreter’
> 
> This warning tells you that the variable
> `flycheck-python-pylint-executable` already had a value before the call
> to `defvaralias`, and that this value is lost at this point.
> 
> Usually the warning is due to the defvaralias call being performed after
> the corresponding `defvar`s.

Right. But why is that a problem?





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  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
  0 siblings, 1 reply; 20+ messages in thread
From: Noam Postavsky @ 2018-08-02 13:28 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: IRIE Shinsuke, 5950, Stefan Monnier

On 2 August 2018 at 09:08, Clément Pit-Claudel <cpitclaudel@gmail.com> wrote:

>> Usually the warning is due to the defvaralias call being performed after
>> the corresponding `defvar`s.
>
> Right. But why is that a problem?

The typical failure case goes like this, user does this:

(setq the-package-setting 'foo)
(require 'the-package)

And the-package.el does this:

(defvar the-package-real-setting 'bar)
;; Oops! User's setting of `foo' is overwritten here:
(defvaralias 'the-package-setting 'the-package-real-setting)

See Bug#31603 for a real life example.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31603





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2018-08-02 13:28                     ` Noam Postavsky
@ 2018-08-02 14:37                       ` Clément Pit-Claudel
  2018-08-02 17:03                         ` Stefan Monnier
  0 siblings, 1 reply; 20+ messages in thread
From: Clément Pit-Claudel @ 2018-08-02 14:37 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: IRIE Shinsuke, 5950, Stefan Monnier

On 2018-08-02 09:28, Noam Postavsky wrote:
> On 2 August 2018 at 09:08, Clément Pit-Claudel <cpitclaudel@gmail.com> wrote:
> 
>>> Usually the warning is due to the defvaralias call being performed after
>>> the corresponding `defvar`s.
>>
>> Right. But why is that a problem?
> 
> The typical failure case goes like this, user does this:
> 
> (setq the-package-setting 'foo)
> (require 'the-package)
> 
> And the-package.el does this:
> 
> (defvar the-package-real-setting 'bar)
> ;; Oops! User's setting of `foo' is overwritten here:
> (defvaralias 'the-package-setting 'the-package-real-setting)

Thanks.  Should the warning be disabled when both variables are already `defvar'd, then?

Clément.





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  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
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Monnier @ 2018-08-02 17:03 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: IRIE Shinsuke, 5950, Noam Postavsky

>> The typical failure case goes like this, user does this:
>> 
>> (setq the-package-setting 'foo)
>> (require 'the-package)
>> 
>> And the-package.el does this:
>> 
>> (defvar the-package-real-setting 'bar)
>> ;; Oops! User's setting of `foo' is overwritten here:
>> (defvaralias 'the-package-setting 'the-package-real-setting)
>
> Thanks.  Should the warning be disabled when both variables are
> already `defvar'd, then?

Why?  Replace `setq` with `defvar` in the above scenario and you have
the same problem.


        Stefan





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  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:58                             ` Stefan Monnier
  0 siblings, 2 replies; 20+ messages in thread
From: Clément Pit-Claudel @ 2018-08-03  3:33 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: IRIE Shinsuke, 5950, Noam Postavsky

On 2018-08-02 13:03, Stefan Monnier wrote:
>>> The typical failure case goes like this, user does this:
>>>
>>> (setq the-package-setting 'foo)
>>> (require 'the-package)
>>>
>>> And the-package.el does this:
>>>
>>> (defvar the-package-real-setting 'bar)
>>> ;; Oops! User's setting of `foo' is overwritten here:
>>> (defvaralias 'the-package-setting 'the-package-real-setting)
>>
>> Thanks.  Should the warning be disabled when both variables are
>> already `defvar'd, then?
> 
> Why?  Replace `setq` with `defvar` in the above scenario and you have
> the same problem.

Do you? I thought the problem was that you were overwriting a user setting… if you change the first setq into a defvar, where's the user setting that you're overwriting?





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  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 21:58                             ` Stefan Monnier
  1 sibling, 1 reply; 20+ messages in thread
From: Noam Postavsky @ 2018-08-03 20:23 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: IRIE Shinsuke, 5950, Stefan Monnier

On 2 August 2018 at 23:33, Clément Pit-Claudel <cpitclaudel@gmail.com> wrote:
> On 2018-08-02 13:03, Stefan Monnier wrote:
>>>> The typical failure case goes like this, user does this:
>>>>
>>>> (setq the-package-setting 'foo)
>>>> (require 'the-package)
>>>>
>>>> And the-package.el does this:
>>>>
>>>> (defvar the-package-real-setting 'bar)
>>>> ;; Oops! User's setting of `foo' is overwritten here:
>>>> (defvaralias 'the-package-setting 'the-package-real-setting)
>>>
>>> Thanks.  Should the warning be disabled when both variables are
>>> already `defvar'd, then?
>>
>> Why?  Replace `setq` with `defvar` in the above scenario and you have
>> the same problem.
>
> Do you? I thought the problem was that you were overwriting a user setting… if you change the first setq into a defvar, where's the user setting that you're overwriting?

If the user writes defvar instead of setq, it has the exact same
effect: they've set the-package-setting to foo, which will be
overwritten to bar, same as before.





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2018-08-03 20:23                             ` Noam Postavsky
@ 2018-08-03 21:00                               ` Clément Pit-Claudel
  2018-08-03 22:09                                 ` Noam Postavsky
  0 siblings, 1 reply; 20+ messages in thread
From: Clément Pit-Claudel @ 2018-08-03 21:00 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: IRIE Shinsuke, 5950, Stefan Monnier

On 2018-08-03 16:23, Noam Postavsky wrote:
> On 2 August 2018 at 23:33, Clément Pit-Claudel <cpitclaudel@gmail.com> wrote:
>> On 2018-08-02 13:03, Stefan Monnier wrote:
>>>>> The typical failure case goes like this, user does this:
>>>>>
>>>>> (setq the-package-setting 'foo)
>>>>> (require 'the-package)
>>>>>
>>>>> And the-package.el does this:
>>>>>
>>>>> (defvar the-package-real-setting 'bar)
>>>>> ;; Oops! User's setting of `foo' is overwritten here:
>>>>> (defvaralias 'the-package-setting 'the-package-real-setting)
>>>>
>>>> Thanks.  Should the warning be disabled when both variables are
>>>> already `defvar'd, then?
>>>
>>> Why?  Replace `setq` with `defvar` in the above scenario and you have
>>> the same problem.
>>
>> Do you? I thought the problem was that you were overwriting a user setting… if you change the first setq into a defvar, where's the user setting that you're overwriting?
> 
> If the user writes defvar instead of setq, it has the exact same
> effect: they've set the-package-setting to foo, which will be
> overwritten to bar, same as before.

Right, I understand that.  But why would a user write 'defvar' to set a user variable?
I guess I'm just a bit confused by the warning.  What's the proper way to resolve it?  Should I create the alias before loading flycheck?






^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2018-08-03  3:33                           ` Clément Pit-Claudel
  2018-08-03 20:23                             ` Noam Postavsky
@ 2018-08-03 21:58                             ` Stefan Monnier
  1 sibling, 0 replies; 20+ messages in thread
From: Stefan Monnier @ 2018-08-03 21:58 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: IRIE Shinsuke, 5950, Noam Postavsky

> Do you?  I thought the problem was that you were overwriting a user setting…
> if you change the first setq into a defvar, where's the user setting that
> you're overwriting?

No, the problem is that you have 2 conflicting settings.
Whether one comes from the user doesn't really matter.


        Stefan





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  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
  0 siblings, 1 reply; 20+ messages in thread
From: Noam Postavsky @ 2018-08-03 22:09 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: 5950, IRIE Shinsuke, Stefan Monnier

Clément Pit-Claudel <cpitclaudel@gmail.com> writes:

> But why would a user write 'defvar' to set a user variable?

I don't know, why would a user write 'defvaralias' to set a user
variable?  They're users, completely unpredictable!  ;)

> What's the proper way to resolve it?  Should I create the alias before
> loading flycheck?

That would work too.  I made a couple of other suggestions in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=5950#42

Maybe we should add something like

    (defun defvaralias! (new-alias base-variable &optional docstring)
      (set new-alias (symbol-value base-variable))
      (defvaralias new-alias base-variable docstring))

Which you could use to tell Emacs not to care about losing the original
value of NEW-ALIAS.





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#5950: defvaralias after defvar should be warned in runtime
  2018-08-03 22:09                                 ` Noam Postavsky
@ 2018-08-03 22:24                                   ` Clément Pit-Claudel
  0 siblings, 0 replies; 20+ messages in thread
From: Clément Pit-Claudel @ 2018-08-03 22:24 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 5950, IRIE Shinsuke, Stefan Monnier

On 2018-08-03 18:09, Noam Postavsky wrote:
> That would work too.  I made a couple of other suggestions in
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=5950#42

Thanks, I missed that reply :) 
Adding setq before the defalias works fine :) Thanks a lot!





^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-08-03 22:24 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).