* Warnings about keymaps
@ 2009-09-11 9:04 Eli Zaretskii
2009-09-11 17:34 ` Glenn Morris
2009-09-11 17:43 ` Stefan Monnier
0 siblings, 2 replies; 13+ messages in thread
From: Eli Zaretskii @ 2009-09-11 9:04 UTC (permalink / raw
To: emacs-devel
Compilation of Lisp files yields warnings such as these two:
Compiling emacs/lisp/cvs-status.el
In toplevel form:
emacs/lisp/cvs-status.el:93:22:Warning: variable assignment to constant
`cvs-status-mode-map'
Compiling emacs/lisp/diff-mode.el
In toplevel form:
emacs/lisp/diff-mode.el:1273:70:Warning: variable assignment to
constant `diff-mode-map'
Sounds like a bug to me.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-11 9:04 Warnings about keymaps Eli Zaretskii
@ 2009-09-11 17:34 ` Glenn Morris
2009-09-11 17:43 ` Stefan Monnier
1 sibling, 0 replies; 13+ messages in thread
From: Glenn Morris @ 2009-09-11 17:34 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii wrote:
> emacs/lisp/cvs-status.el:93:22:Warning: variable assignment to constant
> `cvs-status-mode-map'
I think this is because easy-mmode-defmap defines cvs-status-mode-map
as a constant. Then, later on, the define-derived-mode macro tries to
do a defvar on cvs-status-mode-map. The simple change is to make
easy-mmode-defmap use defvar instead of defconst; the slightly more
correct thing (that I could not get to work...) is to make
define-derived-mode only defvar the mode map if it is not already
defined.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-11 9:04 Warnings about keymaps Eli Zaretskii
2009-09-11 17:34 ` Glenn Morris
@ 2009-09-11 17:43 ` Stefan Monnier
2009-09-14 19:10 ` Glenn Morris
1 sibling, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2009-09-11 17:43 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
> Compilation of Lisp files yields warnings such as these two:
> Compiling emacs/lisp/cvs-status.el
> In toplevel form:
> emacs/lisp/cvs-status.el:93:22:Warning: variable assignment to constant
> `cvs-status-mode-map'
> Compiling emacs/lisp/diff-mode.el
> In toplevel form:
> emacs/lisp/diff-mode.el:1273:70:Warning: variable assignment to
> constant `diff-mode-map'
> Sounds like a bug to me.
Yes, there's some kind of bug in there. The bug can be seen more
directly by byte-compiling the file below:
(defconst foo nil)
(defvar foo nil)
Looks like the "assignment-to-constant" warning should be more careful
to distinguish defvar from setq.
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-11 17:43 ` Stefan Monnier
@ 2009-09-14 19:10 ` Glenn Morris
2009-09-14 21:46 ` Stefan Monnier
0 siblings, 1 reply; 13+ messages in thread
From: Glenn Morris @ 2009-09-14 19:10 UTC (permalink / raw
To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel
Stefan Monnier wrote:
> The bug can be seen more directly by byte-compiling the file below:
>
> (defconst foo nil)
> (defvar foo nil)
>
> Looks like the "assignment-to-constant" warning should be more
> careful to distinguish defvar from setq.
This seems a fairly nonsensical thing to do, so why shouldn't the
compiler warn about it? The defvar is a best a no-op, at worst a bug
(if you were expecting it to have an effect).
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-14 19:10 ` Glenn Morris
@ 2009-09-14 21:46 ` Stefan Monnier
2009-09-15 2:45 ` Glenn Morris
2009-09-15 15:13 ` Richard Stallman
0 siblings, 2 replies; 13+ messages in thread
From: Stefan Monnier @ 2009-09-14 21:46 UTC (permalink / raw
To: Glenn Morris; +Cc: Eli Zaretskii, emacs-devel
>> The bug can be seen more directly by byte-compiling the file below:
>>
>> (defconst foo nil)
>> (defvar foo nil)
>>
>> Looks like the "assignment-to-constant" warning should be more
>> careful to distinguish defvar from setq.
> This seems a fairly nonsensical thing to do, so why shouldn't the
> compiler warn about it?
It could, but it shouldn't complain about assignment to a constant,
since the defvar will not perform any assignment. I.e. maybe a warning
is OK, but not the warning we currently get.
> The defvar is a best a no-op, at worst a bug
> (if you were expecting it to have an effect).
In the OP's case the defvar was expected to have as effect to "provide
a default value to the var in case it's not defined elsewhere". So it
seems to behave as expected (it's a no-op since the var is defined
elsewhere).
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-14 21:46 ` Stefan Monnier
@ 2009-09-15 2:45 ` Glenn Morris
2009-09-15 13:40 ` Stefan Monnier
2009-09-15 15:13 ` Richard Stallman
1 sibling, 1 reply; 13+ messages in thread
From: Glenn Morris @ 2009-09-15 2:45 UTC (permalink / raw
To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel
Stefan Monnier wrote:
> It could, but it shouldn't complain about assignment to a constant,
> since the defvar will not perform any assignment. I.e. maybe a warning
> is OK, but not the warning we currently get.
How about this? Or perhaps the defvar should just be silently
optimized away, like the default-boundp test seems to be trying to do.
*** bytecomp.el 10 Sep 2009 06:22:30 -0000 2.258
--- bytecomp.el 14 Sep 2009 23:30:56 -0000
***************
*** 4025,4032 ****
(let ((tmp (make-symbol "defconst-tmp-var")))
`(funcall '(lambda (,tmp) (defconst ,var ,tmp))
,value))
;; `defvar' sets `var' only when unbound.
! `(if (not (default-boundp ',var)) (setq-default ,var ,value))))
(when (eq fun 'defconst)
;; This will signal an appropriate error at runtime.
`(eval ',form)))
--- 4036,4046 ----
(let ((tmp (make-symbol "defconst-tmp-var")))
`(funcall '(lambda (,tmp) (defconst ,var ,tmp))
,value))
+ (if (memq var byte-compile-const-variables)
+ (byte-compile-warn "attempt to defvar constant `%s' \
+ has no effect" var)
;; `defvar' sets `var' only when unbound.
! `(if (not (default-boundp ',var)) (setq-default ,var ,value)))))
(when (eq fun 'defconst)
;; This will signal an appropriate error at runtime.
`(eval ',form)))
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-15 2:45 ` Glenn Morris
@ 2009-09-15 13:40 ` Stefan Monnier
2009-09-17 7:32 ` Glenn Morris
0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2009-09-15 13:40 UTC (permalink / raw
To: Glenn Morris; +Cc: Eli Zaretskii, emacs-devel
>> It could, but it shouldn't complain about assignment to a constant,
>> since the defvar will not perform any assignment. I.e. maybe a warning
>> is OK, but not the warning we currently get.
> How about this? Or perhaps the defvar should just be silently
> optimized away, like the default-boundp test seems to be trying to do.
This warning might be OK, but I have some issues with it:
- most importantly: it doesn't solve the problem at hand which is how
to get rid of the other warning.
- what's its use? It's not like we've seen lots of bugs where people
use both defconst and defvar for a variable.
- Finally, if the defconst+defvar is really what you want to do (as in
the present case where the defconst and the defvar both come from
macros that we may not want to change), how do you silence
the warning?
Stefan
> *** bytecomp.el 10 Sep 2009 06:22:30 -0000 2.258
> --- bytecomp.el 14 Sep 2009 23:30:56 -0000
> ***************
> *** 4025,4032 ****
> (let ((tmp (make-symbol "defconst-tmp-var")))
> `(funcall '(lambda (,tmp) (defconst ,var ,tmp))
> ,value))
> ;; `defvar' sets `var' only when unbound.
> ! `(if (not (default-boundp ',var)) (setq-default ,var ,value))))
> (when (eq fun 'defconst)
> ;; This will signal an appropriate error at runtime.
> `(eval ',form)))
> --- 4036,4046 ----
> (let ((tmp (make-symbol "defconst-tmp-var")))
> `(funcall '(lambda (,tmp) (defconst ,var ,tmp))
> ,value))
> + (if (memq var byte-compile-const-variables)
> + (byte-compile-warn "attempt to defvar constant `%s' \
> + has no effect" var)
> ;; `defvar' sets `var' only when unbound.
> ! `(if (not (default-boundp ',var)) (setq-default ,var ,value)))))
> (when (eq fun 'defconst)
> ;; This will signal an appropriate error at runtime.
> `(eval ',form)))
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-14 21:46 ` Stefan Monnier
2009-09-15 2:45 ` Glenn Morris
@ 2009-09-15 15:13 ` Richard Stallman
1 sibling, 0 replies; 13+ messages in thread
From: Richard Stallman @ 2009-09-15 15:13 UTC (permalink / raw
To: Stefan Monnier; +Cc: eliz, emacs-devel
Should there be a way to undo the "constant" marking that defconst makes?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-15 13:40 ` Stefan Monnier
@ 2009-09-17 7:32 ` Glenn Morris
2009-09-17 14:02 ` Stefan Monnier
0 siblings, 1 reply; 13+ messages in thread
From: Glenn Morris @ 2009-09-17 7:32 UTC (permalink / raw
To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel
Stefan Monnier wrote:
> - most importantly: it doesn't solve the problem at hand which is how
> to get rid of the other warning.
The other warning?
> - what's its use? It's not like we've seen lots of bugs where people
> use both defconst and defvar for a variable.
It simply replaces the current "variable assignment to a constant"
warning with a slightly more informative one, as you suggested.
> - Finally, if the defconst+defvar is really what you want to do (as in
> the present case where the defconst and the defvar both come from
> macros that we may not want to change), how do you silence
> the warning?
The same way you silence the current one about variable assignment to
a constant. :)
I don't think is a particularly useful change. I think it would be
better to change define-derived-mode to not defvar the map if it is
already defined; or to change easy-mode-defmap to not use defconst
(are you saying users should not add key bindings to
log-edit-mode-map, for example?).
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-17 7:32 ` Glenn Morris
@ 2009-09-17 14:02 ` Stefan Monnier
2009-09-18 22:25 ` Glenn Morris
0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2009-09-17 14:02 UTC (permalink / raw
To: Glenn Morris; +Cc: Eli Zaretskii, emacs-devel
>> - most importantly: it doesn't solve the problem at hand which is how
>> to get rid of the other warning.
> The other warning?
>> - what's its use? It's not like we've seen lots of bugs where people
>> use both defconst and defvar for a variable.
> It simply replaces the current "variable assignment to a constant"
> warning with a slightly more informative one, as you suggested.
Sorry, I misread your patch. So you actually completely remove the
defvar if var was already defconst'd. That's an attractive approach,
tho it's a bit risky, because byte-compile-const-variables only contains
variables that "may have been defconst'd", not those that "we know for
sure have been defconst'd". E.g. try it with code such as
(when toto
(defconst foo bar))
(defvar foo nil)
with your patch, you'd end up with `foo' undefined in the case where
toto is nil. It may seem like a braindead case, but when comparing
"broken code in corner cases" vs "incorrect warning in some cases",
I prefer the incorrect warning.
>> - Finally, if the defconst+defvar is really what you want to do (as in
>> the present case where the defconst and the defvar both come from
>> macros that we may not want to change), how do you silence
>> the warning?
> The same way you silence the current one about variable assignment to
> a constant. :)
The difference is that the current one is incorrect, so the act of
silencing it needs to be done in bytecomp.el. Whereas yours is meant to
be correct, in which case silencing it would need to be done by tweaking
the code and/or using with-no-warnings (which I profoundly disklike).
> I think it would be better to change define-derived-mode to not defvar
> the map if it is already defined;
I don't know how to do that.
> or to change easy-mode-defmap to not use defconst
That might be an acceptable workaround. But the underlying problem will
still be there.
> (are you saying users should not add key bindings to
> log-edit-mode-map, for example?).
That's unrelated: defconst only says that the variable's binding won't
change, not that the object it is bound to is immutable. My locally
hacked Emacs has "strong defconst" which really makes the variable
immutable, so (setq log-edit-mode-map <blabla>) signals an error, but
(define-key log-edit-mode-map [foo] 'bar) still works just fine.
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-17 14:02 ` Stefan Monnier
@ 2009-09-18 22:25 ` Glenn Morris
2009-09-19 0:04 ` Stephen J. Turnbull
2009-09-19 0:43 ` Stefan Monnier
0 siblings, 2 replies; 13+ messages in thread
From: Glenn Morris @ 2009-09-18 22:25 UTC (permalink / raw
To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel
Stefan Monnier wrote:
> It may seem like a braindead case, but when comparing "broken code
> in corner cases" vs "incorrect warning in some cases", I prefer the
> incorrect warning.
Definitely.
>> I think it would be better to change define-derived-mode to not defvar
>> the map if it is already defined;
>
> I don't know how to do that.
Oh well, that makes two of us.
>> or to change easy-mode-defmap to not use defconst
>
> That might be an acceptable workaround. But the underlying problem will
> still be there.
But under a carpet. :)
>> (are you saying users should not add key bindings to
>> log-edit-mode-map, for example?).
>
> That's unrelated: defconst only says that the variable's binding won't
> change, not that the object it is bound to is immutable.
I don't understand that distinction. I just read the doc of defconst:
The intent is that neither programs nor users should ever change
this value.
which seems clear to me.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-18 22:25 ` Glenn Morris
@ 2009-09-19 0:04 ` Stephen J. Turnbull
2009-09-19 0:43 ` Stefan Monnier
1 sibling, 0 replies; 13+ messages in thread
From: Stephen J. Turnbull @ 2009-09-19 0:04 UTC (permalink / raw
To: Glenn Morris; +Cc: Eli Zaretskii, Stefan Monnier, emacs-devel
Glenn Morris writes:
> Stefan Monnier wrote:
> > That's unrelated: defconst only says that the variable's binding won't
> > change, not that the object it is bound to is immutable.
>
> I don't understand that distinction. I just read the doc of defconst:
>
> The intent is that neither programs nor users should ever change
> this value.
It's the same distinction as between "const *" and "* const" in C.
The value of the variable is a constant object, but that object may be
mutable if it is a container. Under this definition, if you were
implementing Emacs Lisp in Emacs Lisp, it would make sense to
implement the obarray as a defconst. (You probably already knew that,
but your question literally says "that distinction", not "why you make
that distinction".)
I would like to know "why?" though. XEmacs's docstring says "The
intent is that programs do not change this value, but users may." The
awkward expression of the Emacs docstring suggests that the XEmacs
version is indeed the original. Can somebody explain why it was
changed? Is Emacs moving toward true constants? (Eg, in r/o memory?)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warnings about keymaps
2009-09-18 22:25 ` Glenn Morris
2009-09-19 0:04 ` Stephen J. Turnbull
@ 2009-09-19 0:43 ` Stefan Monnier
1 sibling, 0 replies; 13+ messages in thread
From: Stefan Monnier @ 2009-09-19 0:43 UTC (permalink / raw
To: Glenn Morris; +Cc: Eli Zaretskii, emacs-devel
>>> (are you saying users should not add key bindings to
>>> log-edit-mode-map, for example?).
>> That's unrelated: defconst only says that the variable's binding won't
>> change, not that the object it is bound to is immutable.
> I don't understand that distinction.
It's the same distinction that makes it possible for a function to
modify an array passed as argument in C, even though arguments are
passed "by value" (i.e. it receives a copy of the pointer to the array,
so it can't change which array is pointed, but it can modify the
array's contents).
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2009-09-19 0:43 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-11 9:04 Warnings about keymaps Eli Zaretskii
2009-09-11 17:34 ` Glenn Morris
2009-09-11 17:43 ` Stefan Monnier
2009-09-14 19:10 ` Glenn Morris
2009-09-14 21:46 ` Stefan Monnier
2009-09-15 2:45 ` Glenn Morris
2009-09-15 13:40 ` Stefan Monnier
2009-09-17 7:32 ` Glenn Morris
2009-09-17 14:02 ` Stefan Monnier
2009-09-18 22:25 ` Glenn Morris
2009-09-19 0:04 ` Stephen J. Turnbull
2009-09-19 0:43 ` Stefan Monnier
2009-09-15 15:13 ` Richard Stallman
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.